| Message ID | 1417453443-9179-1-git-send-email-marc.zyngier@arm.com |
|---|---|
| State | New |
| Headers | show |
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>
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
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.
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; > } >
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.
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
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.
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
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.
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
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; }
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(-)