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

On Tue, Dec 17 2024 at 12:15, Stefan Eichenberger wrote:

A regression was introduced in commit d929e4db22b6
("irqchip/irq-mvebu-icu: Prepare for real per device MSI") that causes
the Armada thermal driver to fail during probe with the following error:
genirq: Setting trigger mode 4 for irq 85 failed (irq_chip_set_type_parent+0x0/0x34)
armada_thermal f2400000.system-controller:thermal-sensor@70: Cannot request threaded IRQ 85
armada_thermal f2400000.system-controller:thermal-sensor@70: probe with driver armada_thermal failed with error -22

The issue occurs because irq_set_type is assigned to
irq_chip_set_type_parent, but the parent IRQ chip does not implement the
irq_set_type operation. This causes the trigger mode configuration to
fail.

This patch resolves the issue by removing the irq_set_type assignment.
With no irq_set_type, __irq_set_trigger safely skips the trigger
configuration, restoring functionality to the thermal driver.

I'm not convinced that this is correct.

The original code had irq_chip_set_type_parent() for the NSR/SEI chips
too and all what d929e4db22b6 does is to convert those chips over to the
new platform MSI mechanism. Here are the original chips:

static struct irq_chip mvebu_icu_nsr_chip = {
	.name			= "ICU-NSR",
        ...
	.irq_set_type		= irq_chip_set_type_parent,
        ...
};

static struct irq_chip mvebu_icu_sei_chip = {
	.name			= "ICU-SEI",
        ...
	.irq_set_type		= irq_chip_set_type_parent,
        ...
};

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.

Thanks,

        tglx