From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Wed, 24 Mar 2021 12:00:48 -0400 Subject: [PATCH v2 1/9] dm: core: Document the common error codes In-Reply-To: References: <20210323041427.3252184-1-sjg@chromium.org> <20210323041427.3252184-2-sjg@chromium.org> Message-ID: <8f1c75e5-e0db-5c3e-089a-b382fc7291ed@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 3/23/21 1:40 AM, Simon Glass wrote: > HI Sean, > > On Tue, 23 Mar 2021 at 17:45, Sean Anderson 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 >>> --- >>> >>> 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. Sounds good. >> >>> + >>> +\-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. I think this still needs to be clarified. Both ENODEV and ENOENT may be used to indicate a missing device. From what I have seen, this tends to be used as a "third error code" when ENOENT is already used for some purpose. >> >>> + >>> +\-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. Should ECOMM be used in new code? The current users are the MMC subsystem for when there is a CRC error, and dm_i2c_ops.xfer for unsupported speeds (though no one seems to implement that). --Sean > >> >>> + >>> +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 >>> --------------- >>> >>> >>