Patchwork [v2,1/4] powerpc/mpic: add irq_set_wake support

login
register
mail settings
Submitter Dongsheng Wang
Date April 2, 2013, 6:40 a.m.
Message ID <1364884840-28635-1-git-send-email-dongsheng.wang@freescale.com>
Download mbox | patch
Permalink /patch/232909/
State Superseded, archived
Delegated to: Kumar Gala
Headers show

Comments

Dongsheng Wang - April 2, 2013, 6:40 a.m.
Add irq_set_wake support. Just add IRQF_NO_SUSPEND to desc->action->flag.
So the wake up interrupt will not be disable in suspend_device_irqs.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
---
v2:
* Add: Check freescale chip in mpic_irq_set_wake().
* Remove: Support mpic_irq_set_wake() in ht_chip.

 arch/powerpc/sysdev/mpic.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Scott Wood - April 3, 2013, 12:36 a.m.
On 04/02/2013 01:40:37 AM, Wang Dongsheng wrote:
> Add irq_set_wake support. Just add IRQF_NO_SUSPEND to  
> desc->action->flag.
> So the wake up interrupt will not be disable in suspend_device_irqs.
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> v2:
> * Add: Check freescale chip in mpic_irq_set_wake().
> * Remove: Support mpic_irq_set_wake() in ht_chip.
> 
>  arch/powerpc/sysdev/mpic.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 3b2efd4..50d1ee1 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -920,6 +920,22 @@ int mpic_set_irq_type(struct irq_data *d,  
> unsigned int flow_type)
>  	return IRQ_SET_MASK_OK_NOCOPY;
>  }
> 
> +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +	struct irq_desc *desc = container_of(d, struct irq_desc,  
> irq_data);
> +	struct mpic *mpic = mpic_from_irq_data(d);
> +
> +	if (!(mpic->flags & MPIC_FSL))
> +		return -EINVAL;

I was thinking more along the lines of using MPIC_FSL during init to  
decide whether to write this function to .irq_set_wake, though that  
could probably wait until there's a second type of MPIC that needs this  
(if ever).

