| Message ID | 1394168285-32275-5-git-send-email-chenhui.zhao@freescale.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Scott Wood |
| Headers | show |
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote: > In 64-bit mode, kernel just clears the irq soft-enable flag > in struct paca_struct to disable external irqs. But, in > the case of suspend, irqs should be disabled by hardware. > Therefore, hook a function to ppc_md.suspend_disable_irqs > to really disable irqs. > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com> > --- > arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > index 3fdf9f3..983d81f 100644 > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > @@ -32,6 +32,13 @@ > #include <sysdev/fsl_pci.h> > #include "smp.h" > > +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND) > +static void fsl_suspend_disable_irqs(void) > +{ > + __hard_irq_disable(); > +} > +#endif Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set? If hard disabling is appropriate here, shouldn't we do it in generic_suspend_disable_irqs()? Are there any existing platforms that supply a ppc_md.suspend_disable_irqs()? I don't see any when grepping. -Scott
On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote: > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote: > > In 64-bit mode, kernel just clears the irq soft-enable flag > > in struct paca_struct to disable external irqs. But, in > > the case of suspend, irqs should be disabled by hardware. > > Therefore, hook a function to ppc_md.suspend_disable_irqs > > to really disable irqs. > > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com> > > --- > > arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++ > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > > index 3fdf9f3..983d81f 100644 > > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > > @@ -32,6 +32,13 @@ > > #include <sysdev/fsl_pci.h> > > #include "smp.h" > > > > +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND) > > +static void fsl_suspend_disable_irqs(void) > > +{ > > + __hard_irq_disable(); > > +} > > +#endif > > Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set? > > If hard disabling is appropriate here, shouldn't we do it in > generic_suspend_disable_irqs()? > > Are there any existing platforms that supply a > ppc_md.suspend_disable_irqs()? I don't see any when grepping. > > -Scott Will use hard_irq_disable(). I think this is a general problem for powerpc. Should clear MSR_EE before suspend. I agree to put it in generic_suspend_disable_irqs(). -Chenhui
On Wed, 2014-03-12 at 15:46 +0800, Chenhui Zhao wrote: > On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote: > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote: > > > In 64-bit mode, kernel just clears the irq soft-enable flag > > > in struct paca_struct to disable external irqs. But, in > > > the case of suspend, irqs should be disabled by hardware. > > > Therefore, hook a function to ppc_md.suspend_disable_irqs > > > to really disable irqs. > > > > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com> > > > --- > > > arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++ > > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > > > index 3fdf9f3..983d81f 100644 > > > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > > > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > > > @@ -32,6 +32,13 @@ > > > #include <sysdev/fsl_pci.h> > > > #include "smp.h" > > > > > > +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND) > > > +static void fsl_suspend_disable_irqs(void) > > > +{ > > > + __hard_irq_disable(); > > > +} > > > +#endif > > > > Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set? > > > > If hard disabling is appropriate here, shouldn't we do it in > > generic_suspend_disable_irqs()? > > > > Are there any existing platforms that supply a > > ppc_md.suspend_disable_irqs()? I don't see any when grepping. > > > > -Scott > > Will use hard_irq_disable(). > > I think this is a general problem for powerpc. > Should clear MSR_EE before suspend. I agree to put it > in generic_suspend_disable_irqs(). BTW, make sure you test this patchset with CONFIG_DEBUG_PREEMPT and similar debugging options to help ensure that the soft IRQ state is being tracked properly. -Scott
On Fri, Mar 14, 2014 at 05:41:41PM -0500, Scott Wood wrote: > On Wed, 2014-03-12 at 15:46 +0800, Chenhui Zhao wrote: > > On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote: > > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote: > > > > In 64-bit mode, kernel just clears the irq soft-enable flag > > > > in struct paca_struct to disable external irqs. But, in > > > > the case of suspend, irqs should be disabled by hardware. > > > > Therefore, hook a function to ppc_md.suspend_disable_irqs > > > > to really disable irqs. > > > > > > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com> > > > > --- > > > > arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++ > > > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > > > > index 3fdf9f3..983d81f 100644 > > > > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > > > > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > > > > @@ -32,6 +32,13 @@ > > > > #include <sysdev/fsl_pci.h> > > > > #include "smp.h" > > > > > > > > +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND) > > > > +static void fsl_suspend_disable_irqs(void) > > > > +{ > > > > + __hard_irq_disable(); > > > > +} > > > > +#endif > > > > > > Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set? > > > > > > If hard disabling is appropriate here, shouldn't we do it in > > > generic_suspend_disable_irqs()? > > > > > > Are there any existing platforms that supply a > > > ppc_md.suspend_disable_irqs()? I don't see any when grepping. > > > > > > -Scott > > > > Will use hard_irq_disable(). > > > > I think this is a general problem for powerpc. > > Should clear MSR_EE before suspend. I agree to put it > > in generic_suspend_disable_irqs(). > > BTW, make sure you test this patchset with CONFIG_DEBUG_PREEMPT and > similar debugging options to help ensure that the soft IRQ state is > being tracked properly. > > -Scott OK. I'll keep that in mind. -Chenhui
diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c index 3fdf9f3..983d81f 100644 --- a/arch/powerpc/platforms/85xx/corenet_generic.c +++ b/arch/powerpc/platforms/85xx/corenet_generic.c @@ -32,6 +32,13 @@ #include <sysdev/fsl_pci.h> #include "smp.h" +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND) +static void fsl_suspend_disable_irqs(void) +{ + __hard_irq_disable(); +} +#endif + void __init corenet_gen_pic_init(void) { struct mpic *mpic; @@ -58,6 +65,11 @@ void __init corenet_gen_setup_arch(void) fsl_rcpm_init(); +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND) + /* physically disable irq for 64-bit mode when suspend */ + ppc_md.suspend_disable_irqs = fsl_suspend_disable_irqs; +#endif + pr_info("%s board from Freescale Semiconductor\n", ppc_md.name); }
In 64-bit mode, kernel just clears the irq soft-enable flag in struct paca_struct to disable external irqs. But, in the case of suspend, irqs should be disabled by hardware. Therefore, hook a function to ppc_md.suspend_disable_irqs to really disable irqs. Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com> --- arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)