[PATCH] irq: imx-irqsteer: irq_count can larger than registered irq

[PATCH] irq: imx-irqsteer: irq_count can larger than registered irq

[Jacky Bai]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

The irqsteer IP itself can support up to 5 channels and each
channel can support max 960 IRQs per. The actual number of
channels and IRQ per channel implemented in HW is SoC related.

For previous i.MX SoCs, normally only one channel and max 512
IRQ is implemented, But for i.MX94, the irqsteer HW is implemented
for 960 IRQs, even it only has 384 IRQs connected.

That means on i.MX94, the 'irq_number' is 6 but the 'irq_count' is
15, so we need to increase the CHAN_MAX_OUTPUT_INT to 0xF(960 / 64)
to make sure the irqsteer can work correctly on i.MX94.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/irqchip/irq-imx-irqsteer.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
index b0e9788c0045..afbfcce3b1e3 100644
--- a/drivers/irqchip/irq-imx-irqsteer.c
+++ b/drivers/irqchip/irq-imx-irqsteer.c
@@ -24,7 +24,7 @@
 #define CHAN_MINTDIS(t)		(CTRL_STRIDE_OFF(t, 3) + 0x4)
 #define CHAN_MASTRSTAT(t)	(CTRL_STRIDE_OFF(t, 3) + 0x8)
 
-#define CHAN_MAX_OUTPUT_INT	0x8
+#define CHAN_MAX_OUTPUT_INT	0xF
 
 struct irqsteer_data {
 	void __iomem		*regs;
@@ -228,10 +228,8 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
 
 	for (i = 0; i < data->irq_count; i++) {
 		data->irq[i] = irq_of_parse_and_map(np, i);
-		if (!data->irq[i]) {
-			ret = -EINVAL;
-			goto out;
-		}
+		if (!data->irq[i])
+			break;
 
 		irq_set_chained_handler_and_data(data->irq[i],
 						 imx_irqsteer_irq_handler,
@@ -254,9 +252,13 @@ static void imx_irqsteer_remove(struct platform_device *pdev)
 	struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
 	int i;
 
-	for (i = 0; i < irqsteer_data->irq_count; i++)
+	for (i = 0; i < irqsteer_data->irq_count; i++) {
+		if (!irqsteer_data->irq[i])
+			break;
+
 		irq_set_chained_handler_and_data(irqsteer_data->irq[i],
 						 NULL, NULL);
+	}
 
 	irq_domain_remove(irqsteer_data->domain);
 
-- 
2.34.1

Re: [PATCH] irq: imx-irqsteer: irq_count can larger than registered irq

[Thomas Gleixner]

On Tue, Mar 04 2025 at 17:45, Jacky Bai wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>

Please use the proper subsystem prefix as documented:

  https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes

Also 'irq_count can larger than registered irq' is not a sentence and
tells absolutely nothing what this patch is about.

> The irqsteer IP itself can support up to 5 channels and each
> channel can support max 960 IRQs per. The actual number of

Can you please write out words, i.e. interrupts, hardware etc.. This is
a changelog and not twatter.

Aside of that: 'per.' does not make any sense. 'per' wants to be
followed by a noun: '...per $WHAT.'

> channels and IRQ per channel implemented in HW is SoC related.

> For previous i.MX SoCs, normally only one channel and max 512
> IRQ is implemented, But for i.MX94, the irqsteer HW is implemented
> for 960 IRQs, even it only has 384 IRQs connected.
>
> That means on i.MX94, the 'irq_number' is 6 but the 'irq_count' is

What means irq_number and irq_count here? 

> 15, so we need to increase the CHAN_MAX_OUTPUT_INT to 0xF(960 / 64)

What kind of math is 0xF(960 / 64) ? And what has this to do with
irq_number=6 ad irq_count=15?

Thanks,

        tglx

RE: [PATCH] irq: imx-irqsteer: irq_count can larger than registered irq

[Jacky Bai]

> Subject: Re: [PATCH] irq: imx-irqsteer: irq_count can larger than registered irq
> 
> On Tue, Mar 04 2025 at 17:45, Jacky Bai wrote:
> > From: Shengjiu Wang <shengjiu.wang@nxp.com>
Hi Thomas,

> 
> Please use the proper subsystem prefix as documented:
> 
> 
...

> 
> Also 'irq_count can larger than registered irq' is not a sentence and tells
> absolutely nothing what this patch is about.
> 

My fault, will fix it.

> > The irqsteer IP itself can support up to 5 channels and each channel
> > can support max 960 IRQs per. The actual number of
> 
> Can you please write out words, i.e. interrupts, hardware etc.. This is a
> changelog and not twatter.
> 
> Aside of that: 'per.' does not make any sense. 'per' wants to be followed by a
> noun: '...per $WHAT.'
> 

Thx, will improve and fix in v2.

> > channels and IRQ per channel implemented in HW is SoC related.
> 
> > For previous i.MX SoCs, normally only one channel and max 512 IRQ is
> > implemented, But for i.MX94, the irqsteer HW is implemented for 960
> > IRQs, even it only has 384 IRQs connected.
> >
> > That means on i.MX94, the 'irq_number' is 6 but the 'irq_count' is
> 
> What means irq_number and irq_count here?

For irqsteer, the interrupt inputs are divided into groups, each group has 64 interrupt
Inputs and use one irqsteer's interrupt output line to raise interrupt to parent interrupt controller.

The irq_number means how many interrupt output lines from irqsteer are really
connected to the parent interrupt controller, while the irq_count means the max
interrupt input line groups in the irqsteer instance.

If the irq_count is 15, as each group has 64 interrupts, it means the irqsteer
can support up to 960 interrupt inputs.

> 
> > 15, so we need to increase the CHAN_MAX_OUTPUT_INT to 0xF(960 / 64)
> 
> What kind of math is 0xF(960 / 64) ? And what has this to do with
> irq_number=6 ad irq_count=15?

As mentioned above, if the irqsteer is designed in HW with max 960 interrupt inputs,
It needs 15 interrupt outputs: 960 / 16 = 15. Even if we only 384, 512, or any number of
interrupt inputs less than 960 are actually connected to valid peripheral interrupt.

For i.MX94, In the SoC design, the irqsteer is designed with max 960 interrupt inputs, the
irq_count is 15. But only 384 inputs are really connected in HW. For these 384 interrupts,
it need 6 irqsteer interrupt output(384 / 64 = 6), the irq_number is 6.

In other world, the irq_number means the number of used interrupt outputs, while irq_count
means the max interrupt outputs that can be supported by an irqsteer instance.

I will refine the changelog to make it more clear. Sorry for the confusion,^_^

BR
Jacky
> 
> Thanks,
> 
>         tglx