Re: [PATCH v1] irqchip/irq-mvebu-icu: Fix irq_set_type for sei and nsr

[Thomas Gleixner <tglx@linutronix.de>]

Hi!

On Thu, Jan 16 2025 at 18:12, Stefan Eichenberger wrote:
> On Wed, Jan 15, 2025 at 09:15:24AM +0100, Thomas Gleixner wrote:
>> And looking at the potential platform MSI providers for MVEBU, then it
>> turns out that GICP and SEI both have the irq_set_type() callback
>> populated, though ODMI has not. So either this has never worked or there
>> is something else fishy.
>> 
>> Can you please enable CONFIG_GENERIC_IRQ_DEBUGFS, build/boot a 6.10
>> kernel and provide the output of
>> 
>> cat /sys/kernel/debug/irq/irq/$N
>> 
>> where $N is the interrupt number of the thermal sensor.
>> 
>> Then provide the same information for a current kernel with your patch
>> applied.
>
> You are right I somehow didn't look back far enough. I tested once with
> kernel 6.12.5 and my patch applied:

> root@localhost:~# uname -a
> Linux localhost.localdomain 6.12.5+ #157 SMP PREEMPT Thu Jan 16 17:32:21 CET 2025 aarch64 GNU/Linux
> root@localhost:~# cat /proc/interrupts |grep thermal
>  35:          0          0          0          0    AP SEI  18 Level     f06f8000.system-controller:thermal-sensor@80
>  90:          0          0          0          0  SEI-ICU-SEI-f21e0000.interrupt-controller:inter 116 Edge      f2400000.system-controller:thermal-sensor@70
>  91:          0          0          0          0  SEI-ICU-SEI-f61e0000.interrupt-controller:inter 116 Edge      f6400000.system-controller:thermal-sensor@70
> root@localhost:~# cat /sys/kernel/debug/irq/irqs/90
> handler:  handle_edge_irq
> device:   f21e0000.interrupt-controller:interrupt-controller@50
> status:   0x00000000
> istate:   0x00004000
> ddepth:   0
> wdepth:   0
> dstate:   0x02400204
>             IRQ_TYPE_LEVEL_HIGH
>             IRQD_ACTIVATED
>             IRQD_IRQ_STARTED
>             IRQD_DEFAULT_TRIGGER_SET
> node:     -1
> affinity: 0-3
> effectiv:
> domain:  :cp0:config-space@f2000000:interrupt-controller@1e0000:interrupt-controller@50-16
>  hwirq:   0x74
>  chip:    SEI-ICU-SEI-f21e0000.interrupt-controller:inter
>   flags:   0x80
>              IRQCHIP_SUPPORTS_LEVEL_MSI
>  parent:
>     domain:  :ap807:config-space@f0000000:interrupt-controller@3f0200-2
>      hwirq:   0x0
>      chip:    CP SEI

Ok. That explains it. CP SEI has:

static int mvebu_sei_cp_set_type(struct irq_data *data, unsigned int type)
{
        if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_EDGE_RISING)
                return -EINVAL;
        return 0;
}                                                                        

But the DT configuration requests:

>             IRQ_TYPE_LEVEL_HIGH

  IRQ_TYPE_LEVEL_HIGH     = 0x00000004,

>  genirq: Setting trigger mode 4 for irq 85 failed (irq_chip_set_type_parent+0x0/0x34)

which is not supported by CP SEI.

With your patch the type request is just stored in the data, but no
actual type setting happens. That's default behaviour for chips which do
not have a set_type() callback.

> root@localhost:~# cat /sys/kernel/debug/irq/irqs/90
> handler:  handle_edge_irq
> device:   (null)
>
> It seems with kernel 6.10 the controller device was not set correctly,
> probably it was ignoring irq_set_type because of this. 

No. That's not related.

> Do you by chance have an idea how to properly fix this or should I do
> some more research?

It's unclear to me how the 6.10 kernel survives that. Do you have a
different device tree for those kernels?

Thanks,

        tglx