[PATCH v2 0/9] Use -ENOSYS consistently

[PATCH v2 0/9] Use -ENOSYS consistently

[Simon Glass]

A few places use -ENOTSUPP when they should use -ENOSYS. In two cases both
are used. This little series tidies this up and documents the conventions.

Changes in v2:
- Add a patch to document the common error codes
- Add new patch for acpi_get_path()
- Add new patch to update clk drivers to use -EINVAL

Simon Glass (9):
  dm: core: Document the common error codes
  dm: core: Use -ENOSPC in acpi_get_path()
  usb: Return -ENOSYS when system call is not available
  spi: Return -ENOSYS when system call is not available
  tlv_eeprom: Return -ENOSYS when system call is not available
  clk: Update drivers to use -EINVAL
  clk: Return -ENOSYS when system call is not available
  simple-pm-bus: Use -ENOSYS for checking missing system call
  pinctrl: Return -ENOSYS when system call is not available

 doc/driver-model/design.rst            | 111 +++++++++++++++++++++++++
 drivers/clk/aspeed/clk_ast2600.c       |   2 +-
 drivers/clk/clk-composite.c            |   8 +-
 drivers/clk/clk-hsdk-cgu.c             |   4 +-
 drivers/clk/imx/clk-imx8.c             |   4 +-
 drivers/clk/imx/clk-imx8qm.c           |   6 +-
 drivers/clk/imx/clk-imx8qxp.c          |   6 +-
 drivers/clk/imx/clk-pllv3.c            |   2 +-
 drivers/clk/kendryte/bypass.c          |   2 +-
 drivers/clk/kendryte/clk.c             |   2 +-
 drivers/clk/mvebu/armada-37xx-periph.c |   6 +-
 drivers/core/acpi.c                    |   2 +-
 drivers/core/simple-pm-bus.c           |   4 +-
 drivers/pinctrl/pinctrl-uclass.c       |  10 ++-
 drivers/usb/gadget/udc/udc-uclass.c    |   2 +-
 include/spi-mem.h                      |   2 +-
 include/tlv_eeprom.h                   |   6 +-
 17 files changed, 146 insertions(+), 33 deletions(-)

-- 
2.31.0.rc2.261.g7f71774620-goog

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

[Simon Glass]

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@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
+
+\-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
+
+\-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
+
+\-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
+
+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.
+
+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
 ---------------
 
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 2/9] dm: core: Use -ENOSPC in acpi_get_path()

[Simon Glass]

Update this function to use -ENOSPC which is more commly used when a buffer
runs out of space.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch for acpi_get_path()

 drivers/core/acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
index 0901b9260a1..2176d8b8365 100644
--- a/drivers/core/acpi.c
+++ b/drivers/core/acpi.c
@@ -91,7 +91,7 @@ int acpi_get_path(const struct udevice *dev, char *out_path, int maxlen)
 	path = dev_read_string(dev, "acpi,path");
 	if (path) {
 		if (strlen(path) >= maxlen)
-			return -E2BIG;
+			return -ENOSPC;
 		strcpy(out_path, path);
 		return 0;
 	}
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 3/9] usb: Return -ENOSYS when system call is not available

[Simon Glass]

Update usb_gadget_release() to use -ENOSYS, which is the correct error
code for U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/usb/gadget/udc/udc-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/udc-uclass.c b/drivers/usb/gadget/udc/udc-uclass.c
index 3053ccf7d97..dbc354e84f9 100644
--- a/drivers/usb/gadget/udc/udc-uclass.c
+++ b/drivers/usb/gadget/udc/udc-uclass.c
@@ -45,7 +45,7 @@ int usb_gadget_release(int index)
 		dev_array[index] = NULL;
 	return ret;
 #else
-	return -ENOTSUPP;
+	return -ENOSYS;
 #endif
 }
 
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 4/9] spi: Return -ENOSYS when system call is not available

[Simon Glass]

Update spi_controller_dma_map_mem_op_data() to use -ENOSYS, which is the
correct error code for U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/spi-mem.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/spi-mem.h b/include/spi-mem.h
index 8be3e2bf6b5..e354c388979 100644
--- a/include/spi-mem.h
+++ b/include/spi-mem.h
@@ -222,7 +222,7 @@ spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
 				   const struct spi_mem_op *op,
 				   struct sg_table *sg)
 {
-	return -ENOTSUPP;
+	return -ENOSYS;
 }
 
 static inline void
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 5/9] tlv_eeprom: Return -ENOSYS when system call is not available

[Simon Glass]

When CMD_TLV_EEPROM is not enabled, use -ENOSYS, which is the correct
error code for U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/tlv_eeprom.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index 1de2fe2337c..a2c333e7446 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -114,19 +114,19 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
 
 static inline int read_tlv_eeprom(void *eeprom, int offset, int len, int dev)
 {
-	return -ENOTSUPP;
+	return -ENOSYS;
 }
 
 static inline int write_tlv_eeprom(void *eeprom, int len)
 {
-	return -ENOTSUPP;
+	return -ENOSYS;
 }
 
 static inline int
 read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
 			struct tlvinfo_tlv **first_entry, int dev)
 {
-	return -ENOTSUPP;
+	return -ENOSYS;
 }
 
 #endif /* CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 6/9] clk: Update drivers to use -EINVAL

[Simon Glass]

At present some drivers use -ENOSUPP to indicate that an unknown or
unsupported clock is used. Most use -EINVAL, indicating an invalid value,
so convert everything to that.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to update clk drivers to use -EINVAL

 drivers/clk/aspeed/clk_ast2600.c       | 2 +-
 drivers/clk/clk-hsdk-cgu.c             | 4 ++--
 drivers/clk/imx/clk-imx8.c             | 4 ++--
 drivers/clk/imx/clk-imx8qm.c           | 6 +++---
 drivers/clk/imx/clk-imx8qxp.c          | 6 +++---
 drivers/clk/imx/clk-pllv3.c            | 2 +-
 drivers/clk/kendryte/bypass.c          | 2 +-
 drivers/clk/kendryte/clk.c             | 2 +-
 drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
 9 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
index acb7eca7414..3a92739f5cf 100644
--- a/drivers/clk/aspeed/clk_ast2600.c
+++ b/drivers/clk/aspeed/clk_ast2600.c
@@ -1140,7 +1140,7 @@ int soc_clk_dump(void)
 
 		clk_free(&clk);
 
-		if (ret == -ENOTSUPP) {
+		if (ret == -EINVAL) {
 			printf("clk ID %lu not supported yet\n",
 			       aspeed_clk_names[i].id);
 			continue;
diff --git a/drivers/clk/clk-hsdk-cgu.c b/drivers/clk/clk-hsdk-cgu.c
index 449b430e230..26b0aa9a26f 100644
--- a/drivers/clk/clk-hsdk-cgu.c
+++ b/drivers/clk/clk-hsdk-cgu.c
@@ -718,7 +718,7 @@ static ulong hsdk_cgu_set_rate(struct clk *sclk, ulong rate)
 	if (clk->map[sclk->id].set_rate)
 		return clk->map[sclk->id].set_rate(sclk, rate);
 
-	return -ENOTSUPP;
+	return -EINVAL;
 }
 
 static int hsdk_cgu_disable(struct clk *sclk)
@@ -731,7 +731,7 @@ static int hsdk_cgu_disable(struct clk *sclk)
 	if (clk->map[sclk->id].disable)
 		return clk->map[sclk->id].disable(sclk);
 
-	return -ENOTSUPP;
+	return -EINVAL;
 }
 
 static const struct clk_ops hsdk_cgu_ops = {
diff --git a/drivers/clk/imx/clk-imx8.c b/drivers/clk/imx/clk-imx8.c
index 8484613eed5..b3dc138c4bb 100644
--- a/drivers/clk/imx/clk-imx8.c
+++ b/drivers/clk/imx/clk-imx8.c
@@ -29,7 +29,7 @@ __weak ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
 
 __weak int __imx8_clk_enable(struct clk *clk, bool enable)
 {
-	return -ENOTSUPP;
+	return -EINVAL;
 }
 
 static int imx8_clk_disable(struct clk *clk)
@@ -70,7 +70,7 @@ int soc_clk_dump(void)
 
 		clk_free(&clk);
 
-		if (ret == -ENOTSUPP) {
+		if (ret == -EINVAL) {
 			printf("clk ID %lu not supported yet\n",
 			       imx8_clk_names[i].id);
 			continue;
diff --git a/drivers/clk/imx/clk-imx8qm.c b/drivers/clk/imx/clk-imx8qm.c
index 7e466d630a0..7759dc63ee1 100644
--- a/drivers/clk/imx/clk-imx8qm.c
+++ b/drivers/clk/imx/clk-imx8qm.c
@@ -133,7 +133,7 @@ ulong imx8_clk_get_rate(struct clk *clk)
 			       __func__, clk->id);
 			return -EINVAL;
 		}
-		return -ENOTSUPP;
+		return -EINVAL;
 	};
 
 	ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
@@ -237,7 +237,7 @@ ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
 			       __func__, clk->id);
 			return -EINVAL;
 		}
-		return -ENOTSUPP;
+		return -EINVAL;
 	};
 
 	ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
@@ -337,7 +337,7 @@ int __imx8_clk_enable(struct clk *clk, bool enable)
 			       __func__, clk->id);
 			return -EINVAL;
 		}
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index e6b2fb40da2..ffa2fcee0b2 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -126,7 +126,7 @@ ulong imx8_clk_get_rate(struct clk *clk)
 			       __func__, clk->id);
 			return -EINVAL;
 		}
-		return -ENOTSUPP;
+		return -EINVAL;
 	};
 
 	ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
@@ -221,7 +221,7 @@ ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
 			       __func__, clk->id);
 			return -EINVAL;
 		}
-		return -ENOTSUPP;
+		return -EINVAL;
 	};
 
 	ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
@@ -311,7 +311,7 @@ int __imx8_clk_enable(struct clk *clk, bool enable)
 			       __func__, clk->id);
 			return -EINVAL;
 		}
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index feacaee1c42..b5cbf800543 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -290,7 +290,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		break;
 	default:
 		kfree(pll);
-		return ERR_PTR(-ENOTSUPP);
+		return ERR_PTR(-EINVAL);
 	}
 
 	pll->base = base;
diff --git a/drivers/clk/kendryte/bypass.c b/drivers/clk/kendryte/bypass.c
index 5f1986f2cb8..bbdbd9a10de 100644
--- a/drivers/clk/kendryte/bypass.c
+++ b/drivers/clk/kendryte/bypass.c
@@ -157,7 +157,7 @@ static int k210_bypass_set_parent(struct clk *clk, struct clk *parent)
 	if (ops->set_parent)
 		return ops->set_parent(bypass->bypassee, parent);
 	else
-		return -ENOTSUPP;
+		return -EINVAL;
 }
 
 /*
diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
index 4b959401a63..3b674a998e3 100644
--- a/drivers/clk/kendryte/clk.c
+++ b/drivers/clk/kendryte/clk.c
@@ -495,7 +495,7 @@ static int k210_clk_probe(struct udevice *dev)
 	 * could fix this, but it's Probably Not Worth It (TM).
 	 */
 	if (probed)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	base = dev_read_addr_ptr(dev_get_parent(dev));
 	if (!base)
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index 0132fcb7e61..b0f47c33b3f 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -340,7 +340,7 @@ static int periph_clk_enable(struct clk *clk, int enable)
 		return -EINVAL;
 
 	if (!periph_clk->can_gate)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	if (enable)
 		clrbits_le32(priv->reg + CLK_DIS, periph_clk->disable_bit);
@@ -408,7 +408,7 @@ static ulong armada_37xx_periph_clk_set_rate(struct clk *clk, ulong req_rate)
 		return old_rate;
 
 	if (!periph_clk->can_gate || !periph_clk->dividers)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	parent_rate = get_parent_rate(priv, clk->id);
 	if (parent_rate == -EINVAL)
@@ -445,7 +445,7 @@ static int armada_37xx_periph_clk_set_parent(struct clk *clk,
 		return -EINVAL;
 
 	if (!periph_clk->can_mux || !periph_clk->can_gate)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	ret = clk_get_by_index(clk->dev, 0, &check_parent);
 	if (ret < 0)
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 7/9] clk: Return -ENOSYS when system call is not available

