Quantcast

[PATCH v2] mmc: sdhci: fix wakeup configuration

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2] mmc: sdhci: fix wakeup configuration

Ludovic Desroches
Activating wakeup event is not enough to get a wakeup signal. The
corresponding events have to be enabled in the Interrupt Status Enable
Register too. It follows the specification and is needed at least by
sdhci-of-at91.

Signed-off-by: Ludovic Desroches <[hidden email]>
---
 drivers/mmc/host/sdhci.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Changes:
- v2:
  - update commit message and comments
  - do not rename val and mask variables

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e010ea4..e351859 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2605,18 +2605,31 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
 \*****************************************************************************/
 
 #ifdef CONFIG_PM
+/*
+ * To enable wakeup events, the corresponding events have to be enabled in
+ * the Interrupt Status Enable register too. See 'Table 1-6: Wakeup Signal
+ * Table' in the SD Host Controller Standard Specification.
+ * It is useless to restore SDHCI_INT_ENABLE state in
+ * sdhci_disable_irq_wakeups() since it will be set by
+ * sdhci_enable_card_detection() or sdhci_init().
+ */
 void sdhci_enable_irq_wakeups(struct sdhci_host *host)
 {
  u8 val;
  u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
  | SDHCI_WAKE_ON_INT;
+ u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
+      SDHCI_INT_CARD_INT;
 
  val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
  val |= mask ;
  /* Avoid fake wake up */
- if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+ if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
  val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
+ irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
+ }
  sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
+ sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
 }
 EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
 
--
2.5.0

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] mmc: sdhci: fix wakeup configuration

Adrian Hunter-3
On 13/05/16 16:16, Ludovic Desroches wrote:
> Activating wakeup event is not enough to get a wakeup signal. The
> corresponding events have to be enabled in the Interrupt Status Enable
> Register too. It follows the specification and is needed at least by
> sdhci-of-at91.
>
> Signed-off-by: Ludovic Desroches <[hidden email]>

Acked-by: Adrian Hunter <[hidden email]>


> ---
>  drivers/mmc/host/sdhci.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> Changes:
> - v2:
>   - update commit message and comments
>   - do not rename val and mask variables
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index e010ea4..e351859 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2605,18 +2605,31 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
>  \*****************************************************************************/
>  
>  #ifdef CONFIG_PM
> +/*
> + * To enable wakeup events, the corresponding events have to be enabled in
> + * the Interrupt Status Enable register too. See 'Table 1-6: Wakeup Signal
> + * Table' in the SD Host Controller Standard Specification.
> + * It is useless to restore SDHCI_INT_ENABLE state in
> + * sdhci_disable_irq_wakeups() since it will be set by
> + * sdhci_enable_card_detection() or sdhci_init().
> + */
>  void sdhci_enable_irq_wakeups(struct sdhci_host *host)
>  {
>   u8 val;
>   u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
>   | SDHCI_WAKE_ON_INT;
> + u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
> +      SDHCI_INT_CARD_INT;
>  
>   val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>   val |= mask ;
>   /* Avoid fake wake up */
> - if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> + if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
>   val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
> + irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
> + }
>   sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
> + sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
>  }
>  EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
>  
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] mmc: sdhci: fix wakeup configuration

Ulf Hansson-2
On 20 May 2016 at 13:46, Adrian Hunter <[hidden email]> wrote:
> On 13/05/16 16:16, Ludovic Desroches wrote:
>> Activating wakeup event is not enough to get a wakeup signal. The
>> corresponding events have to be enabled in the Interrupt Status Enable
>> Register too. It follows the specification and is needed at least by
>> sdhci-of-at91.
>>
>> Signed-off-by: Ludovic Desroches <[hidden email]>
>
> Acked-by: Adrian Hunter <[hidden email]>

Is this material for stable and as a fix for 4.6?

Kind regards
Uffe

>
>
>> ---
>>  drivers/mmc/host/sdhci.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> Changes:
>> - v2:
>>   - update commit message and comments
>>   - do not rename val and mask variables
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index e010ea4..e351859 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -2605,18 +2605,31 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
>>  \*****************************************************************************/
>>
>>  #ifdef CONFIG_PM
>> +/*
>> + * To enable wakeup events, the corresponding events have to be enabled in
>> + * the Interrupt Status Enable register too. See 'Table 1-6: Wakeup Signal
>> + * Table' in the SD Host Controller Standard Specification.
>> + * It is useless to restore SDHCI_INT_ENABLE state in
>> + * sdhci_disable_irq_wakeups() since it will be set by
>> + * sdhci_enable_card_detection() or sdhci_init().
>> + */
>>  void sdhci_enable_irq_wakeups(struct sdhci_host *host)
>>  {
>>       u8 val;
>>       u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
>>                       | SDHCI_WAKE_ON_INT;
>> +     u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
>> +                   SDHCI_INT_CARD_INT;
>>
>>       val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>>       val |= mask ;
>>       /* Avoid fake wake up */
>> -     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> +     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
>>               val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
>> +             irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
>> +     }
>>       sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
>> +     sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
>>  }
>>  EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
>>
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] mmc: sdhci: fix wakeup configuration

Adrian Hunter-3
On 20/05/2016 4:39 p.m., Ulf Hansson wrote:

> On 20 May 2016 at 13:46, Adrian Hunter <[hidden email]> wrote:
>> On 13/05/16 16:16, Ludovic Desroches wrote:
>>> Activating wakeup event is not enough to get a wakeup signal. The
>>> corresponding events have to be enabled in the Interrupt Status Enable
>>> Register too. It follows the specification and is needed at least by
>>> sdhci-of-at91.
>>>
>>> Signed-off-by: Ludovic Desroches <[hidden email]>
>>
>> Acked-by: Adrian Hunter <[hidden email]>
>
> Is this material for stable and as a fix for 4.6?

Not as far as I know.

>
> Kind regards
> Uffe
>
>>
>>
>>> ---
>>>   drivers/mmc/host/sdhci.c | 15 ++++++++++++++-
>>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> Changes:
>>> - v2:
>>>    - update commit message and comments
>>>    - do not rename val and mask variables
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index e010ea4..e351859 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2605,18 +2605,31 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
>>>   \*****************************************************************************/
>>>
>>>   #ifdef CONFIG_PM
>>> +/*
>>> + * To enable wakeup events, the corresponding events have to be enabled in
>>> + * the Interrupt Status Enable register too. See 'Table 1-6: Wakeup Signal
>>> + * Table' in the SD Host Controller Standard Specification.
>>> + * It is useless to restore SDHCI_INT_ENABLE state in
>>> + * sdhci_disable_irq_wakeups() since it will be set by
>>> + * sdhci_enable_card_detection() or sdhci_init().
>>> + */
>>>   void sdhci_enable_irq_wakeups(struct sdhci_host *host)
>>>   {
>>>        u8 val;
>>>        u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
>>>                        | SDHCI_WAKE_ON_INT;
>>> +     u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
>>> +                   SDHCI_INT_CARD_INT;
>>>
>>>        val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
>>>        val |= mask ;
>>>        /* Avoid fake wake up */
>>> -     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>>> +     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
>>>                val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
>>> +             irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
>>> +     }
>>>        sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
>>> +     sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
>>>   }
>>>   EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] mmc: sdhci: fix wakeup configuration

Ludovic Desroches
On Fri, May 20, 2016 at 09:28:56PM +0300, Adrian Hunter wrote:

> On 20/05/2016 4:39 p.m., Ulf Hansson wrote:
> > On 20 May 2016 at 13:46, Adrian Hunter <[hidden email]> wrote:
> > > On 13/05/16 16:16, Ludovic Desroches wrote:
> > > > Activating wakeup event is not enough to get a wakeup signal. The
> > > > corresponding events have to be enabled in the Interrupt Status Enable
> > > > Register too. It follows the specification and is needed at least by
> > > > sdhci-of-at91.
> > > >
> > > > Signed-off-by: Ludovic Desroches <[hidden email]>
> > >
> > > Acked-by: Adrian Hunter <[hidden email]>
> >
> > Is this material for stable and as a fix for 4.6?
>
> Not as far as I know.
>

System PM code for the Atmel SDHCI has not been submitted yet so no need
to take it as a fix.

Regards

Ludovic

> >
> > Kind regards
> > Uffe
> >
> > >
> > >
> > > > ---
> > > >   drivers/mmc/host/sdhci.c | 15 ++++++++++++++-
> > > >   1 file changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > Changes:
> > > > - v2:
> > > >    - update commit message and comments
> > > >    - do not rename val and mask variables
> > > >
> > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > > index e010ea4..e351859 100644
> > > > --- a/drivers/mmc/host/sdhci.c
> > > > +++ b/drivers/mmc/host/sdhci.c
> > > > @@ -2605,18 +2605,31 @@ static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
> > > >   \*****************************************************************************/
> > > >
> > > >   #ifdef CONFIG_PM
> > > > +/*
> > > > + * To enable wakeup events, the corresponding events have to be enabled in
> > > > + * the Interrupt Status Enable register too. See 'Table 1-6: Wakeup Signal
> > > > + * Table' in the SD Host Controller Standard Specification.
> > > > + * It is useless to restore SDHCI_INT_ENABLE state in
> > > > + * sdhci_disable_irq_wakeups() since it will be set by
> > > > + * sdhci_enable_card_detection() or sdhci_init().
> > > > + */
> > > >   void sdhci_enable_irq_wakeups(struct sdhci_host *host)
> > > >   {
> > > >        u8 val;
> > > >        u8 mask = SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE
> > > >                        | SDHCI_WAKE_ON_INT;
> > > > +     u32 irq_val = SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE |
> > > > +                   SDHCI_INT_CARD_INT;
> > > >
> > > >        val = sdhci_readb(host, SDHCI_WAKE_UP_CONTROL);
> > > >        val |= mask ;
> > > >        /* Avoid fake wake up */
> > > > -     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> > > > +     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) {
> > > >                val &= ~(SDHCI_WAKE_ON_INSERT | SDHCI_WAKE_ON_REMOVE);
> > > > +             irq_val &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);
> > > > +     }
> > > >        sdhci_writeb(host, val, SDHCI_WAKE_UP_CONTROL);
> > > > +     sdhci_writel(host, irq_val, SDHCI_INT_ENABLE);
> > > >   }
> > > >   EXPORT_SYMBOL_GPL(sdhci_enable_irq_wakeups);
> > > >
> > > >
> > >
Loading...