Re: [PATCH RFC v2 0/4] gpio: loongson-64bit: add interrupt support for more models

[Miao Wang]

Hi,

> 2026年10月15日 21:24，Bartosz Golaszewski <brgl@xxxxxxxxxx> 写道：
> 
> On Wed, 14 Oct 2026 18:20:49 +0200, Miao Wang <shankerwangmiao@xxxxxxxxx> said:
>> Hi,
>> 
>>> 2026年10月14日 23:33，Bartosz Golaszewski <brgl@xxxxxxxxxx> 写道：
>>> 
>>> On Thu, 8 Oct 2026 12:21:43 +0200, Miao Wang <shankerwangmiao@xxxxxxxxx> said:
>>>> Hi,
>>>> 
>>>>> 2026年10月8日 18:04，Bartosz Golaszewski <brgl@xxxxxxxxxx> 写道：
>>>>> 
>>>>> On Thu, 1 Oct 2026 20:13:18 +0200, Miao Wang via B4 Relay
>>>>> <devnull+shankerwangmiao.gmail.com@xxxxxxxxxx> said:
>>>>>> This patch series aims to correctly add interrupt support for GPIO
>>>>>> controllers found on more Loongson CPUs. The GPIO controllers in such
>>>>>> models do not have a complete intgrated interrupt controller, but choose
>>>>>> to delegate interrupt handling to their parent interrupt controller.
>>>>>> This sounds like a heirarchical interrupt setup, but what makes it
>>>>>> more complex is that multiple GPIO lines in these controllers may share
>>>>>> one interrupt line on the parent interrupt controller. The details is
>>>>>> described in the third patch.
>>>>>> 
>>>>>> Signed-off-by: Miao Wang <shankerwangmiao@xxxxxxxxx>
>>>>>> ---
>>>>> 
>>>>> The series looks pretty good. Who is the RFC addressed at in this case? I can't
>>>>> really answer the questions from patch 3/4 as I don't know this platform nor am
>>>>> I well versed in ACPI. Maybe Cc the GPIO ACPI maintainers for review?
>>>> 
>>>> Thanks for your review. The RFC contains two questions, the first is about
>>>> the firmware interface, including FDT and ACPI; the second is about the
>>>> way handling the delegated IRQs.
>>>> 
>>>> Regarding the firmware interface, it is to be decided how to declare
>>>> platform irq numbers for the GPIO controller. Such a GPIO controller
>>>> may have N gpio lines with M parent interrupt lines (M < N). The
>>>> firmware should a) list M irq numbers corresponding to the M parent
>>>> interrupt lines or b) list N irq numbers (which might be duplicated)
>>>> orresponding to the N gpio lines. Previously b) was chosen by the
>>>> kernel driver. However actual ACPI tables found in the published
>>>> firmware chose a). The discussion about it happened in [1].
>>>> 
>>> 
>>> Unless it impacts DT systems, I'll let Andy handle the ACPI part. Please Cc him
>>> next time as I pointed out under the RFC.
>> 
>> Will add him on the Cc list in further revisions.
>> 
>>>> Regarding to the interrupt handling delegation, since there is no
>>>> complete interrupt controller integrated with the GPIO controller,
>>>> the interrupt handling, including detecting edge triggered interrupts
>>>> and masking/unmasking, should be delegated to the parent IRQ controller.
>>>> I want to check if my implementation approach is acceptable and not
>>>> violating any implicit rules on the interrupt subsystem side.
>>>> 
>>> 
>>> I'd suggest to run your implementation by Thomas Gleixner because directly
>>> accessing the internals of the parent's irq data looks fishy. For instance:
>>> you call irq_set_type() directly without going through __setup_irq() ->
>>> __irq_set_trigger() which does some additional bookkeeping.
>> 
>> I would originally tend to avoid directly calling into ->irq_set_type() and
>> other handlers like this. However, the IRQ subsystem does not seem to provide
>> APIs to mask, unmask or set type of a given IRQ. Furthermore, __setup_irq()
>> is for registering irq_handler for an irq, while in chained irq chip, however,
>> irq_flow_handler is needed and should be registered via __irq_do_set_handler().
>> 
> 
> As I said: I'd involve Thomas in this discussion as it's his territory. I'm
> afraid I lack the deep knowledge of the interrupt subsystem internals to be
> able to help here and I'd also prefer to avoid adding any new abusers of the
> IRQ API to the kernel.

+Cc-ed Thomas

Cheers,

Miao Wang