* [PATCH 1/6] usb: Return -ENOSYS when system call is not available
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
@ 2021-03-21 3:18 ` Simon Glass
2021-03-21 4:09 ` Marek Vasut
2021-03-21 3:18 ` [PATCH 2/6] spi: " Simon Glass
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-03-21 3:18 UTC (permalink / raw)
To: u-boot
Update usb_gadget_release() to use -ENOSYS, which is the correct error
code for U-Boot.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 1/6] usb: Return -ENOSYS when system call is not available
2021-03-21 3:18 ` [PATCH 1/6] usb: Return -ENOSYS when system call is not available Simon Glass
@ 2021-03-21 4:09 ` Marek Vasut
2021-03-21 4:12 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2021-03-21 4:09 UTC (permalink / raw)
To: u-boot
On 3/21/21 4:18 AM, Simon Glass wrote:
> Update usb_gadget_release() to use -ENOSYS, which is the correct error
> code for U-Boot.
It would be good if you could explain _why_ this change is required. Why
is ENOSYS the correct return value ? It is not clear from the patch
description, sorry.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] usb: Return -ENOSYS when system call is not available
2021-03-21 4:09 ` Marek Vasut
@ 2021-03-21 4:12 ` Simon Glass
2021-03-21 18:29 ` Marek Vasut
0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-03-21 4:12 UTC (permalink / raw)
To: u-boot
Hi Marek,
On Sun, 21 Mar 2021 at 17:09, Marek Vasut <marex@denx.de> wrote:
>
> On 3/21/21 4:18 AM, Simon Glass wrote:
> > Update usb_gadget_release() to use -ENOSYS, which is the correct error
> > code for U-Boot.
>
> It would be good if you could explain _why_ this change is required. Why
> is ENOSYS the correct return value ? It is not clear from the patch
> description, sorry.
That is the value used throughout driver model, so it is a convention.
As per the cover letter and one on the patches, people are not sure
which one to use, because -ENOTSUPP has crept in in a few places.
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] usb: Return -ENOSYS when system call is not available
2021-03-21 4:12 ` Simon Glass
@ 2021-03-21 18:29 ` Marek Vasut
0 siblings, 0 replies; 14+ messages in thread
From: Marek Vasut @ 2021-03-21 18:29 UTC (permalink / raw)
To: u-boot
On 3/21/21 5:12 AM, Simon Glass wrote:
> Hi Marek,
>
> On Sun, 21 Mar 2021 at 17:09, Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/21/21 4:18 AM, Simon Glass wrote:
>>> Update usb_gadget_release() to use -ENOSYS, which is the correct error
>>> code for U-Boot.
>>
>> It would be good if you could explain _why_ this change is required. Why
>> is ENOSYS the correct return value ? It is not clear from the patch
>> description, sorry.
>
> That is the value used throughout driver model, so it is a convention.
> As per the cover letter and one on the patches, people are not sure
> which one to use, because -ENOTSUPP has crept in in a few places.
I agree with Tom, this needs to be documented first, then the document
should be agreed upon, and only then should these fixes follow.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] spi: Return -ENOSYS when system call is not available
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
2021-03-21 3:18 ` [PATCH 1/6] usb: Return -ENOSYS when system call is not available Simon Glass
@ 2021-03-21 3:18 ` Simon Glass
2021-03-21 3:18 ` [PATCH 3/6] tlv_eeprom: " Simon Glass
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-03-21 3:18 UTC (permalink / raw)
To: u-boot
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>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/6] tlv_eeprom: Return -ENOSYS when system call is not available
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
2021-03-21 3:18 ` [PATCH 1/6] usb: Return -ENOSYS when system call is not available Simon Glass
2021-03-21 3:18 ` [PATCH 2/6] spi: " Simon Glass
@ 2021-03-21 3:18 ` Simon Glass
2021-03-21 3:18 ` [PATCH 4/6] clk: " Simon Glass
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-03-21 3:18 UTC (permalink / raw)
To: u-boot
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>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/6] clk: Return -ENOSYS when system call is not available
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
` (2 preceding siblings ...)
2021-03-21 3:18 ` [PATCH 3/6] tlv_eeprom: " Simon Glass
@ 2021-03-21 3:18 ` Simon Glass
2021-03-21 4:37 ` Sean Anderson
2021-03-21 3:18 ` [PATCH 5/6] simple-pm-bus: Use -ENOSYS for checking missing system call Simon Glass
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-03-21 3:18 UTC (permalink / raw)
To: u-boot
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>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/6] clk: Return -ENOSYS when system call is not available
2021-03-21 3:18 ` [PATCH 4/6] clk: " Simon Glass
@ 2021-03-21 4:37 ` Sean Anderson
2021-03-23 4:08 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2021-03-21 4:37 UTC (permalink / raw)
To: u-boot
On 3/20/21 11:18 PM, Simon Glass wrote:
> 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>
> ---
>
> 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)
>
Reviewed-by: Sean Anderson <seanga2@gmail.com>
Will this be applied to the whole clock subsystem? From what I can tell,
the clock subsystem returns ENOSYS in these situations, and drivers
return ENOTSUPP. It would be great if we could unified all these so
callers could check for one return value.
--Sean
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/6] clk: Return -ENOSYS when system call is not available
2021-03-21 4:37 ` Sean Anderson
@ 2021-03-23 4:08 ` Simon Glass
0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-03-23 4:08 UTC (permalink / raw)
To: u-boot
Hi Seam,
On Sun, 21 Mar 2021 at 17:37, Sean Anderson <seanga2@gmail.com> wrote:
>
>
> On 3/20/21 11:18 PM, Simon Glass wrote:
> > 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>
> > ---
> >
> > 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)
> >
>
> Reviewed-by: Sean Anderson <seanga2@gmail.com>
>
> Will this be applied to the whole clock subsystem? From what I can tell,
> the clock subsystem returns ENOSYS in these situations, and drivers
> return ENOTSUPP. It would be great if we could unified all these so
> callers could check for one return value.
Most drivers use -EINVAL for an invalid / unknown clock, whereas
-ENOSYS is for an unimplemented method or sub-method.
So I'll add a patch to use -EINVAL consistently.
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/6] simple-pm-bus: Use -ENOSYS for checking missing system call
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
` (3 preceding siblings ...)
2021-03-21 3:18 ` [PATCH 4/6] clk: " Simon Glass
@ 2021-03-21 3:18 ` Simon Glass
2021-03-21 4:39 ` Sean Anderson
2021-03-21 3:18 ` [PATCH 6/6] pinctrl: Return -ENOSYS when system call is not available Simon Glass
2021-03-21 16:38 ` [PATCH 0/6] Use -ENOSYS consistently Tom Rini
6 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-03-21 3:18 UTC (permalink / raw)
To: u-boot
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>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6] simple-pm-bus: Use -ENOSYS for checking missing system call
2021-03-21 3:18 ` [PATCH 5/6] simple-pm-bus: Use -ENOSYS for checking missing system call Simon Glass
@ 2021-03-21 4:39 ` Sean Anderson
0 siblings, 0 replies; 14+ messages in thread
From: Sean Anderson @ 2021-03-21 4:39 UTC (permalink / raw)
To: u-boot
On 3/20/21 11:18 PM, 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>
> ---
>
> 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;
>
Nak. This cannot be removed until all drivers have been converted to use
-ENOSYS.
--Sean
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/6] pinctrl: Return -ENOSYS when system call is not available
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
` (4 preceding siblings ...)
2021-03-21 3:18 ` [PATCH 5/6] simple-pm-bus: Use -ENOSYS for checking missing system call Simon Glass
@ 2021-03-21 3:18 ` Simon Glass
2021-03-21 16:38 ` [PATCH 0/6] Use -ENOSYS consistently Tom Rini
6 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-03-21 3:18 UTC (permalink / raw)
To: u-boot
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>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 0/6] Use -ENOSYS consistently
2021-03-21 3:18 [PATCH 0/6] Use -ENOSYS consistently Simon Glass
` (5 preceding siblings ...)
2021-03-21 3:18 ` [PATCH 6/6] pinctrl: Return -ENOSYS when system call is not available Simon Glass
@ 2021-03-21 16:38 ` Tom Rini
6 siblings, 0 replies; 14+ messages in thread
From: Tom Rini @ 2021-03-21 16:38 UTC (permalink / raw)
To: u-boot
On Sun, Mar 21, 2021 at 04:18:18PM +1300, Simon Glass wrote:
> A few places use -ENOTSUPP when they should use -ENOSYS. In two cases both
> are used. This little series tidies this up.
>
>
> Simon Glass (6):
> 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: 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
>
> drivers/clk/clk-composite.c | 8 ++++----
> 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 +++---
> 6 files changed, 17 insertions(+), 15 deletions(-)
I think we need to start this series with a doc update that makes it
clear why this is correct and when it should be used. There's two
instances of "ENOSYS" in doc/driver-model/ today and neither explains
why this series is correct (which I'm sure it is). Thanks.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210321/f063b944/attachment.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread