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

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

[Stefan Eichenberger]

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.

Fixes: d929e4db22b6 ("irqchip/irq-mvebu-icu: Prepare for real per device MSI")
Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
 drivers/irqchip/irq-mvebu-icu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
index b337f6c05f18..1d4dde719db6 100644
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -188,7 +188,6 @@ static const struct msi_domain_template mvebu_icu_nsr_msi_template = {
 		.irq_mask		= irq_chip_mask_parent,
 		.irq_unmask		= irq_chip_unmask_parent,
 		.irq_eoi		= irq_chip_eoi_parent,
-		.irq_set_type		= irq_chip_set_type_parent,
 		.irq_write_msi_msg	= mvebu_icu_write_msi_msg,
 		.flags			= IRQCHIP_SUPPORTS_LEVEL_MSI,
 	},
@@ -212,7 +211,6 @@ static const struct msi_domain_template mvebu_icu_sei_msi_template = {
 		.irq_mask		= irq_chip_mask_parent,
 		.irq_unmask		= irq_chip_unmask_parent,
 		.irq_ack		= irq_chip_ack_parent,
-		.irq_set_type		= irq_chip_set_type_parent,
 		.irq_write_msi_msg	= mvebu_icu_write_msi_msg,
 		.flags			= IRQCHIP_SUPPORTS_LEVEL_MSI,
 	},
-- 
2.45.2

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

[Thomas Gleixner]

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

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

[Stefan Eichenberger]

Hi Thomas,

On Wed, Jan 15, 2025 at 09:15:24AM +0100, Thomas Gleixner wrote:
> 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.

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
      flags:   0x0
     parent:
        domain:  :ap807:config-space@f0000000:interrupt-controller@3f0200-5
         hwirq:   0x15
         chip:    SEI
          flags:   0x0

And then with kernel 6.10.14 without any patches:
root@localhost:~# uname -a
Linux localhost.localdomain 6.10.14 #1 SMP PREEMPT Thu Jan 16 17:52:59 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   ICU-SEI 116 Edge      f2400000.system-controller:thermal-sensor@70
 91:          0          0          0          0   ICU-SEI 116 Edge      f6400000.system-controller:thermal-sensor@70
root@localhost:~# cat /sys/kernel/debug/irq/irqs/90
handler:  handle_edge_irq
device:   (null)
status:   0x00000001
istate:   0x00004000
ddepth:   0
wdepth:   0
dstate:   0x02400201
            IRQ_TYPE_EDGE_RISING
            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
 hwirq:   0x74
 chip:    ICU-SEI
  flags:   0x0
 parent:
    domain:  :ap807:config-space@f0000000:interrupt-controller@3f0200-4
     hwirq:   0x505a
     chip:    SEI pMSI
      flags:   0x0
     parent:
        domain:  :ap807:config-space@f0000000:interrupt-controller@3f0200-2
         hwirq:   0x0
         chip:    CP SEI
          flags:   0x0
         parent:
            domain:  :ap807:config-space@f0000000:interrupt-controller@3f0200-5
             hwirq:   0x15
             chip:    SEI
              flags:   0x0

It seems with kernel 6.10 the controller device was not set correctly,
probably it was ignoring irq_set_type because of this. Do you by chance
have an idea how to properly fix this or should I do some more research?

Thanks and regards,
Stefan

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

[Thomas Gleixner]

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

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

[Stefan Eichenberger]

On Thu, Jan 16, 2025 at 10:05:25PM +0100, Thomas Gleixner wrote:
> 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?

I used the same device tree for the 6.10 kernel as I did for kernel
6.12. I didn't even recompile it. Unfortunately, I couldn't figure out
what exactly is different so far. However, on the 6.10 kernel I got the
following trace:
root@localhost:/sys/kernel/tracing# cat trace
# tracer: function_graph
#
# CPU  DURATION                  FUNCTION CALLS
# |     |   |                     |   |   |   |
 1)               |  devm_request_threaded_irq() {
 1)               |    request_threaded_irq() {
 0)               |      __irq_set_trigger() {
 0)               |        irq_chip_set_type_parent() {
 0)               |          irq_chip_set_type_parent() {
 0)   2.560 us    |            mvebu_sei_cp_set_type(); /* = 0x0 */
 0)   8.640 us    |          } /* irq_chip_set_type_parent = 0x0 */
 0) + 14.400 us   |        } /* irq_chip_set_type_parent = 0x0 */
 0) + 24.320 us   |      } /* __irq_set_trigger = 0x0 */
 0) * 13601.60 us |    } /* request_threaded_irq = 0x0 */
 0) * 13617.44 us |  } /* devm_request_threaded_irq = 0x0 */

It seems that irq_chip_set_type_parent is not failing. By adding some
debug messages to kernel/irq/manage.c I found that irqd_get_trigger_type
returns IRQF_TRIGGER_RISING even though it should return
IRQF_TRIGGER_HIGH according to the device tree. Maybe this was fixed
between 6.10 and 6.12 but I need to analyze that again in more detail.
[   91.069407] genirq: __setup_irq - kernel/irq/manage.c:1530, flags: 0x0
[   91.076042] genirq: __setup_irq - kernel/irq/manage.c:1533, flags: 0x1

