From: Serge Semin <fancer.lancer@gmail.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Bartosz Golaszewski <bgolaszewski@baylibre.com>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH v2 13/14] gpio: dwapb: Use positive conditional in dwapb_configure_irqs()
Date: Fri, 17 Apr 2020 23:53:49 +0300 [thread overview]
Message-ID: <20200417205349.mv5y6tqwbxuoqnle@mobilestation> (raw)
In-Reply-To: <CACRpkdYCOHQ-e8sswUqv=exuKEasanWKUjPOjsvGDQC5sV4C4g@mail.gmail.com>
On Fri, Apr 17, 2020 at 12:42:33PM +0200, Linus Walleij wrote:
> On Thu, Apr 16, 2020 at 3:48 PM Serge Semin <fancer.lancer@gmail.com> 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@gmail.com> 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.
Ok. Thanks.
Regards,
-Sergey
>
> Yours,
> Linus Walleij
next prev parent reply other threads:[~2020-04-17 20:53 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-15 14:15 [PATCH v2 00/14] gpio: dwapb: Clean up the driver and a fix Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 01/14] gpio: dwapb: Append MODULE_ALIAS for platform driver Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 02/14] gpio: dwapb: Refactor IRQ handler to use bit operations Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 03/14] gpio: dwapb: Use chained IRQ prologue and epilogue Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 04/14] gpio: dwapb: set default handler to be handle_bad_irq() Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 05/14] gpio: dwapb: Deduplicate IRQ resource management Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 06/14] gpio: dwapb: Convert to use irqd_to_hwirq() Andy Shevchenko
2020-04-15 15:18 ` Serge Semin
2020-04-15 14:15 ` [PATCH v2 07/14] gpio: dwapb: Use device_get_match_data() to simplify code Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 08/14] gpio: dwapb: Convert to use IRQ core provided macros Andy Shevchenko
2020-04-15 17:53 ` Serge Semin
2020-04-16 10:39 ` Andy Shevchenko
2020-04-16 11:01 ` Serge Semin
2020-04-16 12:00 ` Jan Kiszka
2020-04-15 14:15 ` [PATCH v2 09/14] gpio: dwapb: Switch to more usual pattern of RMW in dwapb_gpio_set_debounce() Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 10/14] gpio: dwapb: Drop bogus BUG_ON()s Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 11/14] gpio: dwapb: Drop of_match_ptr() & ACPI_PTR() calls Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 12/14] gpio: dwapb: Split out dwapb_get_irq() helper Andy Shevchenko
2020-04-15 14:15 ` [PATCH v2 13/14] gpio: dwapb: Use positive conditional in dwapb_configure_irqs() Andy Shevchenko
2020-04-15 16:37 ` Serge Semin
2020-04-16 11:53 ` Linus Walleij
2020-04-16 13:48 ` Serge Semin
2020-04-17 10:42 ` Linus Walleij
2020-04-17 12:56 ` Andy Shevchenko
2020-04-17 20:53 ` Serge Semin [this message]
2020-04-15 14:15 ` [PATCH v2 14/14] gpio: dwapb: Amend indentation in some cases Andy Shevchenko
2020-04-15 17:15 ` Serge Semin
2020-04-16 10:56 ` Andy Shevchenko
2020-04-16 11:06 ` Serge Semin
2020-04-16 12:14 ` Linus Walleij
2020-04-16 13:37 ` Serge Semin
2020-04-16 13:59 ` Linus Walleij
2020-04-16 14:09 ` Andy Shevchenko
2020-04-16 7:26 ` [PATCH v2 00/14] gpio: dwapb: Clean up the driver and a fix Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200417205349.mv5y6tqwbxuoqnle@mobilestation \
--to=fancer.lancer@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bgolaszewski@baylibre.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).