RFC: clarification and standardization of ENOTSUP/ENOSYS error returns

[pabigot]

See also #11207 which this extends/supersedes simply because it's newer and therefore might get some attention. If this resolves as not-worth-fixing so should that.

Historically and/or recently API wrappers are documented to and do return -ENOTSUP in cases where a system call is not implemented for a particular driver.

It turns out this is inconsistent with more precise usage in Linux and other systems. The POSIX descriptions are:

• ENOTSUP Not supported.
• ENOSYS Function not supported.

which fails to provide clarity. The best summary I can find of existing practice is from Samba which supports the description proposed in #11207:

• -ENOSYS driver has not implemented this function.
• -ENOTSUP device does not support given configuration at this time.

A real-world example is:

• A GPIO peripheral does not support interrupts at all; thus gpio_pin_interrupt_configure might legitimately return -ENOSYS;
• A GPIO peripheral supports edge interrupts, but not level interrupts; thus a call that requests level interrupts would return -ENOTSUP.

If we were to do anything about this the task is go through the Zephyr baseline and update API to conform to this interpretation. This includes:

• All the syscall wrappers that currently look at the api table and return -ENOTSUP when the required function pointer is null should instead be returning -ENOSYS;
• All instances of -ENOTSUP in drivers should be inspected and switched to -ENOSYS if there is no condition under which the call might succeed;

There are over 600 of returns of -ENOTSUP in the drivers/ subdirectory alone. There are 11 returns of -ENOSYS in the entire Zephyr tree.

The API meeting 2020-03-24, in the context of discussing #23661, suggests continuing to use -ENOTSUP for both cases unless/until we commit to changing this. Such a change is an API change per this description in that user code must be reviewed and changed to distinguish between -ENOTSUP (current) and -ENOSYS (future).

We may want to decide now whether the trauma of making this change is worthwhile, and if so, determine the timing and constraints on such a change.

If it's not worth doing this RFC can serve as the record of why we should consistently use -ENOTSUP for both situations, and possibly some of the existing -ENOSYS should be changed.

[de-nordic]

I think that rationale presented here by @pabigot is valid and we should fallow with proposed changes.

I have one, controversial, insight: I think that in some cases code that would return -ENOSYS should be allowed to be conditionally thrown out of compilation; there are some cases when such error would be caught once, while the developer figures out that the hardware does not poses given function, or never, when the developer knew while writing the code; afterwards the code would never be executed again, but would be carried with firmware anyway.

[carlescufi]

API meeting 2021.03.23:

Conclusion on this matter:

• Zephyr is to use ENOTSUP exclusively for the time being
• ENOSYS is not allowed until further notice
• An issue will be opened to:
  • The usages of ENOSYS in the tree will be replaced by ENOTSUP
  • checkpatch will be modified so that the error message matches Zephyr's use case
• Post-LTS we need to re-evaluate this decision along with an in-depth look at error codes in general

[nashif]

after sleeping on it and going through the drivers we have right now I think this is the wrong approach:

* Zephyr is to use `ENOTSUP` exclusively for the time being

Both NOSYS and NOTSUP have their own well document usecases and we should not pick one over the other and force using a return code that in some cases might not be suitable

* `ENOSYS` is not allowed until further notice

Not allowing a well established and well documented return code is an extreme. There are definitely use cases for this in Zephyr and we can't just error in checkpatch whenever this is being used. Current checkpatch check should just be removed and usage should be allowed and in the future should get better reviews.

There are over 600 of returns of -ENOTSUP in the drivers/ sub-directory alone. There are 11 returns of -ENOSYS in the entire Zephyr tree.

Looking quickly on all those occurrences in drivers I can see that the usage of NOTSUP is correct in majority of the cases, there is no need to change everything. NOTSUP has been used correctly for the most part.

We also have to acknowledge that the incorrect usage and mixup is the direct result of having a checkpatch warning that is Linux specific, so this has contributed to developers moving away from NOSYS to fix CI and checkpatch warnings.

The only cases where the usage is not correct is for APIs that are not implemented. In many cases we have those return an error in the driver implementation itself, i.e. they are not implemented but available as part of the driver. Such empty implementations could be removed and the check could be done on the API level. Did not count those, but this is the minority of occurrences of incorrect usage of NOTSUP under drivers/. There is also some inconsistency dealing with unimplemented functions in the API itself.

We have the opportunity to fix this now and close this topic once and for all instead of revisiting it every year.

[carlescufi]

Looking quickly on all those occurrences in drivers I can see that the usage of NOTSUP is correct in majority of the cases, there is no need to change everything. NOTSUP has been used correctly for the most part.

How so? According to the definition below that statement is not correct?

-ENOSYS driver has not implemented this function.
-ENOTSUP device does not support given configuration at this time.

EDIT: I do see now that many of the cases in drivers/ are actually proper usage of ENOTSUP. This is of course not the case of include/drivers

[de-nordic]

Such empty implementations could be removed and the check could be done on the API level.

This is additional check over interface that in, let say, 90% of a time would result in call anyway. I think it is more efficient to just enforce implementation of every API call, as a simple function that will return error code, rather than adding additional check in call path.
In case of simple, direct calls user will, in most cases, quickly figure out while testing that the function is not supported and will have to change code anyway, and the API level checks will then result in calls in 100%, but will remain there.

The other case is when iterating over some interface, when API function are called on some list of objects. The user will have to process error codes for objects that do not support the call so it does not matter for the user whether the API check returns the error or function is called from the object and returns it.