Re: [PATCH] pwm: meson: Simplify using dev_err_probe()

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3038 bytes --]

Hello,

On Mon, Dec 11, 2023 at 12:01:33PM +0100, Thierry Reding wrote:
> On Fri, Dec 08, 2023 at 08:06:20PM +0100, Uwe Kleine-König wrote:
> > On Fri, Dec 08, 2023 at 04:52:57PM +0100, Thierry Reding wrote:
> > > This is a lot of churn for very little gain.
> > 
> > We seem to have different conceptions of churn. Each hunk here is an
> > improvement for both SLOC count and usefulness of the generated error
> > message.
> > 
> > 	failed to register somename: -5
> > 
> > is worse than
> > 
> > 	error EIO: failed to register somename
> > 
> > , isn't it?
> 
> That's entirely subjective.

It's not. You and me both know that -5 is EIO. But there are quite a few
people who don't. And for other error codes I'm not that fluent. (Do you
know -2 and -19?) Also some constants are architecture specific, so e.g.
-11 is -35 on alpha.

> I think the first version is just fine. I,
> and I suspect most developers will, know what to do with either of those
> error messages.

Error messages aren't only for (kernel) developers. If you don't know
that the kernel uses negative error numbers as return codes, the
translation of -5 to EIO is even further away than opening
/usr/include/errno.h.
 
> > > None of these functions are ever going to return -EPROBE_DEFER. And
> > > yes, I know that function's doc says that it is "deemed acceptable to
> > > use" elsewhere. However, the existing error messages are just fine, no
> > > need to churn just for the sake of it.
> > 
> > We had this disagreement already before. Yes dev_err_probe() is useful
> > for three reasons and this driver only benefits from two of these.
> > That's IMHO still one reason more than needed to justify such a change.
> 
> I disagree. There are certainly cases where dev_err_probe() can be a
> significant improvement, but there are others where the improvement is
> very minor (if there's any at all) and in my opinion the churn isn't
> justified.

What is churn for you? Many changes? Big changes? For me churn is only a
big amount of changes where a considerable part cancels out if it was
squashed together. For you this concept seems to be broader.

> Otherwise we'll just forever keep rewriting the same code
> over and over again because somebody comes up with yet another variant
> of mostly the same code.

If there is an improvement in each adaption that's fine for me.
 
> > And if you think that a function should only be used if all advantages
> > are useful for the caller, let us reconsider if we really need capture
> > support in the pwm framework as only two of the 68 drivers make use of
> > it.
> 
> That's a ridiculous argument and you know it. You are comparing apples
> to oranges.

I would have been surprised if it had convinced you, but I honestly
think there is a (admittedly small) point.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic