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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 7/17/20 3:59 PM, Hans de Goede wrote:
> Hi,
> 
> On 7/17/20 1:25 PM, 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.
>>
>> 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>
>> ---
>> Intel folks and Hans: I hope someone can test this, I'm
>> a bit uncertain if IRQs could fire before registering
>> the chip and if we need a hw_init() in this driver to cope.
> 
> I've added this to my personal tree for testing. I will get back
> to you when I've either hit an issue, or used it for a while without
> issues :)
> 
> Hmm, testing this might be tricky, I don't think any boards
> actually use any GPIOs on the PMIC (which this driver is for)
> as interrupts...
> 
> So the best I can do is boot a machine and test there are no
> regressions I guess.

Erm, it does not even compile:

drivers/gpio/gpio-crystalcove.c: In function ‘crystalcove_gpio_probe’:
drivers/gpio/gpio-crystalcove.c:357:10: error: ‘ch’ undeclared (first use in this function); did you mean ‘cg’?
   357 |  girq = &ch->chip.irq;
       |          ^~
       |          cg
drivers/gpio/gpio-crystalcove.c:357:10: note: each undeclared identifier is reported only once for each function it appears in

I've fixed this up locally.

Regards,

Hans


>> +    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;
>>   }
>>