ARM: versatile: fix MMC/SD interrupt assignment
Subsystems (2)
PATCH 2/2 ARM: versatile: fix MMC/SD interrupt assignment
Commit 0976c946a610d06e907335b7a3afa6db046f8e1b "arm/versatile: Fix versatile irq specifications" has an off-by-one error on the Versatile AB that has been regressing the Versatile AB hardware for some time.
However it seems like the interrupt assignments have never been correct and I have now adjusted them according to the specification. The masks for the valid interrupts made it impossible to assign the right SIC interrupt for the MMCI, so I went in and fixed these to correspond to the specifications, and added references if anyone wants to double-check.
Due to the Versatile PB including the Versatile AB as a base DTS file, we need to override and correct some values to correspond to the actual changes in the hardware.
For the Versatile PB I don't think the IRQ line assignment for MMCI has ever been correct for either of the two MMCI blocks. It would be nice if someone with the physical PB board could test this.
Patch tested on the Versatile AB, QEMU for Versatile AB and QEMU for Versatile PB.
---
ARM SoC people: please apply this directly for fixes if you find it OK.
--- arch/arm/boot/dts/versatile-ab.dts | 10 +++++++--- arch/arm/boot/dts/versatile-pb.dts | 20 +++++++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-)
arch/arm/boot/dts/versatile-ab.dts +7 -3
@@ -110,7 +110,11 @@ interrupt-parent = <&vic>; interrupts = <31>; /* Cascaded to vic */ clear-mask = <0xffffffff>; - valid-mask = <0xffc203f8>; + /* + * Valid interrupt lines mask according to + * table 4-36 page 4-50 of ARM DUI 0225D + */ + valid-mask = <0x0760031b>; }; dma at 10130000 {
@@ -266,8 +270,8 @@ }; mmc at 5000 { compatible = "arm,pl180", "arm,primecell"; - reg = < 0x5000 0x1000>; - interrupts-extended = <&vic 22 &sic 2>; + reg = <0x5000 0x1000>; + interrupts-extended = <&vic 22 &sic 1>; clocks = <&xtal24mhz>, <&pclk>; clock-names = "mclk", "apb_pclk"; };
arch/arm/boot/dts/versatile-pb.dts +19 -1
@@ -5,6 +5,16 @@ compatible = "arm,versatile-pb"; amba { + /* The Versatile PB is using more SIC IRQ lines than the AB */ + sic: intc at 10003000 { + clear-mask = <0xffffffff>; + /* + * Valid interrupt lines mask according to + * figure 3-30 page 3-74 of ARM DUI 0224B + */ + valid-mask = <0x7fe003ff>; + }; + gpio2: gpio at 101e6000 { compatible = "arm,pl061", "arm,primecell"; reg = <0x101e6000 0x1000>;
@@ -67,6 +77,13 @@ }; fpga { + mmc at 5000 { + /* + * Overrides the interrupt assignment from + * the Versatile AB board file. + */ + interrupts-extended = <&sic 22 &sic 23>; + }; uart at 9000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x9000 0x1000>;
@@ -86,7 +103,8 @@ mmc at b000 { compatible = "arm,pl180", "arm,primecell"; reg = <0xb000 0x1000>; - interrupts-extended = <&vic 23 &sic 2>; + interrupt-parent = <&sic>; + interrupts = <1>, <2>; clocks = <&xtal24mhz>, <&pclk>; clock-names = "mclk", "apb_pclk"; };
Signature · 1 line
2.4.3
Rob Herring
maintainerPATCH 2/2 ARM: versatile: fix MMC/SD interrupt assignment
Quoted 44 lines · Linus Walleij
Commit
0976c946a610d06e907335b7a3afa6db046f8e1b"arm/versatile: Fix versatile irq specifications" has an off-by-one error on the Versatile AB that has been regressing the Versatile AB hardware for some time.However it seems like the interrupt assignments have never been correct and I have now adjusted them according to the specification. The masks for the valid interrupts made it impossible to assign the right SIC interrupt for the MMCI, so I went in and fixed these to correspond to the specifications, and added references if anyone wants to double-check.
Due to the Versatile PB including the Versatile AB as a base DTS file, we need to override and correct some values to correspond to the actual changes in the hardware.
For the Versatile PB I don't think the IRQ line assignment for MMCI has ever been correct for either of the two MMCI blocks. It would be nice if someone with the physical PB board could test this.
Patch tested on the Versatile AB, QEMU for Versatile AB and QEMU for Versatile PB.
Cc Rob Herring <[email protected]>Cc Grant Likely <[email protected]>Cc stable at vger.kernel.orgFixes 0976c946a610 ("arm/versatile: Fix versatile irq specifications")Signed-off-by Linus Walleij <[email protected]>---
ARM SoC people: please apply this directly for fixes if you find it OK.--- arch/arm/boot/dts/versatile-ab.dts | 10 +++++++--- arch/arm/boot/dts/versatile-pb.dts | 20 +++++++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-)arch/arm/boot/dts/versatile-ab.dts +0
-0
arch/arm/boot/dts/versatile-ab.dts line 117
Quoted 9 lines
arch/arm/boot/dts/versatile-ab.dts +5
-1@@ -110,7 +110,11 @@ interrupt-parent = <&vic>; interrupts = <31>; /* Cascaded to vic */ clear-mask = <0xffffffff>; - valid-mask = <0xffc203f8>; + /* + * Valid interrupt lines mask according to + * table 4-36 page 4-50 of ARM DUI 0225D + */ + valid-mask = <0x0760031b>;
I never really liked valid-mask in the first place. Valid interrupts are the ones specified by devices and we don't need this extra data. If we do, then *every* interrupt controller needs this property. Perhaps the driver should just ignore it. But you've already done the work here, so this is okay too.
Rob
PATCH 2/2 ARM: versatile: fix MMC/SD interrupt assignment
Quoted 14 lines · Rob Herring
Quoted 12 lines · Linus Walleij
arch/arm/boot/dts/versatile-ab.dts +3
-1@@ -110,7 +110,11 @@ interrupt-parent = <&vic>; interrupts = <31>; /* Cascaded to vic */ clear-mask = <0xffffffff>; - valid-mask = <0xffc203f8>; + /* + * Valid interrupt lines mask according to + * table 4-36 page 4-50 of ARM DUI 0225D
Quoted 8 lines
arch/arm/boot/dts/versatile-ab.dts line 117
Quoted 2 lines
arch/arm/boot/dts/versatile-ab.dts +2
-0+ */ + valid-mask = <0x0760031b>;I never really liked valid-mask in the first place. Valid interrupts are the ones specified by devices and we don't need this extra data. If we do, then *every* interrupt controller needs this property. Perhaps the driver should just ignore it. But you've already done the work here, so this is okay too.
It's a migrated feature from the boardfile days as it happens.
Before commit d7057e1de8d6c180eb2ecd90b0cab9d1a8a4d8ab "ARM: integrator: delete non-devicetree boot path" the Integrator/CP was doing this:
static void __init intcp_init_irq(void)
{
u32 pic_mask, cic_mask, sic_mask;
/* These masks are for the HW IRQ registers */
pic_mask = ~((~0u) << (11 - 0));
pic_mask |= (~((~0u) << (29 - 22))) << 22;
cic_mask = ~((~0u) << (1 + IRQ_CIC_END - IRQ_CIC_START));
sic_mask = ~((~0u) << (1 + IRQ_SIC_END - IRQ_SIC_START));
/*
* Disable all interrupt sources
*/
writel(0xffffffff, INTCP_VA_PIC_BASE + IRQ_ENABLE_CLEAR);
writel(0xffffffff, INTCP_VA_PIC_BASE + FIQ_ENABLE_CLEAR);
writel(0xffffffff, INTCP_VA_CIC_BASE + IRQ_ENABLE_CLEAR);
writel(0xffffffff, INTCP_VA_CIC_BASE + FIQ_ENABLE_CLEAR);
writel(sic_mask, INTCP_VA_SIC_BASE + IRQ_ENABLE_CLEAR);
writel(sic_mask, INTCP_VA_SIC_BASE + FIQ_ENABLE_CLEAR);
fpga_irq_init(INTCP_VA_PIC_BASE, "PIC", IRQ_PIC_START,
-1, pic_mask, NULL);
fpga_irq_init(INTCP_VA_CIC_BASE, "CIC", IRQ_CIC_START,
-1, cic_mask, NULL);
fpga_irq_init(INTCP_VA_SIC_BASE, "SIC", IRQ_SIC_START,
IRQ_CP_CPPLDINT, sic_mask, NULL);(...)
The clear-mask and valid-mask:s found in arch/arm/boot/dts/integratorcp.dts comes from this for example. And that is a modernization of code that was always there.
I think it's because the Integrator will crash if you try to even clear a non-existing interrupt by writing a '1' in the wrong bit, and that is why we have the clear-mask. And valid-mask is to avoid even mapping one of these non-existing IRQs. The issue doesn't seem to exist on the Versatile (I tried with 0xffffffff and it worked) and probably not on Integrator QEMU either, but on the real hardware I've experienced it.
So the FPGA interrupt controller is sensitive stuff ... and it seemed natural to preserve this overzealous code (and bindings).
Yours,
Linus Walleij
Rob Herring
maintainerPATCH 2/2 ARM: versatile: fix MMC/SD interrupt assignment
Quoted 25 lines · Linus Walleij
Quoted 14 lines · Rob Herring
Quoted 12 lines · Linus Walleij
arch/arm/boot/dts/versatile-ab.dts +3
-1@@ -110,7 +110,11 @@ interrupt-parent = <&vic>; interrupts = <31>; /* Cascaded to vic */ clear-mask = <0xffffffff>; - valid-mask = <0xffc203f8>; + /* + * Valid interrupt lines mask according to + * table 4-36 page 4-50 of ARM DUI 0225DQuoted 8 lines
arch/arm/boot/dts/versatile-ab.dts line 117
Quoted 2 lines
arch/arm/boot/dts/versatile-ab.dts +2
-0+ */ + valid-mask = <0x0760031b>;I never really liked valid-mask in the first place. Valid interrupts are the ones specified by devices and we don't need this extra data. If we do, then *every* interrupt controller needs this property. Perhaps the driver should just ignore it. But you've already done the work here, so this is okay too.
It's a migrated feature from the boardfile days as it happens.
[...]
Quoted 11 lines
I think it's because the Integrator will crash if you try to even clear a non-existing interrupt by writing a '1' in the wrong bit, and that is why we have the clear-mask. And valid-mask is to avoid even mapping one of these non-existing IRQs. The issue doesn't seem to exist on the Versatile (I tried with
0xffffffffand it worked) and probably not on Integrator QEMU either, but on the real hardware I've experienced it.So the FPGA interrupt controller is sensitive stuff ... and it seemed natural to preserve this overzealous code (and bindings).
Okay, well this is certainly a better reason than I had heard before.
Rob
Olof Johansson
AppliedPATCH 2/2 ARM: versatile: fix MMC/SD interrupt assignment
In reply to Linus WalleijQuoted 26 lines · Linus Walleij
Commit
0976c946a610d06e907335b7a3afa6db046f8e1b"arm/versatile: Fix versatile irq specifications" has an off-by-one error on the Versatile AB that has been regressing the Versatile AB hardware for some time.However it seems like the interrupt assignments have never been correct and I have now adjusted them according to the specification. The masks for the valid interrupts made it impossible to assign the right SIC interrupt for the MMCI, so I went in and fixed these to correspond to the specifications, and added references if anyone wants to double-check.
Due to the Versatile PB including the Versatile AB as a base DTS file, we need to override and correct some values to correspond to the actual changes in the hardware.
For the Versatile PB I don't think the IRQ line assignment for MMCI has ever been correct for either of the two MMCI blocks. It would be nice if someone with the physical PB board could test this.
Patch tested on the Versatile AB, QEMU for Versatile AB and QEMU for Versatile PB.
Quoted 8 lines
Cc Rob Herring <[email protected]>Cc Grant Likely <[email protected]>Cc stable at vger.kernel.orgFixes 0976c946a610 ("arm/versatile: Fix versatile irq specifications")Signed-off-by Linus Walleij <[email protected]>---
ARM SoC people: please apply this directly for fixes if you find it OK.
Applied to fixes. Seems like Rob's comments were concerns but not blockers, so picking up the fix seems to make sense.
-Olof