diff mbox series

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

Message ID 20240117223856.2303475-16-hugo@hugovil.com
State New
Headers
Series serial: max310x: cleanups and improvements |

Commit Message

Hugo Villeneuve Jan. 17, 2024, 10:38 p.m. UTC 
  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(-)

Comments

Andy Shevchenko Jan. 17, 2024, 11:24 p.m. UTC | #1 
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.
Hugo Villeneuve Jan. 17, 2024, 11:59 p.m. UTC | #2 
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
Andy Shevchenko Jan. 18, 2024, 8:59 a.m. UTC | #3 
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?
Kent Gibson Jan. 18, 2024, 1:15 p.m. UTC | #4 
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.
diff mbox series

Patch

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