[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[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 <marc.zyngier@arm.com>
---
 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(-)

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 1dabf43..66662a1 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -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);
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 1455829..5f3602e 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -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)
diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index 5c3af8f..d815d1b 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -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;
 }
-- 
2.1.3

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Fabio Estevam]

On Mon, Dec 1, 2014 at 3:04 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 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 <marc.zyngier@arm.com>

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

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Philipp Zabel]

Am Montag, den 01.12.2014, 17:04 +0000 schrieb 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 <marc.zyngier@arm.com>

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Marc Zyngier]

On 01/12/14 17:15, Philipp Zabel wrote:
> Am Montag, den 01.12.2014, 17:04 +0000 schrieb 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 <marc.zyngier@arm.com>
> 
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

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.
-- 
Jazz is not dead. It just smells funny...

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Vladimir Murzin]

Hi Marc,

On 01/12/14 17:04, Marc Zyngier wrote:
> 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

> 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 <marc.zyngier@arm.com>
> ---
>  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(-)
> 
> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> index 1dabf43..66662a1 100644
> --- a/arch/arm/mach-imx/common.h
> +++ b/arch/arm/mach-imx/common.h
> @@ -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);
> diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
> index 1455829..5f3602e 100644
> --- a/arch/arm/mach-imx/gpc.c
> +++ b/arch/arm/mach-imx/gpc.c
> @@ -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)
> diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
> index 5c3af8f..d815d1b 100644
> --- a/arch/arm/mach-imx/pm-imx6.c
> +++ b/arch/arm/mach-imx/pm-imx6.c
> @@ -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;
>  }
> 

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Marc Zyngier]

On 01/12/14 17:57, Vladimir Murzin wrote:
> Hi Marc,
> 
> On 01/12/14 17:04, Marc Zyngier wrote:
>> 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.
-- 
Jazz is not dead. It just smells funny...

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Shawn Guo]

On Mon, Dec 01, 2014 at 05:04:03PM +0000, Marc Zyngier wrote:
> 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 <marc.zyngier@arm.com>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

Arnd, Olof,

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

Shawn

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Marc Zyngier]

Hi Shawn,

On 02/12/14 14:48, Shawn Guo wrote:
> On Mon, Dec 01, 2014 at 05:04:03PM +0000, Marc Zyngier wrote:
>> 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 <marc.zyngier@arm.com>
> 
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> 
> 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.
-- 
Jazz is not dead. It just smells funny...

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Shawn Guo]

On Tue, Dec 02, 2014 at 02:53:08PM +0000, Marc Zyngier wrote:
> Hi Shawn,
> 
> On 02/12/14 14:48, Shawn Guo wrote:
> > On Mon, Dec 01, 2014 at 05:04:03PM +0000, Marc Zyngier wrote:
> >> 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 <marc.zyngier@arm.com>
> > 
> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > 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

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Marc Zyngier]

On 02/12/14 15:15, Shawn Guo wrote:
> On Tue, Dec 02, 2014 at 02:53:08PM +0000, Marc Zyngier wrote:
>> Hi Shawn,
>>
>> On 02/12/14 14:48, Shawn Guo wrote:
>>> On Mon, Dec 01, 2014 at 05:04:03PM +0000, Marc Zyngier wrote:
>>>> 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 <marc.zyngier@arm.com>
>>>
>>> Acked-by: Shawn Guo <shawn.guo@linaro.org>
>>>
>>> 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.

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

Thanks,

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

[PATCH] ARM: imx6: fix bogus use of irq_get_irq_data

[Shawn Guo]

On Tue, Dec 02, 2014 at 03:22:19PM +0000, Marc Zyngier wrote:
> On 02/12/14 15:15, Shawn Guo wrote:
> > On Tue, Dec 02, 2014 at 02:53:08PM +0000, Marc Zyngier wrote:
> >> Hi Shawn,
> >>
> >> On 02/12/14 14:48, Shawn Guo wrote:
> >>> On Mon, Dec 01, 2014 at 05:04:03PM +0000, Marc Zyngier wrote:
> >>>> 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 <marc.zyngier@arm.com>
> >>>
> >>> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> >>>
> >>> 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.
> 
> Sure. Can I put your Acked-by on both patches?

Yes, Marc.

Shawn