On Wed, Nov 10, 2021 at 10:21:36AM +0200, Sakari Ailus wrote:
> On Wed, Nov 10, 2021 at 10:18:20AM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 10, 2021 at 10:09:04AM +0200, Sakari Ailus wrote:
> > > On Tue, Nov 09, 2021 at 02:19:13PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Nov 09, 2021 at 01:19:34PM +0200, Sakari Ailus wrote:
...
> > > > > - } else if (is_acpi_device_node(fwnode)) {
> > > > > + }
> > > >
> > > > > + if (is_acpi_device_node(fwnode)) {
> > > >
> > > > Unneeded change. Yes I know that 'else' here can be skipped. But in such cases
> > > > it's a trade-off between changes, code readability and maintenance. Since here
> > > > it's a fix, backporting concerns are also play role.
> > >
> > > The patch applies cleanly to 5.5, the oldest kernel where it's needed.
> >
> > Why? I don't see how this affects the workflow.
> >
> > > Do you prefer another patch to remove the else clause?
> >
> > Nope.
> >
> > > I think it's a bit overkill...
> >
> > Exactly, that's why the question is why have you split the if-else-if to
> > two if:s?
>
> The else clause is useless, I think the code simply looks better without
> it.
I see a contradiction here:
Statement 1: 'else' is useless.
Statement 2: patch to remove it is overkill.
Either separate patch for that, or no need to touch this code, esp. taken into
consideration that this is a fix (subject to backport).
--
With Best Regards,
Andy Shevchenko
