Re: [PATCH v2 1/5] pwm: dwc: drop redundant error check

From: Andy Shevchenko
Date: Thu Feb 15 2024 - 14:26:08 EST


On Thu, Feb 15, 2024 at 06:20:15PM +0100, Uwe Kleine-König wrote:
> On Thu, Feb 15, 2024 at 03:36:12PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 15, 2024 at 10:22:57AM +0100, Uwe Kleine-König wrote:
> > > If a driver author knows it while writing the code, it's obvious. But if
> > > the driver author looks again in 2 years or someone else (e.g. me with
> > > the PWM maintainer hat on and with little pci experience) that knowledge
> > > might be faded.
> >
> > This is widely used pattern. Anybody who works with Git should know how
> > to use `git grep` tool. If in doubts, always can ask in the mailing lists.
>
> IMHO you're assuming to much. If someone sees this pattern and quickly
> looks at the implementation of pcim_iomap_table() they might (as I did)
> conclude that this call should be error checked. If they send a patch in
> say 2 years I think I won't remember this discussion/patch and happily
> accept this patch. And I probably won't get enough doubts to start
> grepping around.
>
> > I still consider it redundant.
> >
> > P.S. That's what you call "bikeshedding" (done by yourself here)?
>
> I can understand that you consider that bikeshedding given that for you
> it's obvious that the second function cannot fail. For me it's not and I
> take this as a hint that it's not obvious for everyone.

The bottom line that PCI devres code should be refactored. And IIRC somebody
is doing that job, not sure at which stage it is now.

--
With Best Regards,
Andy Shevchenko