-Scott
Wang Dongsheng-B40534 - April 3, 2013, 2:49 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 03, 2013 8:37 AM
> To: Wang Dongsheng-B40534
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: Re: [PATCH v2 1/4] powerpc/mpic: add irq_set_wake support
> 
> On 04/02/2013 01:40:37 AM, Wang Dongsheng wrote:
> > Add irq_set_wake support. Just add IRQF_NO_SUSPEND to
> > desc->action->flag.
> > So the wake up interrupt will not be disable in suspend_device_irqs.
> >
> > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > ---
> > v2:
> > * Add: Check freescale chip in mpic_irq_set_wake().
> > * Remove: Support mpic_irq_set_wake() in ht_chip.
> >
> >  arch/powerpc/sysdev/mpic.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 3b2efd4..50d1ee1 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -920,6 +920,22 @@ int mpic_set_irq_type(struct irq_data *d,
> > unsigned int flow_type)
> >  	return IRQ_SET_MASK_OK_NOCOPY;
> >  }
> >
> > +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on) {
> > +	struct irq_desc *desc = container_of(d, struct irq_desc,
> > irq_data);
> > +	struct mpic *mpic = mpic_from_irq_data(d);
> > +
> > +	if (!(mpic->flags & MPIC_FSL))
> > +		return -EINVAL;
> 
> I was thinking more along the lines of using MPIC_FSL during init to
> decide whether to write this function to .irq_set_wake,

I think the static registration method is more reasonable. We must consider
readability. And mpic_irq_set_wake() will not be frequent calls. So within 
mpic_irq_set_wake() to decide is reasonable.

> though that could probably wait until there's a second type of MPIC
> that needs this (if ever).
Even if the mpic_irq_set_wake() register in the first type of MPIC that is not
belong MPIC_FSL, the function can return errno. I think the errno should be
"-ENXIO".
See kernel/irq/manage.c, set_irq_wake_real() the return value.
The desc->irq_data.chip->irq_set_wake is null, the errno is -ENXIO.

s/-EINVAL/-ENXIO/
Wang Dongsheng-B40534 - April 7, 2013, 3:40 a.m.
> -----Original Message-----
> From: Wang Dongsheng-B40534
> Sent: Wednesday, April 03, 2013 10:49 AM
> To: Wood Scott-B07421
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: RE: [PATCH v2 1/4] powerpc/mpic: add irq_set_wake support
> 
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, April 03, 2013 8:37 AM
> > To: Wang Dongsheng-B40534
> > Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> > Subject: Re: [PATCH v2 1/4] powerpc/mpic: add irq_set_wake support
> >
> > On 04/02/2013 01:40:37 AM, Wang Dongsheng wrote:
> > > Add irq_set_wake support. Just add IRQF_NO_SUSPEND to
> > > desc->action->flag.
> > > So the wake up interrupt will not be disable in suspend_device_irqs.
> > >
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > ---
> > > v2:
> > > * Add: Check freescale chip in mpic_irq_set_wake().
> > > * Remove: Support mpic_irq_set_wake() in ht_chip.
> > >
> > >  arch/powerpc/sysdev/mpic.c | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > > index 3b2efd4..50d1ee1 100644
> > > --- a/arch/powerpc/sysdev/mpic.c
> > > +++ b/arch/powerpc/sysdev/mpic.c
> > > @@ -920,6 +920,22 @@ int mpic_set_irq_type(struct irq_data *d,
> > > unsigned int flow_type)
> > >  	return IRQ_SET_MASK_OK_NOCOPY;
> > >  }
> > >
> > > +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on) {
> > > +	struct irq_desc *desc = container_of(d, struct irq_desc,
> > > irq_data);
> > > +	struct mpic *mpic = mpic_from_irq_data(d);
> > > +
> > > +	if (!(mpic->flags & MPIC_FSL))
> > > +		return -EINVAL;
> >
> > I was thinking more along the lines of using MPIC_FSL during init to
> > decide whether to write this function to .irq_set_wake,
> 
> I think the static registration method is more reasonable. We must
> consider readability. And mpic_irq_set_wake() will not be frequent calls.
> So within
> mpic_irq_set_wake() to decide is reasonable.
> 
> > though that could probably wait until there's a second type of MPIC
> > that needs this (if ever).
> Even if the mpic_irq_set_wake() register in the first type of MPIC that
> is not belong MPIC_FSL, the function can return errno. I think the errno
> should be "-ENXIO".
> See kernel/irq/manage.c, set_irq_wake_real() the return value.
> The desc->irq_data.chip->irq_set_wake is null, the errno is -ENXIO.
> 
> s/-EINVAL/-ENXIO/
About patches, if there are no other suggestion I'll send the V3.

Patch

diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 3b2efd4..50d1ee1 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -920,6 +920,22 @@  int mpic_set_irq_type(struct irq_data *d, unsigned int flow_type)
 	return IRQ_SET_MASK_OK_NOCOPY;
 }
 
+static int mpic_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	struct irq_desc *desc = container_of(d, struct irq_desc, irq_data);
+	struct mpic *mpic = mpic_from_irq_data(d);
+
+	if (!(mpic->flags & MPIC_FSL))
+		return -EINVAL;
+
+	if (on)
+		desc->action->flags |= IRQF_NO_SUSPEND;
+	else
+		desc->action->flags &= ~IRQF_NO_SUSPEND;
+
+	return 0;
+}
+
 void mpic_set_vector(unsigned int virq, unsigned int vector)
 {
 	struct mpic *mpic = mpic_from_irq(virq);
@@ -957,6 +973,7 @@  static struct irq_chip mpic_irq_chip = {
 	.irq_unmask	= mpic_unmask_irq,
 	.irq_eoi	= mpic_end_irq,
 	.irq_set_type	= mpic_set_irq_type,
+	.irq_set_wake	= mpic_irq_set_wake,
 };
 
 #ifdef CONFIG_SMP
@@ -971,6 +988,7 @@  static struct irq_chip mpic_tm_chip = {
 	.irq_mask	= mpic_mask_tm,
 	.irq_unmask	= mpic_unmask_tm,
 	.irq_eoi	= mpic_end_irq,
+	.irq_set_wake	= mpic_irq_set_wake,
 };
 
 #ifdef CONFIG_MPIC_U3_HT_IRQS