Re: [PATCH] gpio: tegra186: Don't set parent IRQ affinity

[Jon Hunter <jonathanh@xxxxxxxxxx>]

Hi Linus,

On 07/05/2021 11:41, Linus Walleij wrote:
> Hi Jon,
> 
> On Fri, May 7, 2021 at 12:34 PM Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
> 
>> When hotplugging CPUs on Tegra186 and Tegra194 errors such as the
>> following are seen ...
>>
>>  IRQ63: set affinity failed(-22).
>>  IRQ65: set affinity failed(-22).
>>  IRQ66: set affinity failed(-22).
>>  IRQ67: set affinity failed(-22).
>>
>> Looking at the /proc/interrupts the above are all interrupts associated
>> with GPIOs. The reason why these error messages occur is because there
>> is no 'parent_data' associated with any of the GPIO interrupts and so
>> tegra186_irq_set_affinity() simply returns -EINVAL.
>>
>> To understand why there is no 'parent_data' it is first necessary to
>> understand that in addition to the GPIO interrupts being routed to the
>> interrupt controller (GIC), the interrupts for some GPIOs are also
>> routed to the Tegra Power Management Controller (PMC) to wake up the
>> system from low power states. In order to configure GPIO events as
>> wake events in the PMC, the PMC is configured as IRQ parent domain
>> for the GPIO IRQ domain. Originally the GIC was the IRQ parent domain
>> of the PMC and although this was working, this started causing issues
>> once commit 64a267e9a41c ("irqchip/gic: Configure SGIs as standard
>> interrupts") was added, because technically, the GIC is not a parent
>> of the PMC. Commit c351ab7bf2a5 ("soc/tegra: pmc: Don't create fake
>> interrupt hierarchy levels") fixed this by severing the IRQ domain
>> hierarchy for the Tegra GPIOs and hence, there may be no IRQ parent
>> domain for the GPIOs.
>>
>> The GPIO controllers on Tegra186 and Tegra194 have either one or six
>> interrupt lines to the interrupt controller. For GPIO controllers with
>> six interrupts, the mapping of the GPIO interrupt to the controller
>> interrupt is configurable within the GPIO controller. Currently a
>> default mapping is used, however, it could be possible to use the
>> set affinity callback for the Tegra186 GPIO driver to do something a
>> bit more interesting. Currently, because interrupts for all GPIOs are
>> have the same mapping and any attempts to configure the affinity for
>> a given GPIO can conflict with another that shares the same IRQ, for
>> now it is simpler to just remove set affinity support and this avoids
>> the above warnings being seen.
>>
>> Cc: <stable@xxxxxxxxxxxxxxx>
>> Fixes: c4e1f7d92cd6 ("gpio: tegra186: Set affinity callback to parent")
>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> 
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> and sorry for the mess.

No worries, your change made complete sense in principle.

> I don't know if it would be possible to take some inspiration from
> the Qualcomm pin control driver:
> drivers/pinctrl/qcom/pinctrl-msm.c
> 
> This has quite elaborate handling of this especially marking the
> lines that can be used for sleeping and IIUC are reparented to
> the PDC (power domain controller) that Qcom is using and
> which I guess is similar to your PMC.


Good to know. I am was wondering how others handle this sort of thing.

Thanks!
Jon

-- 
nvpublic