[Simon Glass]

Update clk_composite_set_parent() to use -ENOSYS, which is the correct
error code for U-Boot. Also rearrange the code so that the error condition
is clearly indicated and the function runs to the end in the normal case,
since this is the common style in U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Sean Anderson <seanga2@gmail.com>
---

(no changes since v1)

 drivers/clk/clk-composite.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 7e99c5b910d..bb5351ebc0b 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -37,10 +37,10 @@ static int clk_composite_set_parent(struct clk *clk, struct clk *parent)
 	const struct clk_ops *mux_ops = composite->mux_ops;
 	struct clk *mux = composite->mux;
 
-	if (mux && mux_ops)
-		return mux_ops->set_parent(mux, parent);
-	else
-		return -ENOTSUPP;
+	if (!mux || !mux_ops)
+		return -ENOSYS;
+
+	return mux_ops->set_parent(mux, parent);
 }
 
 static unsigned long clk_composite_recalc_rate(struct clk *clk)
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 8/9] simple-pm-bus: Use -ENOSYS for checking missing system call

[Simon Glass]

We don't need to check -ENOTSUPP since this is not used for this purpose
in U-Boot. Update the code accordingly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/core/simple-pm-bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/core/simple-pm-bus.c b/drivers/core/simple-pm-bus.c
index 7a18953cba1..1bb0d86e289 100644
--- a/drivers/core/simple-pm-bus.c
+++ b/drivers/core/simple-pm-bus.c
@@ -21,7 +21,7 @@ static int simple_pm_bus_probe(struct udevice *dev)
 		return ret;
 
 	ret = clk_enable_bulk(bulk);
-	if (ret && ret != -ENOSYS && ret != -ENOTSUPP) {
+	if (ret && ret != -ENOSYS) {
 		clk_release_bulk(bulk);
 		return ret;
 	}
@@ -34,7 +34,7 @@ static int simple_pm_bus_remove(struct udevice *dev)
 	struct clk_bulk *bulk = dev_get_priv(dev);
 
 	ret = clk_release_bulk(bulk);
-	if (ret && ret != -ENOSYS && ret != -ENOTSUPP)
+	if (ret && ret != -ENOSYS)
 		return ret;
 	else
 		return 0;
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 9/9] pinctrl: Return -ENOSYS when system call is not available

[Simon Glass]

Update the code to use -ENOSYS, which is the correct error code for an
unimplemented system call in U-Boot.

Also we should not check for a missing operations array as this is not
permitted. For now this can be covered by an assert().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 drivers/pinctrl/pinctrl-uclass.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
index b0f30aa1f75..6e68e52c32c 100644
--- a/drivers/pinctrl/pinctrl-uclass.c
+++ b/drivers/pinctrl/pinctrl-uclass.c
@@ -235,8 +235,9 @@ int pinctrl_gpio_request(struct udevice *dev, unsigned offset)
 		return ret;
 
 	ops = pinctrl_get_ops(pctldev);
-	if (!ops || !ops->gpio_request_enable)
-		return -ENOTSUPP;
+	assert(ops);
+	if (!ops->gpio_request_enable)
+		return -ENOSYS;
 
 	return ops->gpio_request_enable(pctldev, pin_selector);
 }
@@ -261,8 +262,9 @@ int pinctrl_gpio_free(struct udevice *dev, unsigned offset)
 		return ret;
 
 	ops = pinctrl_get_ops(pctldev);
-	if (!ops || !ops->gpio_disable_free)
-		return -ENOTSUPP;
+	assert(ops);
+	if (!ops->gpio_disable_free)
+		return -ENOSYS;
 
 	return ops->gpio_disable_free(pctldev, pin_selector);
 }
-- 
2.31.0.rc2.261.g7f71774620-goog

[PATCH v2 6/9] clk: Update drivers to use -EINVAL

[Sean Anderson]

