Re: [PATCH v2 3/3] pinctrl: cherryview: Pass irqchip when adding gpiochip

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

Hi,

On 06-11-2019 17:17, Andy Shevchenko wrote:
> On Wed, Nov 06, 2019 at 06:16:22PM +0200, Andy Shevchenko wrote:
>> On Wed, Nov 06, 2019 at 04:47:15PM +0100, Hans de Goede wrote:
>>> We need to convert all old gpio irqchips to pass the irqchip
>>> setup along when adding the gpio_chip. For more info see
>>> drivers/gpio/TODO.
>>>
>>> For chained irqchips this is a pretty straight-forward conversion.
>>
>>> +	chip->irq.chip = &pctrl->irqchip;
>>
>>> +	if (pctrl->need_valid_mask)
>>> +		chip->irq.init_valid_mask = chv_init_irq_valid_mask;
>>
>> I just realize we probably may assign here unconditionally
> 
> Continuing...
> 
>>
>>> +	chip->irq.init_hw = chv_gpio_irq_init_hw;
>>> +	chip->irq.parent_handler = chv_gpio_irq_handler;
>>> +	chip->irq.num_parents = 1;
>>> +	chip->irq.parents = &pctrl->irq;
>>> +	chip->irq.default_type = IRQ_TYPE_NONE;
>>> +	chip->irq.handler = handle_bad_irq;
>>>   
>>>   	if (!pctrl->need_valid_mask) {
> 
> And here turn it back to NULL and check the pointer against NULL instead of
> additional variable.
> 
> What do you think?

I think that first setting it and then clearing it again is not
very pretty. But ...

I do think you are on to something, we can use pctrl->chip.irq.init_valid_mask
instead of storing the dmi quirk in the chv_pinctrl struct.

Then we can leave the dmi handling as is, and replace later checks for
the dmi quirk (in callbacks) with a check for pctrl->chip.irq.init_valid_mask
I do believe that that is better then adding a need_validmask member to
the chv_pinctrl struct, I will prepare a v3 of the series with this change.

Regards,

Hans