Re: [PATCH v1 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs()

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Mon, Aug 02, 2021 at 04:40:21PM +0300, Serge Semin wrote:
> Hello Andy
> Thanks for the cleanup series. A tiny note is below.

Thanks for review!

> On Mon, Jul 26, 2021 at 03:54:33PM +0300, Andy Shevchenko wrote:
> > Shared IRQ is only enabled for ACPI enumeration, there is no need
> > to have a special flag for that, since we simple can test if device
> > has been enumerated by ACPI. This unifies the checks in dwapb_get_irq()
> > and dwapb_configure_irqs().
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/gpio/gpio-dwapb.c                | 13 ++++++-------
> >  drivers/mfd/intel_quark_i2c_gpio.c       |  1 -
> >  include/linux/platform_data/gpio-dwapb.h |  1 -
> >  3 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> > index 3eb13d6d31ef..f6ae69d5d644 100644
> > --- a/drivers/gpio/gpio-dwapb.c
> > +++ b/drivers/gpio/gpio-dwapb.c
> > @@ -436,12 +436,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
> >  	pirq->irqchip.irq_set_wake = dwapb_irq_set_wake;
> >  #endif
> >  
> 
> > -	if (!pp->irq_shared) {
> > -		girq->num_parents = pirq->nr_irqs;
> > -		girq->parents = pirq->irq;
> > -		girq->parent_handler_data = gpio;
> > -		girq->parent_handler = dwapb_irq_handler;
> > -	} else {
> > +	if (has_acpi_companion(gpio->dev)) {
> 
> Before this patch the platform flag irq_shared has been as kind of a
> hint regarding the shared IRQ case being covered here. But now it
> doesn't seem obvious why we've got the ACPI and ACPI-less cases
> differently handled. What about adding a small comment about that?
> E.g. like this: "Intel ACPI-based platforms mostly have the DW APB
> GPIO IRQ lane shared between several devices. In that case the
> parental IRQ has to be handled in the shared way so to be properly
> delivered to all the connected devices." or something more detailed
> for your preference. After that the rest of the comments in the
> if-clause could be discarded.

Sure!


-- 
With Best Regards,
Andy Shevchenko