Re: [PATCH v2 13/14] gpio: dwapb: Use positive conditional in dwapb_configure_irqs()

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Thu, Apr 16, 2020 at 01:53:25PM +0200, Linus Walleij wrote:
> On Wed, Apr 15, 2020 at 6:37 PM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> > On Wed, Apr 15, 2020 at 05:15:33PM +0300, Andy Shevchenko wrote:
> > > The negative conditionals are harder to parse by reader.
> > > Switch to positive one in dwapb_configure_irqs().
> >
> > Sorry as for me this modification is redundant. Yes, I know that if-else
> > statement in some cases better to start with positive expression to make it
> > a bit more clear, but in this case I'd leave it as is. First this rule is
> > applicable if both branches are more or less equal, but here I see the most
> > normal case of using the dt-based generic device, which doesn't declare the
> > IRQs as shared seeing it is selected by far more devices at the moment.
> > Second the non-shared IRQs case also covers a combined and multiple-lined
> > GPIO IRQs (chained cascaded GPIO irqchip), while the irq_shared clause have
> > only a single IRQ source supported. Finally If the code was like you
> > suggested from the very beginning I wouldn't say a word, but this patch seems
> > to me at least just moving the code around with gaining less than we have at
> > the moment.
> >
> > Linus, Bartosz and other GPIO-ers may think differently though. Lets see their
> > opinion.
> 
> I think I already applied all patches with the batch application tool b4,
> without properly checking which patches you reviewed and not, sorry :(
> 
> However if any change is controversial I can revert or pull the patch out.

In this case it's up to you to decide. I was against this patch in the first
place. As for me it seemed redundant (see my justification above). But you or
Bartosz may think differently that's why I asked your opinion to decide its
destiny. So if you are ok with what the patch provides, then there is no need
to revert or reset anything. But if you agree with me, then pulling the patch
out with resetting the git history will be the best option.

Regards,
-Sergey

> 
> Yours,
> Linus Walleij