Re: [PATCH v2] gpio: crystalcove: Use irqchip template

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Jul 21, 2020 at 10:08:57AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> Hi,
> 
> On 7/21/20 7:01 AM, Linus Walleij wrote:
> > This makes the driver use the irqchip template to assign
> > properties to the gpio_irq_chip instead of using the
> > explicit calls to gpiochip_irqchip_add_nested() and
> > gpiochip_set_nested_irqchip(). The irqchip is instead
> > added while adding the gpiochip.
> Looks good to me.

Thanks!

>     Reviewed-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>

It's not first time your tag goes like this. Please, fix your tools to be it like
Reviewed-by: Name <address@com>
(no leading spaces, no split -- one line)

> > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> > Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > ---
> > ChangeLog v1->V2:
> > - Fixed a variable name ch->cg
> > ---
> >   drivers/gpio/gpio-crystalcove.c | 24 +++++++++++++++---------
> >   1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c
> > index 14d1f4c933b6..39349b0e6923 100644
> > --- a/drivers/gpio/gpio-crystalcove.c
> > +++ b/drivers/gpio/gpio-crystalcove.c
> > @@ -330,6 +330,7 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
> >   	int retval;
> >   	struct device *dev = pdev->dev.parent;
> >   	struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> > +	struct gpio_irq_chip *girq;
> >   	if (irq < 0)
> >   		return irq;
> > @@ -353,14 +354,15 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
> >   	cg->chip.dbg_show = crystalcove_gpio_dbg_show;
> >   	cg->regmap = pmic->regmap;
> > -	retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
> > -	if (retval) {
> > -		dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
> > -		return retval;
> > -	}
> > -
> > -	gpiochip_irqchip_add_nested(&cg->chip, &crystalcove_irqchip, 0,
> > -				    handle_simple_irq, IRQ_TYPE_NONE);
> > +	girq = &cg->chip.irq;
> > +	girq->chip = &crystalcove_irqchip;
> > +	/* This will let us handle the parent IRQ in the driver */
> > +	girq->parent_handler = NULL;
> > +	girq->num_parents = 0;
> > +	girq->parents = NULL;
> > +	girq->default_type = IRQ_TYPE_NONE;
> > +	girq->handler = handle_simple_irq;
> > +	girq->threaded = true;
> >   	retval = request_threaded_irq(irq, NULL, crystalcove_gpio_irq_handler,
> >   				      IRQF_ONESHOT, KBUILD_MODNAME, cg);
> > @@ -370,7 +372,11 @@ static int crystalcove_gpio_probe(struct platform_device *pdev)
> >   		return retval;
> >   	}
> > -	gpiochip_set_nested_irqchip(&cg->chip, &crystalcove_irqchip, irq);
> > +	retval = devm_gpiochip_add_data(&pdev->dev, &cg->chip, cg);
> > +	if (retval) {
> > +		dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval);
> > +		return retval;
> > +	}
> >   	return 0;
> >   }
> > 
> 
> -- 
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer

-- 
With Best Regards,
Andy Shevchenko