For a possible solution, should I just change the device tree so that
IRQF_TRIGGER_RISING is used instead of IRQF_TRIGGER_HIGH?

Regards,
Stefan

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

[Thomas Gleixner]

On Fri, Jan 17 2025 at 19:03, Stefan Eichenberger wrote:
> It seems that irq_chip_set_type_parent is not failing. By adding some
> debug messages to kernel/irq/manage.c I found that irqd_get_trigger_type
> returns IRQF_TRIGGER_RISING even though it should return
> IRQF_TRIGGER_HIGH according to the device tree. Maybe this was fixed
> between 6.10 and 6.12 but I need to analyze that again in more detail.

The trigger type is fixed up in mvebu_icu_irq_domain_translate(), but
that should be the same with the new code in mvebu_icu_translate().

Can you instrument mvebu_icu_translate() and validate that it is

    1) Invoked at all

    2) The type fixup is done:

       if (msi_data->subset_data->icu_group == ICU_GRP_SEI)
              *type = IRQ_TYPE_EDGE_RISING;

Thanks,

        tglx

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

[Stefan Eichenberger]

On Mon, Jan 20, 2025 at 09:45:43AM +0100, Thomas Gleixner wrote:
> On Fri, Jan 17 2025 at 19:03, Stefan Eichenberger wrote:
> > It seems that irq_chip_set_type_parent is not failing. By adding some
> > debug messages to kernel/irq/manage.c I found that irqd_get_trigger_type
> > returns IRQF_TRIGGER_RISING even though it should return
> > IRQF_TRIGGER_HIGH according to the device tree. Maybe this was fixed
> > between 6.10 and 6.12 but I need to analyze that again in more detail.
> 
> The trigger type is fixed up in mvebu_icu_irq_domain_translate(), but
> that should be the same with the new code in mvebu_icu_translate().
> 
> Can you instrument mvebu_icu_translate() and validate that it is
> 
>     1) Invoked at all
> 
>     2) The type fixup is done:
> 
>        if (msi_data->subset_data->icu_group == ICU_GRP_SEI)
>               *type = IRQ_TYPE_EDGE_RISING;

Thanks a lot for pointing me in the right direction. I think I found
what is going on. My instrumentation showed that we still end up in that
function but the translation does not work. I think the reason is that
the msi_data conversion is wrong. The following patch would solve the
issue:

diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
index b337f6c05f18..4eebed39880a 100644
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -68,7 +68,8 @@ static int mvebu_icu_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
                               unsigned long *hwirq, unsigned int *type)
 {
        unsigned int param_count = static_branch_unlikely(&legacy_bindings) ? 3 : 2;
-       struct mvebu_icu_msi_data *msi_data = d->host_data;
+       struct msi_domain_info *info = d->host_data;
+       struct mvebu_icu_msi_data *msi_data = info->chip_data;
        struct mvebu_icu *icu = msi_data->icu;

        /* Check the count of the parameters in dt */

Do you agree with that fix? If so I will prepare a proper patch.

Thanks for your help, regards,
Stefan

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

[Thomas Gleixner]

On Mon, Jan 20 2025 at 17:57, Stefan Eichenberger wrote:
> On Mon, Jan 20, 2025 at 09:45:43AM +0100, Thomas Gleixner wrote:
>> On Fri, Jan 17 2025 at 19:03, Stefan Eichenberger wrote:
>> > It seems that irq_chip_set_type_parent is not failing. By adding some
>> > debug messages to kernel/irq/manage.c I found that irqd_get_trigger_type
>> > returns IRQF_TRIGGER_RISING even though it should return
>> > IRQF_TRIGGER_HIGH according to the device tree. Maybe this was fixed
>> > between 6.10 and 6.12 but I need to analyze that again in more detail.
>> 
>> The trigger type is fixed up in mvebu_icu_irq_domain_translate(), but
>> that should be the same with the new code in mvebu_icu_translate().
>> 
>> Can you instrument mvebu_icu_translate() and validate that it is
>> 
>>     1) Invoked at all
>> 
>>     2) The type fixup is done:
>> 
>>        if (msi_data->subset_data->icu_group == ICU_GRP_SEI)
>>               *type = IRQ_TYPE_EDGE_RISING;
>
> Thanks a lot for pointing me in the right direction. I think I found
> what is going on. My instrumentation showed that we still end up in that
> function but the translation does not work. I think the reason is that
> the msi_data conversion is wrong. The following patch would solve the
> issue:
>
> diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
> index b337f6c05f18..4eebed39880a 100644
> --- a/drivers/irqchip/irq-mvebu-icu.c
> +++ b/drivers/irqchip/irq-mvebu-icu.c
> @@ -68,7 +68,8 @@ static int mvebu_icu_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
>                                unsigned long *hwirq, unsigned int *type)
>  {
>         unsigned int param_count = static_branch_unlikely(&legacy_bindings) ? 3 : 2;
> -       struct mvebu_icu_msi_data *msi_data = d->host_data;
> +       struct msi_domain_info *info = d->host_data;
> +       struct mvebu_icu_msi_data *msi_data = info->chip_data;
>         struct mvebu_icu *icu = msi_data->icu;
>
>         /* Check the count of the parameters in dt */
>
> Do you agree with that fix? If so I will prepare a proper patch.

Duh, yes. What was I thinking?

Thanks,

        tglx