Re: [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
On 4/27/21 1:30 AM, Marc Zyngier wrote:
> Hi Guenter,
>
> Thanks for the heads up.
>
> On Mon, 26 Apr 2021 23:39:42 +0100,
> Guenter Roeck wrote:
>>
>> On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote:
>>> irq_create_strict_mappings() is a poor way to allow the use of
>>> a linear IRQ domain as a legacy one. Let's be upfront about
>>> it and use a legacy domain when appropriate.
>>>
>>> Signed-off-by: Marc Zyngier
>>> ---
>>
>> When running the "mainstone" qemu emulation, this patch results
>> in many (32, actually) runtime warnings such as the following.
>>
>> [0.528272] [ cut here ]
>> [0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550
>> irq_domain_associate+0x194/0x1f0
>> [0.528315] error: virq335 is not allocated
>
> [...]
>
> This looks like a case of CONFIG_SPARSE_IRQ, combined with a lack of
> brain engagement. I've come up with the following patch, which lets
> the kernel boot in QEMU without screaming (other than the lack of a
> rootfs...).
>
> Please let me know if this helps.
>
It does.
Tested-by: Guenter Roeck
Thanks,
Guenter
> Thanks,
>
> M.
>
> From 4d7f6ddbbfdff1c9f029bafca79020d3294dc32c Mon Sep 17 00:00:00 2001
> From: Marc Zyngier
> Date: Tue, 27 Apr 2021 09:00:28 +0100
> Subject: [PATCH] ARM: PXA: Fix cplds irqdesc allocation when using legacy mode
>
> The Mainstone PXA platform uses CONFIG_SPARSE_IRQ, and thus we
> cannot rely on the irq descriptors to be readilly allocated
> before creating the irqdomain in legacy mode. The kernel then
> complains loudly about not being able to associate the interrupt
> in the domain -- can't blame it.
>
> Fix it by allocating the irqdescs upfront in the legacy case.
>
> Fixes: b68761da0111 ("ARM: PXA: Kill use of irq_create_strict_mappings()")
> Reported-by: Guenter Roeck
> Signed-off-by: Marc Zyngier
> ---
> arch/arm/mach-pxa/pxa_cplds_irqs.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c
> b/arch/arm/mach-pxa/pxa_cplds_irqs.c
> index ec0d9b094744..bddfc7cd5d40 100644
> --- a/arch/arm/mach-pxa/pxa_cplds_irqs.c
> +++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c
> @@ -121,8 +121,13 @@ static int cplds_probe(struct platform_device *pdev)
> return fpga->irq;
>
> base_irq = platform_get_irq(pdev, 1);
> - if (base_irq < 0)
> + if (base_irq < 0) {
> base_irq = 0;
> + } else {
> + ret = devm_irq_alloc_descs(>dev, base_irq, base_irq,
> CPLDS_NB_IRQ, 0);
> + if (ret < 0)
> + return ret;
> + }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> fpga->base = devm_ioremap_resource(>dev, res);
>
Re: [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
Hi Guenter,
Thanks for the heads up.
On Mon, 26 Apr 2021 23:39:42 +0100,
Guenter Roeck wrote:
>
> On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote:
> > irq_create_strict_mappings() is a poor way to allow the use of
> > a linear IRQ domain as a legacy one. Let's be upfront about
> > it and use a legacy domain when appropriate.
> >
> > Signed-off-by: Marc Zyngier
> > ---
>
> When running the "mainstone" qemu emulation, this patch results
> in many (32, actually) runtime warnings such as the following.
>
> [0.528272] [ cut here ]
> [0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550
> irq_domain_associate+0x194/0x1f0
> [0.528315] error: virq335 is not allocated
[...]
This looks like a case of CONFIG_SPARSE_IRQ, combined with a lack of
brain engagement. I've come up with the following patch, which lets
the kernel boot in QEMU without screaming (other than the lack of a
rootfs...).
Please let me know if this helps.
Thanks,
M.
From 4d7f6ddbbfdff1c9f029bafca79020d3294dc32c Mon Sep 17 00:00:00 2001
From: Marc Zyngier
Date: Tue, 27 Apr 2021 09:00:28 +0100
Subject: [PATCH] ARM: PXA: Fix cplds irqdesc allocation when using legacy mode
The Mainstone PXA platform uses CONFIG_SPARSE_IRQ, and thus we
cannot rely on the irq descriptors to be readilly allocated
before creating the irqdomain in legacy mode. The kernel then
complains loudly about not being able to associate the interrupt
in the domain -- can't blame it.
Fix it by allocating the irqdescs upfront in the legacy case.
Fixes: b68761da0111 ("ARM: PXA: Kill use of irq_create_strict_mappings()")
Reported-by: Guenter Roeck
Signed-off-by: Marc Zyngier
---
arch/arm/mach-pxa/pxa_cplds_irqs.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c
b/arch/arm/mach-pxa/pxa_cplds_irqs.c
index ec0d9b094744..bddfc7cd5d40 100644
--- a/arch/arm/mach-pxa/pxa_cplds_irqs.c
+++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c
@@ -121,8 +121,13 @@ static int cplds_probe(struct platform_device *pdev)
return fpga->irq;
base_irq = platform_get_irq(pdev, 1);
- if (base_irq < 0)
+ if (base_irq < 0) {
base_irq = 0;
+ } else {
+ ret = devm_irq_alloc_descs(>dev, base_irq, base_irq,
CPLDS_NB_IRQ, 0);
+ if (ret < 0)
+ return ret;
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
fpga->base = devm_ioremap_resource(>dev, res);
--
2.30.2
--
Without deviation from the norm, progress is not possible.
Re: [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote: > irq_create_strict_mappings() is a poor way to allow the use of > a linear IRQ domain as a legacy one. Let's be upfront about > it and use a legacy domain when appropriate. > > Signed-off-by: Marc Zyngier > --- When running the "mainstone" qemu emulation, this patch results in many (32, actually) runtime warnings such as the following. [0.528272] [ cut here ] [0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0 [0.528315] error: virq335 is not allocated [0.528325] Modules linked in: [0.528351] CPU: 0 PID: 1 Comm: swapper Tainted: GW 5.12.0-rc8-next-20210423 #1 [0.528372] Hardware name: Intel HCDDBBVA0 Development Platform (aka Mainstone) [0.528387] Backtrace: [0.528406] [] (dump_backtrace) from [] (show_stack+0x20/0x24) [0.528441] r7:0226 r6:c00796e8 r5:0009 r4:c088d2a0 [0.528454] [] (show_stack) from [] (dump_stack+0x28/0x30) [0.528479] [] (dump_stack) from [] (__warn+0xe8/0x110) [0.528507] r5:0009 r4:c0872698 [0.528520] [] (__warn) from [] (warn_slowpath_fmt+0xa0/0xe0) [0.528551] r7:c00796e8 r6:0226 r5:c0872698 r4:c0872700 [0.528564] [] (warn_slowpath_fmt) from [] (irq_domain_associate+0x194/0x1f0) [0.528597] r8:0130 r7:014f r6:001f r5: r4:c11bd780 [0.528610] [] (irq_domain_associate) from [] (irq_domain_associate_many+0x60/0xa4) [0.528642] r8:0130 r7:c11bd780 r6:fed0 r5:0150 r4:0150 [0.528655] [] (irq_domain_associate_many) from [] (irq_domain_create_legacy+0x5c/0x68) [0.528687] r8:0130 r7:0130 r6:0020 r5: r4:c11bd780 [0.528699] [] (irq_domain_create_legacy) from [] (irq_domain_add_legacy+0x34/0x3c) [0.528730] r7:c09b1370 r6:c09b1360 r5:c11bd3a0 r4: [0.528743] [] (irq_domain_add_legacy) from [] (cplds_probe+0x170/0x1ac) [0.528768] [] (cplds_probe) from [] (platform_probe+0x50/0xb0) [0.528800] r8:c09d2c94 r7:c0aa4f88 r6:c09d2c94 r5:c09b1370 r4: [0.528814] [] (platform_probe) from [] (really_probe+0x100/0x4d4) [0.528844] r7:c0aa4f88 r6: r5: r4:c09b1370 [0.528858] [] (really_probe) from [] (driver_probe_device+0x88/0x20c) [0.528892] r10:c0974830 r9:c0a7 r8:c093b224 r7:c0a31de8 r6:c09d2c94 r5:c09d2c94 [0.528907] r4:c09b1370 [0.528919] [] (driver_probe_device) from [] (device_driver_attach+0x68/0x70) [0.528953] r9:c0a7 r8:c093b224 r7:c0a31de8 r6:c09d2c94 r5: r4:c09b1370 [0.528969] [] (device_driver_attach) from [] (__driver_attach+0xc0/0x164) [0.528997] r7:c0a31de8 r6:c09b1370 r5:c09d2c94 r4: [0.529009] [] (__driver_attach) from [] (bus_for_each_dev+0x84/0xcc) [0.529039] r7:c0a31de8 r6:c04306d4 r5:c09d2c94 r4: [0.529052] [] (bus_for_each_dev) from [] (driver_attach+0x28/0x30) [0.529082] r6: r5:c11bd200 r4:c09d2c94 [0.529095] [] (driver_attach) from [] (bus_add_driver+0x168/0x210) [0.529122] [] (bus_add_driver) from [] (driver_register+0x88/0x120) [0.529152] r7:c0a5c7e0 r6: r5:e000 r4:c09d2c94 [0.529165] [] (driver_register) from [] (__platform_driver_register+0x2c/0x34) [0.529191] r5:e000 r4:c094ba64 [0.529204] [] (__platform_driver_register) from [] (cplds_driver_init+0x20/0x28) [0.529230] [] (cplds_driver_init) from [] (do_one_initcall+0x60/0x27c) [0.529255] [] (do_one_initcall) from [] (kernel_init_freeable+0x158/0x1e4) [0.529284] r7:c0974850 r6:0007 r5:c0c0f720 r4:c09a02fc [0.529297] [] (kernel_init_freeable) from [] (kernel_init+0x18/0x110) [0.529328] r10: r9: r8: r7: r6: r5:c06c525c [0.529343] r4: [0.529354] [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) [0.529387] Exception stack(0xc0bdffb0 to 0xc0bdfff8) [0.529467] ffa0: [0.529587] ffc0: [0.529684] ffe0: 0013 [0.529726] r5:c06c525c r4: [0.529752] ---[ end trace d199929d2b87e077 ]--- Bisect log attached. Guenter --- # bad: [e3d35712f85ac84fb06234848f6c043ab418cf8b] Add linux-next specific files for 20210423 # good: [bf05bf16c76bb44ab5156223e1e58e26dfe30a88] Linux 5.12-rc8 git bisect start 'HEAD' 'v5.12-rc8' # good: [d4b5d9d94679a18bfa4ccdafd19876d58777911e] Merge remote-tracking branch 'crypto/master' git bisect good d4b5d9d94679a18bfa4ccdafd19876d58777911e # good: [27628e42fe59a698e66b671bf1e1f01f6a3fe765] Merge remote-tracking branch 'tip/auto-latest' git bisect good 27628e42fe59a698e66b671bf1e1f01f6a3fe765 # bad: [bc6c3ae4f662fc719d0bf144f150f72cab8912d4] Merge remote-tracking branch 'vfio/next' git bisect
[PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
irq_create_strict_mappings() is a poor way to allow the use of
a linear IRQ domain as a legacy one. Let's be upfront about
it and use a legacy domain when appropriate.
Signed-off-by: Marc Zyngier
---
arch/arm/mach-pxa/pxa_cplds_irqs.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c
b/arch/arm/mach-pxa/pxa_cplds_irqs.c
index 45c19ca96f7a..ec0d9b094744 100644
--- a/arch/arm/mach-pxa/pxa_cplds_irqs.c
+++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c
@@ -147,22 +147,20 @@ static int cplds_probe(struct platform_device *pdev)
}
irq_set_irq_wake(fpga->irq, 1);
- fpga->irqdomain = irq_domain_add_linear(pdev->dev.of_node,
- CPLDS_NB_IRQ,
- _irq_domain_ops, fpga);
+ if (base_irq)
+ fpga->irqdomain = irq_domain_add_legacy(pdev->dev.of_node,
+ CPLDS_NB_IRQ,
+ base_irq, 0,
+ _irq_domain_ops,
+ fpga);
+ else
+ fpga->irqdomain = irq_domain_add_linear(pdev->dev.of_node,
+ CPLDS_NB_IRQ,
+ _irq_domain_ops,
+ fpga);
if (!fpga->irqdomain)
return -ENODEV;
- if (base_irq) {
- ret = irq_create_strict_mappings(fpga->irqdomain, base_irq, 0,
-CPLDS_NB_IRQ);
- if (ret) {
- dev_err(>dev, "couldn't create the irq mapping
%d..%d\n",
- base_irq, base_irq + CPLDS_NB_IRQ);
- return ret;
- }
- }
-
return 0;
}
--
2.29.2