On 3/23/21 12:14 AM, Simon Glass wrote:
> At present some drivers use -ENOSUPP to indicate that an unknown or
> unsupported clock is used. Most use -EINVAL, indicating an invalid value,
> so convert everything to that.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add new patch to update clk drivers to use -EINVAL
> 
>   drivers/clk/aspeed/clk_ast2600.c       | 2 +-
>   drivers/clk/clk-hsdk-cgu.c             | 4 ++--
>   drivers/clk/imx/clk-imx8.c             | 4 ++--
>   drivers/clk/imx/clk-imx8qm.c           | 6 +++---
>   drivers/clk/imx/clk-imx8qxp.c          | 6 +++---
>   drivers/clk/imx/clk-pllv3.c            | 2 +-
>   drivers/clk/kendryte/bypass.c          | 2 +-
>   drivers/clk/kendryte/clk.c             | 2 +-
>   drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
>   9 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
> index acb7eca7414..3a92739f5cf 100644
> --- a/drivers/clk/aspeed/clk_ast2600.c
> +++ b/drivers/clk/aspeed/clk_ast2600.c
> @@ -1140,7 +1140,7 @@ int soc_clk_dump(void)
>   
>   		clk_free(&clk);
>   
> -		if (ret == -ENOTSUPP) {
> +		if (ret == -EINVAL) {
>   			printf("clk ID %lu not supported yet\n",
>   			       aspeed_clk_names[i].id);
>   			continue;
> diff --git a/drivers/clk/clk-hsdk-cgu.c b/drivers/clk/clk-hsdk-cgu.c
> index 449b430e230..26b0aa9a26f 100644
> --- a/drivers/clk/clk-hsdk-cgu.c
> +++ b/drivers/clk/clk-hsdk-cgu.c
> @@ -718,7 +718,7 @@ static ulong hsdk_cgu_set_rate(struct clk *sclk, ulong rate)
>   	if (clk->map[sclk->id].set_rate)
>   		return clk->map[sclk->id].set_rate(sclk, rate);
>   
> -	return -ENOTSUPP;
> +	return -EINVAL;
>   }
>   
>   static int hsdk_cgu_disable(struct clk *sclk)
> @@ -731,7 +731,7 @@ static int hsdk_cgu_disable(struct clk *sclk)
>   	if (clk->map[sclk->id].disable)
>   		return clk->map[sclk->id].disable(sclk);
>   
> -	return -ENOTSUPP;
> +	return -EINVAL;
>   }
>   
>   static const struct clk_ops hsdk_cgu_ops = {
> diff --git a/drivers/clk/imx/clk-imx8.c b/drivers/clk/imx/clk-imx8.c
> index 8484613eed5..b3dc138c4bb 100644
> --- a/drivers/clk/imx/clk-imx8.c
> +++ b/drivers/clk/imx/clk-imx8.c
> @@ -29,7 +29,7 @@ __weak ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
>   
>   __weak int __imx8_clk_enable(struct clk *clk, bool enable)
>   {
> -	return -ENOTSUPP;
> +	return -EINVAL;
>   }
>   
>   static int imx8_clk_disable(struct clk *clk)
> @@ -70,7 +70,7 @@ int soc_clk_dump(void)
>   
>   		clk_free(&clk);
>   
> -		if (ret == -ENOTSUPP) {
> +		if (ret == -EINVAL) {
>   			printf("clk ID %lu not supported yet\n",
>   			       imx8_clk_names[i].id);
>   			continue;
> diff --git a/drivers/clk/imx/clk-imx8qm.c b/drivers/clk/imx/clk-imx8qm.c
> index 7e466d630a0..7759dc63ee1 100644
> --- a/drivers/clk/imx/clk-imx8qm.c
> +++ b/drivers/clk/imx/clk-imx8qm.c
> @@ -133,7 +133,7 @@ ulong imx8_clk_get_rate(struct clk *clk)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
> @@ -237,7 +237,7 @@ ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
> @@ -337,7 +337,7 @@ int __imx8_clk_enable(struct clk *clk, bool enable)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	}
>   
>   	ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index e6b2fb40da2..ffa2fcee0b2 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -126,7 +126,7 @@ ulong imx8_clk_get_rate(struct clk *clk)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
> @@ -221,7 +221,7 @@ ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
> @@ -311,7 +311,7 @@ int __imx8_clk_enable(struct clk *clk, bool enable)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	}
>   
>   	ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
> diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> index feacaee1c42..b5cbf800543 100644
> --- a/drivers/clk/imx/clk-pllv3.c
> +++ b/drivers/clk/imx/clk-pllv3.c
> @@ -290,7 +290,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
>   		break;
>   	default:
>   		kfree(pll);
> -		return ERR_PTR(-ENOTSUPP);
> +		return ERR_PTR(-EINVAL);
>   	}
>   
>   	pll->base = base;
> diff --git a/drivers/clk/kendryte/bypass.c b/drivers/clk/kendryte/bypass.c
> index 5f1986f2cb8..bbdbd9a10de 100644
> --- a/drivers/clk/kendryte/bypass.c
> +++ b/drivers/clk/kendryte/bypass.c
> @@ -157,7 +157,7 @@ static int k210_bypass_set_parent(struct clk *clk, struct clk *parent)
>   	if (ops->set_parent)
>   		return ops->set_parent(bypass->bypassee, parent);
>   	else
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   }

