On Tue, Mar 30, 2021 at 6:32 AM Matti Vaittinen
[off-list ref] wrote:

Morning Folks,

On Mon, 2021-03-29 at 16:30 +0300, Andy Shevchenko wrote:

quoted

On Mon, Mar 29, 2021 at 03:20:07PM +0300, Matti Vaittinen wrote:

quoted

On Mon, 2021-03-29 at 14:59 +0300, Andy Shevchenko wrote:

quoted

On Mon, Mar 29, 2021 at 2:43 PM Matti Vaittinen
[off-list ref] wrote:

quoted

The checkpacth instructs to switch from ENOSUPP to EOPNOTSUPP.

quoted

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

Make the gpiolib allow drivers to return both so driver
developers
can avoid one of the checkpatch complaints.

Internally we are fine to use the ENOTSUPP.
Checkpatch false positives there.

I agree. OTOH, the checkpatch check makes sense to user-visible
stuff.
Yet, the checkpatch has hard time guessing what is user-visible -
so it
probably is easiest to nag about all ENOTSUPP uses as it does now.

quoted

I doubt we need this change. Rather checkpatch should rephrase
this
to
point out that this is only applicable to _user-visible_ error
path.
Cc'ed Joe.

Yes, thanks for pulling Joe in.

Anyways, no matter what the warning says, all false positives are
annoying. I don't see why we should stay with ENOTSUPP in gpiolib?
(other than the burden of changing it).

For sake of the changing we are not changing the code.

No. But for the sake of making it better / more consistent :)

Anyway - after giving this second thought (thanks Andy for provoking me
to thinking this further) - I do agree with Andy that this particular
change is bad. More I think of this, less I like the idea of having two
separate return values to indicate the same thing. So we should support
only one which makes my patch terrible.

For the sake of consistency it would be cleaner to use same, single
value, for same error both inside the gpiolib and at user-interface.
That would be EOPNOTSUPP. As I said, having two separate error codes to
indicate same thing is confusing. Now the confusion is at the boundary
of gpiolib and user-land. Please educate me - is there difference in
the meaning of ENOTSUPP and EOPNOTSUPP or are they really indicating
the same thing? If yes, then yes - correct fix would be to use only one
and ditch the other. Whether the amount of work is such it is
practically worth is another topic - but that would be the right thing
to do (tm).

In user-space there's no ENOTSUPP but there's ENOTSUP which is defined
in most sane toolchains as:

#define ENOTSUP EOPNOTSUPP

While ENOTSUPP is not the same number as EOPNOTSUPP.

So to answer the question: they mean the same thing in the kernel but
not to user-space. We must never return ENOTSUPP to user-space because
it doesn't know it.

Bartosz

quoted

quoted

But I have no strong opinion on this. All options I see have
downsides.

Accepting both ENOTSUPP and EOPNOTSUPP is the easy way to avoid
allowing checkpatch warnings - but I admit it isn't stylish.

I think the error code which is Linux kernel internal is for a
reason.

If so, then the current checkpatch warning is even more questionable.

quoted

quoted

Converting all ENOTSUPP cases inside gpiolib to EOPNOTSUPP is
teodious
although end result might be nicer.

Why? You still missed the justification except satisfying some tool
that gives
you false positives. We don't do that. It's the tool that has to be
fixed /
amended.

quoted

Leaving it as is gives annoying false-positives to driver
developers.

My personal preference was this patch - others can have other view
like
Andy does. I'll leave this to community/maintainers to evaluate :)

This patch misses documentation fixes, for example.

Valid point.

quoted

Also, do you suggest that we have to do the same in entire pin
control
subsystem?

After reading/writing this, I am unsure. This is why the discussion is
good :) I don't see why we should have two separate error codes for
same thing - but as you put it:

quoted

I think the error code which is Linux kernel internal is for a
reason.

not all of us thinks the same. So maybe I just don't get it? :)

Best Regards
        Matti Vaittinen