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

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Thu, Apr 16, 2020 at 3:48 PM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> 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 backed out the last two patches now and kept the rest except 13 and 14.

Yours,
Linus Walleij