[PATCH v2] gpio: crystalcove: Use irqchip template

[PATCH v2] gpio: crystalcove: Use irqchip template

[Linus Walleij]

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.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
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;
 }
-- 
2.26.2

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

[Andy Shevchenko]

On Tue, Jul 21, 2020 at 04:01:53PM +0200, 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.

Took this version instead.

I dunno if Hans can come with some comments / testing results, I would like to
send a PR today or tomorrow.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Jul 21, 2020 at 06:39:36PM +0300, Andy Shevchenko wrote:
> On Tue, Jul 21, 2020 at 04:01:53PM +0200, 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.
> 
> Took this version instead.
> 
> I dunno if Hans can come with some comments / testing results, I would like to
> send a PR today or tomorrow.

Same for wcove patch.

-- 
With Best Regards,
Andy Shevchenko

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

[Hans de Goede]

Hi,

On 7/21/20 5:39 PM, Andy Shevchenko wrote:
> On Tue, Jul 21, 2020 at 04:01:53PM +0200, 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.
> 
> Took this version instead.
> 
> I dunno if Hans can come with some comments / testing results, I would like to
> send a PR today or tomorrow.

Sorry for being a bit slow with testing this.

I've given this patch a test-run on a machine with the
PMIC the driver is for and with the caveat that I've not
actually tested the GPIO IRQ functionality, since that
does not seem to be used on any machines, I see no adverse
side effects from this patch, so it is:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

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

[Andy Shevchenko]

On Wed, Jul 22, 2020 at 11:56 AM Hans de Goede <hdegoede@redhat.com> wrote:
> On 7/21/20 5:39 PM, Andy Shevchenko wrote:
> > On Tue, Jul 21, 2020 at 04:01:53PM +0200, 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.
> >
> > Took this version instead.
> >
> > I dunno if Hans can come with some comments / testing results, I would like to
> > send a PR today or tomorrow.
>
> Sorry for being a bit slow with testing this.
>
> I've given this patch a test-run on a machine with the
> PMIC the driver is for and with the caveat that I've not
> actually tested the GPIO IRQ functionality, since that
> does not seem to be used on any machines, I see no adverse
> side effects from this patch, so it is:
>
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Thank you, Hans! It seems we also don't have such a reference design
which uses PMIC GPIOs for IRQ.

P.S. Does wcove case similar?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Wed, Jul 22, 2020 at 11:59 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jul 22, 2020 at 11:56 AM Hans de Goede <hdegoede@redhat.com> wrote:

> P.S. Does wcove case similar?

Never mind, I found the answer :-)

-- 
With Best Regards,
Andy Shevchenko

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

[Kuppuswamy, Sathyanarayanan]

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.
     Reviewed-by: Kuppuswamy Sathyanarayanan 
<sathyanarayanan.kuppuswamy@linux.intel.com>

> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> 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

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

[Andy Shevchenko]

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@linux.intel.com>

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@linux.intel.com>
> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > 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

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

[Kuppuswamy, Sathyanarayanan]

On 7/21/20 2:54 PM, Andy Shevchenko wrote:
> 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@linux.intel.com>
> 
> 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)
Ok. I will fix it in future emails.
> 
>>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>> ---
>>> 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
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer