ARM: imx6: fix bogus use of irq_get_irq_data

PATCH New Marc Zyngier 5 participants 11 messages Dec 1, 2014 – Dec 2, 2014

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape

lore

Share & tools

lore

Marc Zyngier

author

2014-12-01 17:04 UTC #0 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.

Fix it by using a pair of helper functions that deal with the actual hardware.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm/mach-imx/common.h  |  4 ++--
 arch/arm/mach-imx/gpc.c     | 34 ++++++++++++++++++++++------------
 arch/arm/mach-imx/pm-imx6.c |  5 ++---
 3 files changed, 26 insertions(+), 17 deletions(-)

arch/arm/mach-imx/common.h +2 -2

index 1dabf43..66662a1 100644
@@ -108,8 +108,8 @@ void imx_gpc_pre_suspend(bool arm_power_off);
 void imx_gpc_post_resume(void);
 void imx_gpc_mask_all(void);
 void imx_gpc_restore_all(void);
-void imx_gpc_irq_mask(struct irq_data *d);
-void imx_gpc_irq_unmask(struct irq_data *d);
+void imx_gpc_hwirq_mask(unsigned int hwirq);
+void imx_gpc_hwirq_unmask(unsigned int hwirq);
 void imx_anatop_init(void);
 void imx_anatop_pre_suspend(void);
 void imx_anatop_post_resume(void);

arch/arm/mach-imx/gpc.c +22 ~~-12~~

index 1455829..5f3602e 100644
@@ -91,34 +91,44 @@ void imx_gpc_restore_all(void)
 		writel_relaxed(gpc_saved_imrs[i], reg_imr1 + i * 4);
 }
 
-void imx_gpc_irq_unmask(struct irq_data *d)
+void imx_gpc_hwirq_unmask(unsigned int hwirq)
 {
 	void __iomem *reg;
 	u32 val;
 
-	/* Sanity check for SPI irq */
-	if (d->hwirq < 32)
-		return;
-
-	reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4;
+	reg = gpc_base + GPC_IMR1 + (hwirq / 32 - 1) * 4;
 	val = readl_relaxed(reg);
-	val &= ~(1 << d->hwirq % 32);
+	val &= ~(1 << hwirq % 32);
 	writel_relaxed(val, reg);
 }
 
-void imx_gpc_irq_mask(struct irq_data *d)
+void imx_gpc_hwirq_mask(unsigned int hwirq)
 {
 	void __iomem *reg;
 	u32 val;
 
+	reg = gpc_base + GPC_IMR1 + (hwirq / 32 - 1) * 4;
+	val = readl_relaxed(reg);
+	val |= 1 << (hwirq % 32);
+	writel_relaxed(val, reg);
+}
+
+static void imx_gpc_irq_unmask(struct irq_data *d)
+{
+	/* Sanity check for SPI irq */
+	if (d->hwirq < 32)
+		return;
+
+	imx_gpc_hwirq_unmask(d->hwirq);
+}
+
+static void imx_gpc_irq_mask(struct irq_data *d)
+{
 	/* Sanity check for SPI irq */
 	if (d->hwirq < 32)
 		return;
 
-	reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4;
-	val = readl_relaxed(reg);
-	val |= 1 << (d->hwirq % 32);
-	writel_relaxed(val, reg);
+	imx_gpc_hwirq_mask(d->hwirq);
 }
 
 void __init imx_gpc_init(void)

arch/arm/mach-imx/pm-imx6.c +2 -3

index 5c3af8f..d815d1b 100644
@@ -261,7 +261,6 @@ static void imx6q_enable_wb(bool enable)
 
 int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
 {
-	struct irq_data *iomuxc_irq_data = irq_get_irq_data(32);
 	u32 val = readl_relaxed(ccm_base + CLPCR);
 
 	val &= ~BM_CLPCR_LPM;

@@ -316,9 +315,9 @@ int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
 	 * 3) Software should mask IRQ #32 right after CCM Low-Power mode
 	 *    is set (set bits 0-1 of CCM_CLPCR).
 	 */
-	imx_gpc_irq_unmask(iomuxc_irq_data);
+	imx_gpc_hwirq_unmask(32);
 	writel_relaxed(val, ccm_base + CLPCR);
-	imx_gpc_irq_mask(iomuxc_irq_data);
+	imx_gpc_hwirq_mask(32);
 
 	return 0;
 }

Signature · 1 line

2.1.3

Fabio Estevam

reviewer

2014-12-01 17:13 UTC #1 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Quoted 9 lines · Marc Zyngier

The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.
Signed-off-by Marc Zyngier <[email protected]>

Thanks a lot, Marc! This fixes mx6q boot on linux-next:

Tested-by Fabio Estevam <[email protected]>

Philipp Zabel

2014-12-01 17:15 UTC #2 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

In reply to Marc Zyngier

Am Montag, den 01.12.2014, 17:04 +0000 schrieb Marc Zyngier:

Quoted 9 lines

The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.
Signed-off-by Marc Zyngier <[email protected]>

Acked-by Philipp Zabel <[email protected]>

regards
Philipp

Marc Zyngier

author

2014-12-01 17:23 UTC #3 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Quoted 6 lines · Philipp Zabel

Am Montag, den 01.12.2014, 17:04 +0000 schrieb Marc Zyngier:
Quoted 5 lines
The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.

Quoted 6 lines

Quoted 4 lines
Fix it by using a pair of helper functions that deal with the actual hardware.
Signed-off-by Marc Zyngier <[email protected]>
Acked-by Philipp Zabel <[email protected]>

Thanks Fabio and Philipp for your prompt testing.

Arnd/Olof: Can you please queue these two patches? This is an issue similar to the one found on Tegra last week, and this is going to explode badly once the merge window opens.

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Vladimir Murzin

2014-12-01 17:57 UTC #4 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

In reply to Marc Zyngier

Hi Marc,

Quoted 2 lines · Marc Zyngier

The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt

        ^^^^^^^^^ ^^^^^^^^^

More "something" here! ;)

Vladimir

Quoted 114 lines

number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.

Fix it by using a pair of helper functions that deal with the actual hardware.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm/mach-imx/common.h  |  4 ++--
 arch/arm/mach-imx/gpc.c     | 34 ++++++++++++++++++++++------------
 arch/arm/mach-imx/pm-imx6.c |  5 ++---
 3 files changed, 26 insertions(+), 17 deletions(-)

arch/arm/mach-imx/common.h +2 -2

index 1dabf43..66662a1 100644
@@ -108,8 +108,8 @@ void imx_gpc_pre_suspend(bool arm_power_off);
 void imx_gpc_post_resume(void);
 void imx_gpc_mask_all(void);
 void imx_gpc_restore_all(void);
-void imx_gpc_irq_mask(struct irq_data *d);
-void imx_gpc_irq_unmask(struct irq_data *d);
+void imx_gpc_hwirq_mask(unsigned int hwirq);
+void imx_gpc_hwirq_unmask(unsigned int hwirq);
 void imx_anatop_init(void);
 void imx_anatop_pre_suspend(void);
 void imx_anatop_post_resume(void);

arch/arm/mach-imx/gpc.c +22 ~~-12~~

index 1455829..5f3602e 100644
@@ -91,34 +91,44 @@ void imx_gpc_restore_all(void)
 		writel_relaxed(gpc_saved_imrs[i], reg_imr1 + i * 4);
 }
 
-void imx_gpc_irq_unmask(struct irq_data *d)
+void imx_gpc_hwirq_unmask(unsigned int hwirq)
 {
 	void __iomem *reg;
 	u32 val;
 
-	/* Sanity check for SPI irq */
-	if (d->hwirq < 32)
-		return;
-
-	reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4;
+	reg = gpc_base + GPC_IMR1 + (hwirq / 32 - 1) * 4;
 	val = readl_relaxed(reg);
-	val &= ~(1 << d->hwirq % 32);
+	val &= ~(1 << hwirq % 32);
 	writel_relaxed(val, reg);
 }
 
-void imx_gpc_irq_mask(struct irq_data *d)
+void imx_gpc_hwirq_mask(unsigned int hwirq)
 {
 	void __iomem *reg;
 	u32 val;
 
+	reg = gpc_base + GPC_IMR1 + (hwirq / 32 - 1) * 4;
+	val = readl_relaxed(reg);
+	val |= 1 << (hwirq % 32);
+	writel_relaxed(val, reg);
+}
+
+static void imx_gpc_irq_unmask(struct irq_data *d)
+{
+	/* Sanity check for SPI irq */
+	if (d->hwirq < 32)
+		return;
+
+	imx_gpc_hwirq_unmask(d->hwirq);
+}
+
+static void imx_gpc_irq_mask(struct irq_data *d)
+{
 	/* Sanity check for SPI irq */
 	if (d->hwirq < 32)
 		return;
 
-	reg = gpc_base + GPC_IMR1 + (d->hwirq / 32 - 1) * 4;
-	val = readl_relaxed(reg);
-	val |= 1 << (d->hwirq % 32);
-	writel_relaxed(val, reg);
+	imx_gpc_hwirq_mask(d->hwirq);
 }
 
 void __init imx_gpc_init(void)

arch/arm/mach-imx/pm-imx6.c +2 -3

index 5c3af8f..d815d1b 100644
@@ -261,7 +261,6 @@ static void imx6q_enable_wb(bool enable)
 
 int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
 {
-	struct irq_data *iomuxc_irq_data = irq_get_irq_data(32);
 	u32 val = readl_relaxed(ccm_base + CLPCR);
 
 	val &= ~BM_CLPCR_LPM;

@@ -316,9 +315,9 @@ int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
 	 * 3) Software should mask IRQ #32 right after CCM Low-Power mode
 	 *    is set (set bits 0-1 of CCM_CLPCR).
 	 */
-	imx_gpc_irq_unmask(iomuxc_irq_data);
+	imx_gpc_hwirq_unmask(32);
 	writel_relaxed(val, ccm_base + CLPCR);
-	imx_gpc_irq_mask(iomuxc_irq_data);
+	imx_gpc_hwirq_mask(32);
 
 	return 0;
 }

Marc Zyngier

author

2014-12-01 18:09 UTC #5 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Quoted 7 lines · Vladimir Murzin

Hi Marc,
Quoted 2 lines · Marc Zyngier
The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt
        ^^^^^^^^^ ^^^^^^^^^
More "something" here! ;)

Better for me to write "something" a few times too many, rather than what I actually had in mind when I fixed that code...

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Shawn Guo

2014-12-02 14:48 UTC #6 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

In reply to Marc Zyngier

Quoted 9 lines · Marc Zyngier

The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.
Signed-off-by Marc Zyngier <[email protected]>

Acked-by Shawn Guo <[email protected]>

Arnd, Olof,

To avoid a non-booting 3.19-rc1 on i.MX6, please apply it for 3.19-rc1. Thanks.

Shawn

Marc Zyngier

author

2014-12-02 14:53 UTC #7 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Hi Shawn,

Quoted 9 lines · Shawn Guo

Quoted 8 lines · Marc Zyngier
The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.

Quoted 7 lines

Quoted 1 line
Signed-off-by Marc Zyngier <[email protected]>
Acked-by Shawn Guo <[email protected]>
Arnd, Olof,
To avoid a non-booting 3.19-rc1 on i.MX6, please apply it for 3.19-rc1.

The previous patch (ARM: imx: irq: fix buggy usage of irq_data irq field) should also be applied (you cannot apply one without the other anyway).

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Shawn Guo

2014-12-02 15:15 UTC #8 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Quoted 16 lines · Marc Zyngier

Hi Shawn,
Quoted 13 lines · Shawn Guo
Quoted 9 lines · Marc Zyngier
The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.
Signed-off-by Marc Zyngier <[email protected]>
Acked-by Shawn Guo <[email protected]>

Quoted 7 lines

Quoted 3 lines
Arnd, Olof,
To avoid a non-booting 3.19-rc1 on i.MX6, please apply it for 3.19-rc1.
The previous patch (ARM: imx: irq: fix buggy usage of irq_data irq field) should also be applied (you cannot apply one without the other anyway).

Marc,

Thanks a lot for the fixes. To ease arm-soc folks' work, you should probably resend the patches with squashing them into one or having them in a series.

Shawn

Marc Zyngier

author

2014-12-02 15:22 UTC #9 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Quoted 25 lines · Shawn Guo

Quoted 16 lines · Marc Zyngier
Hi Shawn,
Quoted 13 lines · Shawn Guo
Quoted 9 lines · Marc Zyngier
The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.
Signed-off-by Marc Zyngier <[email protected]>
Acked-by Shawn Guo <[email protected]>
Quoted 7 lines
Quoted 3 lines
Arnd, Olof,
To avoid a non-booting 3.19-rc1 on i.MX6, please apply it for 3.19-rc1.
The previous patch (ARM: imx: irq: fix buggy usage of irq_data irq field) should also be applied (you cannot apply one without the other anyway).

Quoted 5 lines

Marc,
Thanks a lot for the fixes. To ease arm-soc folks' work, you should probably resend the patches with squashing them into one or having them in a series.

Sure. Can I put your Acked-by on both patches?

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Shawn Guo

2014-12-02 15:30 UTC #10 lore raw

PATCH 1/1 ARM: imx6: fix bogus use of irq_get_irq_data

Quoted 26 lines · Marc Zyngier

Quoted 25 lines · Shawn Guo
Quoted 23 lines · Marc Zyngier
Hi Shawn,
Quoted 9 lines · Shawn Guo
Quoted 8 lines · Marc Zyngier
The imx6 PM code seems to be quite creative in its use of irq_data, using something something that is very much a hardware interrupt number where we expect a virtual one. Yes, it worked so far, but that's only cheer luck, and it will definitely explode in 3.19.
Fix it by using a pair of helper functions that deal with the actual hardware.
Quoted 7 lines
Quoted 1 line
Signed-off-by Marc Zyngier <[email protected]>
Acked-by Shawn Guo <[email protected]>
Arnd, Olof,
To avoid a non-booting 3.19-rc1 on i.MX6, please apply it for 3.19-rc1.
The previous patch (ARM: imx: irq: fix buggy usage of irq_data irq field) should also be applied (you cannot apply one without the other anyway).

Quoted 7 lines

Quoted 5 lines
Marc,
Thanks a lot for the fixes. To ease arm-soc folks' work, you should probably resend the patches with squashing them into one or having them in a series.
Sure. Can I put your Acked-by on both patches?

Yes, Marc.

Shawn