For kendryte drivers:

Acked-by: Sean Anderson <seanga2@gmail.com>

>   /*
> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
> index 4b959401a63..3b674a998e3 100644
> --- a/drivers/clk/kendryte/clk.c
> +++ b/drivers/clk/kendryte/clk.c
> @@ -495,7 +495,7 @@ static int k210_clk_probe(struct udevice *dev)
>   	 * could fix this, but it's Probably Not Worth It (TM).
>   	 */
>   	if (probed)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	base = dev_read_addr_ptr(dev_get_parent(dev));
>   	if (!base)
> diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> index 0132fcb7e61..b0f47c33b3f 100644
> --- a/drivers/clk/mvebu/armada-37xx-periph.c
> +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> @@ -340,7 +340,7 @@ static int periph_clk_enable(struct clk *clk, int enable)
>   		return -EINVAL;
>   
>   	if (!periph_clk->can_gate)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	if (enable)
>   		clrbits_le32(priv->reg + CLK_DIS, periph_clk->disable_bit);
> @@ -408,7 +408,7 @@ static ulong armada_37xx_periph_clk_set_rate(struct clk *clk, ulong req_rate)
>   		return old_rate;
>   
>   	if (!periph_clk->can_gate || !periph_clk->dividers)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	parent_rate = get_parent_rate(priv, clk->id);
>   	if (parent_rate == -EINVAL)
> @@ -445,7 +445,7 @@ static int armada_37xx_periph_clk_set_parent(struct clk *clk,
>   		return -EINVAL;
>   
>   	if (!periph_clk->can_mux || !periph_clk->can_gate)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	ret = clk_get_by_index(clk->dev, 0, &check_parent);
>   	if (ret < 0)
> 

[PATCH v2 8/9] simple-pm-bus: Use -ENOSYS for checking missing system call

[Sean Anderson]

On 3/23/21 12:14 AM, Simon Glass wrote:
> We don't need to check -ENOTSUPP since this is not used for this purpose
> in U-Boot. Update the code accordingly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v1)
> 
>   drivers/core/simple-pm-bus.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/core/simple-pm-bus.c b/drivers/core/simple-pm-bus.c
> index 7a18953cba1..1bb0d86e289 100644
> --- a/drivers/core/simple-pm-bus.c
> +++ b/drivers/core/simple-pm-bus.c
> @@ -21,7 +21,7 @@ static int simple_pm_bus_probe(struct udevice *dev)
>   		return ret;
>   
>   	ret = clk_enable_bulk(bulk);
> -	if (ret && ret != -ENOSYS && ret != -ENOTSUPP) {
> +	if (ret && ret != -ENOSYS) {
>   		clk_release_bulk(bulk);
>   		return ret;
>   	}
> @@ -34,7 +34,7 @@ static int simple_pm_bus_remove(struct udevice *dev)
>   	struct clk_bulk *bulk = dev_get_priv(dev);
>   
>   	ret = clk_release_bulk(bulk);
> -	if (ret && ret != -ENOSYS && ret != -ENOTSUPP)
> +	if (ret && ret != -ENOSYS)
>   		return ret;
>   	else
>   		return 0;
> 

Reviewed-by: Sean Anderson <seanga2@gmail.com>

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

[Sean Anderson]

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@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?

> +
> +\-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.

> +
> +\-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?

> +
> +\-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?

> +
> +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?

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

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

--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
>   ---------------
>   
> 

[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@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@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
> >   ---------------
> >
> >
>

[PATCH v2 6/9] clk: Update drivers to use -EINVAL

[Stefan Roese]

On 23.03.21 05:14, Simon Glass wrote:
> At present some drivers use -ENOSUPP to indicate that an unknown or
> unsupported clock is used. Most use -EINVAL, indicating an invalid value,
> so convert everything to that.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
> Changes in v2:
> - Add new patch to update clk drivers to use -EINVAL
> 
>   drivers/clk/aspeed/clk_ast2600.c       | 2 +-
>   drivers/clk/clk-hsdk-cgu.c             | 4 ++--
>   drivers/clk/imx/clk-imx8.c             | 4 ++--
>   drivers/clk/imx/clk-imx8qm.c           | 6 +++---
>   drivers/clk/imx/clk-imx8qxp.c          | 6 +++---
>   drivers/clk/imx/clk-pllv3.c            | 2 +-
>   drivers/clk/kendryte/bypass.c          | 2 +-
>   drivers/clk/kendryte/clk.c             | 2 +-
>   drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
>   9 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
> index acb7eca7414..3a92739f5cf 100644
> --- a/drivers/clk/aspeed/clk_ast2600.c
> +++ b/drivers/clk/aspeed/clk_ast2600.c
> @@ -1140,7 +1140,7 @@ int soc_clk_dump(void)
>   
>   		clk_free(&clk);
>   
> -		if (ret == -ENOTSUPP) {
> +		if (ret == -EINVAL) {
>   			printf("clk ID %lu not supported yet\n",
>   			       aspeed_clk_names[i].id);
>   			continue;
> diff --git a/drivers/clk/clk-hsdk-cgu.c b/drivers/clk/clk-hsdk-cgu.c
> index 449b430e230..26b0aa9a26f 100644
> --- a/drivers/clk/clk-hsdk-cgu.c
> +++ b/drivers/clk/clk-hsdk-cgu.c
> @@ -718,7 +718,7 @@ static ulong hsdk_cgu_set_rate(struct clk *sclk, ulong rate)
>   	if (clk->map[sclk->id].set_rate)
>   		return clk->map[sclk->id].set_rate(sclk, rate);
>   
> -	return -ENOTSUPP;
> +	return -EINVAL;
>   }
>   
>   static int hsdk_cgu_disable(struct clk *sclk)
> @@ -731,7 +731,7 @@ static int hsdk_cgu_disable(struct clk *sclk)
>   	if (clk->map[sclk->id].disable)
>   		return clk->map[sclk->id].disable(sclk);
>   
> -	return -ENOTSUPP;
> +	return -EINVAL;
>   }
>   
>   static const struct clk_ops hsdk_cgu_ops = {
> diff --git a/drivers/clk/imx/clk-imx8.c b/drivers/clk/imx/clk-imx8.c
> index 8484613eed5..b3dc138c4bb 100644
> --- a/drivers/clk/imx/clk-imx8.c
> +++ b/drivers/clk/imx/clk-imx8.c
> @@ -29,7 +29,7 @@ __weak ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
>   
>   __weak int __imx8_clk_enable(struct clk *clk, bool enable)
>   {
> -	return -ENOTSUPP;
> +	return -EINVAL;
>   }
>   
>   static int imx8_clk_disable(struct clk *clk)
> @@ -70,7 +70,7 @@ int soc_clk_dump(void)
>   
>   		clk_free(&clk);
>   
> -		if (ret == -ENOTSUPP) {
> +		if (ret == -EINVAL) {
>   			printf("clk ID %lu not supported yet\n",
>   			       imx8_clk_names[i].id);
>   			continue;
> diff --git a/drivers/clk/imx/clk-imx8qm.c b/drivers/clk/imx/clk-imx8qm.c
> index 7e466d630a0..7759dc63ee1 100644
> --- a/drivers/clk/imx/clk-imx8qm.c
> +++ b/drivers/clk/imx/clk-imx8qm.c
> @@ -133,7 +133,7 @@ ulong imx8_clk_get_rate(struct clk *clk)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
> @@ -237,7 +237,7 @@ ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
> @@ -337,7 +337,7 @@ int __imx8_clk_enable(struct clk *clk, bool enable)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	}
>   
>   	ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index e6b2fb40da2..ffa2fcee0b2 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -126,7 +126,7 @@ ulong imx8_clk_get_rate(struct clk *clk)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
> @@ -221,7 +221,7 @@ ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	};
>   
>   	ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
> @@ -311,7 +311,7 @@ int __imx8_clk_enable(struct clk *clk, bool enable)
>   			       __func__, clk->id);
>   			return -EINVAL;
>   		}
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   	}
>   
>   	ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
> diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> index feacaee1c42..b5cbf800543 100644
> --- a/drivers/clk/imx/clk-pllv3.c
> +++ b/drivers/clk/imx/clk-pllv3.c
> @@ -290,7 +290,7 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
>   		break;
>   	default:
>   		kfree(pll);
> -		return ERR_PTR(-ENOTSUPP);
> +		return ERR_PTR(-EINVAL);
>   	}
>   
>   	pll->base = base;
> diff --git a/drivers/clk/kendryte/bypass.c b/drivers/clk/kendryte/bypass.c
> index 5f1986f2cb8..bbdbd9a10de 100644
> --- a/drivers/clk/kendryte/bypass.c
> +++ b/drivers/clk/kendryte/bypass.c
> @@ -157,7 +157,7 @@ static int k210_bypass_set_parent(struct clk *clk, struct clk *parent)
>   	if (ops->set_parent)
>   		return ops->set_parent(bypass->bypassee, parent);
>   	else
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   }
>   
>   /*
> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
> index 4b959401a63..3b674a998e3 100644
> --- a/drivers/clk/kendryte/clk.c
> +++ b/drivers/clk/kendryte/clk.c
> @@ -495,7 +495,7 @@ static int k210_clk_probe(struct udevice *dev)
>   	 * could fix this, but it's Probably Not Worth It (TM).
>   	 */
>   	if (probed)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	base = dev_read_addr_ptr(dev_get_parent(dev));
>   	if (!base)
> diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> index 0132fcb7e61..b0f47c33b3f 100644
> --- a/drivers/clk/mvebu/armada-37xx-periph.c
> +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> @@ -340,7 +340,7 @@ static int periph_clk_enable(struct clk *clk, int enable)
>   		return -EINVAL;
>   
>   	if (!periph_clk->can_gate)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	if (enable)
>   		clrbits_le32(priv->reg + CLK_DIS, periph_clk->disable_bit);
> @@ -408,7 +408,7 @@ static ulong armada_37xx_periph_clk_set_rate(struct clk *clk, ulong req_rate)
>   		return old_rate;
>   
>   	if (!periph_clk->can_gate || !periph_clk->dividers)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	parent_rate = get_parent_rate(priv, clk->id);
>   	if (parent_rate == -EINVAL)
> @@ -445,7 +445,7 @@ static int armada_37xx_periph_clk_set_parent(struct clk *clk,
>   		return -EINVAL;
>   
>   	if (!periph_clk->can_mux || !periph_clk->can_gate)
> -		return -ENOTSUPP;
> +		return -EINVAL;
>   
>   	ret = clk_get_by_index(clk->dev, 0, &check_parent);
>   	if (ret < 0)
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

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

[Sean Anderson]

On 3/23/21 1:40 AM, Simon Glass wrote:
> HI Sean,
> 
> On Tue, 23 Mar 2021 at 17:45, Sean Anderson <seanga2@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@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.

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
>>>    ---------------
>>>
>>>
>>

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

[Simon Glass]

Hi Sean,

On Thu, 25 Mar 2021 at 05:00, Sean Anderson <seanga2@gmail.com> wrote:
>
> On 3/23/21 1:40 AM, Simon Glass wrote:
> > HI Sean,
> >
> > On Tue, 23 Mar 2021 at 17:45, Sean Anderson <seanga2@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@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.
>
> 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.

OK I can say that.

>
> >>
> >>> +
> >>> +\-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).

The comment says 'communication error on send'.  It sounds like EREMOTEIO to me.

I can add a note about it.

Regards,
Simon