[PATCH 15/18] serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)

Hugo Villeneuve posted 18 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH 15/18] serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)
Posted by Hugo Villeneuve 1 year, 11 months ago
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Fixes the following checkpatch warning:

    WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP

According to include/linux/errno.h, ENOTSUPP is
"Defined for the NFSv3 protocol", so replace it with preferred EOPNOTSUPP.

Similar to commit c7581a414d28 ("drm: Use EOPNOTSUPP, not ENOTSUPP").

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/max310x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index e39d8ea51e4e..12219b22b880 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1217,7 +1217,7 @@ static int max310x_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
 				1 << ((offset % 4) + 4), 0);
 		return 0;
 	default:
-		return -ENOTSUPP;
+		return -EOPNOTSUPP;
 	}
 }
 #endif
-- 
2.39.2
Re: [PATCH 15/18] serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)
Posted by Andy Shevchenko 1 year, 11 months ago
On Thu, Jan 18, 2024 at 12:39 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> Fixes the following checkpatch warning:
>
>     WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP

NAK.
It's a false positive.

> According to include/linux/errno.h, ENOTSUPP is
> "Defined for the NFSv3 protocol", so replace it with preferred EOPNOTSUPP.

The GPIO subsystem uses this internal error code internally. User
space won't get it, so users may not see this one.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 15/18] serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)
Posted by Hugo Villeneuve 1 year, 11 months ago
On Thu, 18 Jan 2024 01:24:11 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jan 18, 2024 at 12:39 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Fixes the following checkpatch warning:
> >
> >     WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP
> 
> NAK.
> It's a false positive.
> 
> > According to include/linux/errno.h, ENOTSUPP is
> > "Defined for the NFSv3 protocol", so replace it with preferred EOPNOTSUPP.
> 
> The GPIO subsystem uses this internal error code internally. User
> space won't get it, so users may not see this one.

Hi Andy,
I will drop the patch then.

What about adding a comment to prevent future fixes?

-               return -ENOTSUPP;
+               return -ENOTSUPP; /*
+                                  * ENOTSUPP is used for backward compatibility
+                                  * with GPIO subsystem.
+                                  */

Hugo Villeneuve
Re: [PATCH 15/18] serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)
Posted by Andy Shevchenko 1 year, 11 months ago
On Thu, Jan 18, 2024 at 1:59 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> On Thu, 18 Jan 2024 01:24:11 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jan 18, 2024 at 12:39 AM Hugo Villeneuve <hugo@hugovil.com> wrote:

...

> > > Fixes the following checkpatch warning:
> > >
> > >     WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP
> >
> > NAK.
> > It's a false positive.
> >
> > > According to include/linux/errno.h, ENOTSUPP is
> > > "Defined for the NFSv3 protocol", so replace it with preferred EOPNOTSUPP.
> >
> > The GPIO subsystem uses this internal error code internally. User
> > space won't get it, so users may not see this one.
>
> Hi Andy,
> I will drop the patch then.
>
> What about adding a comment to prevent future fixes?
>
> -               return -ENOTSUPP;
> +               return -ENOTSUPP; /*
> +                                  * ENOTSUPP is used for backward compatibility
> +                                  * with GPIO subsystem.
> +                                  */

It's kinda useless to add it to a single (GPIO) driver.
Rather it needs to be mentioned somewhere between
https://www.kernel.org/doc/html/latest/driver-api/gpio/index.html.

+Cc: Kent, Bart. It seems we have a handful of drivers violating this
(basically following what checkpatch says) and GPIO not documenting
this specific error code and its scope. Did I miss anything?


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 15/18] serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)
Posted by Kent Gibson 1 year, 11 months ago
On Thu, Jan 18, 2024 at 10:59:34AM +0200, Andy Shevchenko wrote:
> On Thu, Jan 18, 2024 at 1:59 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > On Thu, 18 Jan 2024 01:24:11 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Jan 18, 2024 at 12:39 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
>
> ...
>
> > > > Fixes the following checkpatch warning:
> > > >
> > > >     WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP
> > >
> > > NAK.
> > > It's a false positive.
> > >
> > > > According to include/linux/errno.h, ENOTSUPP is
> > > > "Defined for the NFSv3 protocol", so replace it with preferred EOPNOTSUPP.
> > >
> > > The GPIO subsystem uses this internal error code internally. User
> > > space won't get it, so users may not see this one.
> >
> > Hi Andy,
> > I will drop the patch then.
> >
> > What about adding a comment to prevent future fixes?
> >
> > -               return -ENOTSUPP;
> > +               return -ENOTSUPP; /*
> > +                                  * ENOTSUPP is used for backward compatibility
> > +                                  * with GPIO subsystem.
> > +                                  */
>
> It's kinda useless to add it to a single (GPIO) driver.
> Rather it needs to be mentioned somewhere between
> https://www.kernel.org/doc/html/latest/driver-api/gpio/index.html.
>
> +Cc: Kent, Bart. It seems we have a handful of drivers violating this
> (basically following what checkpatch says) and GPIO not documenting
> this specific error code and its scope. Did I miss anything?
>

You are correct - the GPIO subsystem is expecting ENOTSUPP if the config
is not supported.  In some cases it absorbs the failure or emulates the
feature instead (open drain/source, debounce). Returning EOPNOTSUPP
would be unfortunate, so checkpatch is not being helpful here.

And don't get me started on the gpio_chip interface contract being too
vague.

There are a handful of ways this could be addressed (documentation,
checkpatch, handle either, switch to EOPNOTSUPP, ... or some combination),
but making that call is definitely in Bart's court.

Cheers,
Kent.