[PATCH v2 1/9] dm: core: Document the common error codes

[Simon Glass]

HI Sean,

On Tue, 23 Mar 2021 at 17:45, Sean Anderson <seanga2 at gmail.com> wrote:
>
> On 3/23/21 12:14 AM, Simon Glass wrote:
> > Driver model uses quite strong conventions on error codes, but these are
> > currently not clearly documented. Add a description of the commonly used
> > errors.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add a patch to document the common error codes
> >
> >   doc/driver-model/design.rst | 111 ++++++++++++++++++++++++++++++++++++
> >   1 file changed, 111 insertions(+)
> >
> > diff --git a/doc/driver-model/design.rst b/doc/driver-model/design.rst
> > index 4e5cecbab6a..30a07bdf768 100644
> > --- a/doc/driver-model/design.rst
> > +++ b/doc/driver-model/design.rst
> > @@ -900,6 +900,117 @@ Some special flags are used to determine whether to remove the device:
> >   The dm_remove_devices_flags() function can be used to remove devices based on
> >   their driver flags.
> >
> > +
> > +Error codes
> > +-----------
> > +
> > +Driver model tries to use errors codes in a consistent way, as follows:
> > +
> > +\-EAGAIN
> > +   Try later, e.g. dependencies not ready
> > +
> > +\-EINVAL
> > +   Invalid argument, such as `dev_read_...()` failed or any other
> > +   devicetree-related access. Also used when a driver method is passed an
> > +   argument it considers invalid or does not support.
> > +
> > +\-EIO
> > +   Failed to perform I/O
>
> Do you mind providing a longer explanation here? What sort of IO
> failures should return EIO instead of (e.g.) ETIMEDOUT? Would ECOMM as
> used by the MMC subsystem be a good example of what to use EIO for in
> new code?

I forgot about -ETIMEDOUT, will add.

How about:

Failed to perform an I/O operation. This is used when a local device
(i.e. part of the SOC) does not work as expected. Use -EREMOTEIO for
failures to talk to a separate device, e.g. over an I2C or SPI
channel.

>
> > +
> > +\-ENODEV
> > +   Do not bind the device. This should not be used to indicate an
> > +   error probing the device or for any other purpose, lest driver model get
> > +   confused. Using `-ENODEV` inside a driver method makes no sense, since
> > +   clearly there is a device.
> > +
> > +\-ENOENT
> > +   Entry or object not found
>
> Could we add some examples here? Off the top of my head, this is used
> for missing device-tree properties/nodes, non-udevice devices (clocks,
> pinctrl groups, etc.) and of course files and directories.

How about:

Entry or object not found. This is used when a device, file, directory
cannot be found (e.g. when looked up by name), It can also indicate a
missing devicetree subnode.

>
> > +
> > +\-ENOMEM
> > +   Out of memory
> > +
> > +\-ENOSPC
> > +   Ran out of space (e.g. in a buffer or limited-size array)
> > +
> > +\-ENOSYS
> > +   Function not implemented. This is returned by uclasses where the driver does
> > +   not implement a particular method. It can also be returned by drivers when
> > +   a particular sub-method is not implemented. This is widely checked in the
> > +   wider code base, where a feature may or may not be compiled into U-Boot. It
> > +   indicates that the feature is not available, but this is often just normal
> > +   operation. Please do not use -ENOSUPP. If an incorrect or unknown argument
> > +   is provided to a method (e.g. an unknown clock ID), return -EINVAL.
> > +
> > +\-ENXIO
> > +   Couldn't find device/address
>
> How does this differ from ENODEV and ENOENT?

How about:

Couldn't find device/address. This is used when a device or address
could not be obtained or is not valid.

>
> > +
> > +\-EPERM
> > +   This is -1 so some older code may use it as a generic error. This indicates
> > +   that an operation is not permitted, e.g. a security violation or policy
> > +   constraint. It is returned internally when binding devices before relocation,
> > +   if the device is not marked for pre-relocation use.
> > +
> > +\-EPFNOSUPPORT
> > +   Missing uclass. This is deliberately an uncommon error code so that it can
> > +   easily be distinguished. If you see this very early in U-Boot, it means that
> > +   a device exists with a particular uclass but the uclass does not (mostly
> > +   likely because it is not compiled in). Enable DEBUG in uclass.c or lists.c
> > +   to see which uclass ID or driver is causing the problem.
> > +
> > +\-EREMOTEIO
> > +   Cannot talk to peripheral, e.g. i2c
>
> How does this differ from EIO or ECOMM?

This indicates an error in talking to a peripheral over a comms link,
such as I2C or SPI. It might indicate that the device is not present
or is not responding as expected.

>
> > +
> > +Less common ones:
> > +
> > +\-EKEYREJECTED
> > +   Attempt to remove a device which does not match the removal flags. See
> > +   device_remove().
> > +
> > +\-EILSEQ
> > +   Devicetree read failure, specifically trying to read a string index which
> > +   does not exist, in a string-listg property
> > +
> > +\-ENOEXEC
> > +   Attempt to use a uclass method on a device not in that uclass. This is
> > +   seldom checked at present, since it is generally a programming error and a
> > +   waste of code space. A DEBUG-only check would be useful here.
> > +
> > +\-ENODATA
> > +   Devicetree read error, where a property exists but has no data associated
> > +   with it
> > +
> > +\-EOVERFLOW
> > +   Devicetree read error, where the property is longer than expected
> > +
> > +\-EPROBE_DEFER
> > +   Attempt to remove a non-vital device when the removal flags indicate that
> > +   only vital devices should be removed
> > +
> > +\-ERANGE
> > +   Returned by regmap functions when arguments are out of range. This can be
> > +   useful for disinguishing regmap errors from other errors obtained while
> > +   probing devices.
> > +
> > +Drivers should use the same conventions so that things function as expected.
> > +In particular, if a driver fails to probe, or a uclass operation fails, the
> > +error code is the primary way to indicate what actually happened.
> > +
> > +Printing error messages in drivers is discouraged due to code size bloat and
> > +since it can result in messages appearing in normal operation. For example, if
> > +a command tries two different devices and uses whichever one probes correctly,
> > +we don't want an error message displayed, even if the command itself might show
> > +a warning or informational message.
>
> So should errors while probing always be DEBUG?

Ideally.

>
> Should misconfiguration (e.g. missing a requried devicetree property)
> and device errors be logged differently?

To me that is -EINVAL since it indicates the devicetree node is
invalid for the device.

>
> Thanks for documenting this. It is useful to know what the "official"
> stance on different return codes is, especially when existing code uses
> everything.

Yes, Tom and Marek poked me about it.

Still some grey areas and inconsistencies. But once this is figured
out I will send v3.

Regards,
Simon

>
> --Sean
>
> > +
> > +Error messages can be logged using `log_msg_ret()`, so that enabling
> > +`CONFIG_LOG` and `CONFIG_LOG_ERROR_RETURN` shows a trace of error codes returned
> > +through the call stack. That can be a handy way of quickly figuring out where
> > +an error occurred. Get into the habit of return errors with
> > +`return log_msg_ret("here", ret)` instead of just `return ret`. The string
> > +just needs to be long enough to find in a single function, since a log record
> > +stores (and can print with `CONFIG_LOGF_FUNC`) the function where it was
> > +generated.
> > +
> > +
> >   Data Structures
> >   ---------------
> >
> >
>