[PATCH v2] mmc: sdhci: fix wakeup configuration

[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 <ludovic.desroches@atmel.com>
---
 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

[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 <ludovic.desroches@atmel.com>
---
 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

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

[Adrian Hunter]

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 <ludovic.desroches@atmel.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>


> ---
>  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);
>  
> 

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

[Ulf Hansson]

On 20 May 2016 at 13:46, Adrian Hunter <adrian.hunter@intel.com> 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 <ludovic.desroches@atmel.com>
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

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);
>>
>>
>

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

[Adrian Hunter]

On 20/05/2016 4:39 p.m., Ulf Hansson wrote:
> On 20 May 2016 at 13:46, Adrian Hunter <adrian.hunter@intel.com> 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 <ludovic.desroches@atmel.com>
>>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>
> 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);
>>>
>>>
>>

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 <adrian.hunter@intel.com> 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 <ludovic.desroches@atmel.com>
> > > 
> > > Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> > 
> > 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);
> > > > 
> > > > 
> > > 

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

[Ulf Hansson]

On 26 May 2016 at 14:30, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> 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 <adrian.hunter@intel.com> 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 <ludovic.desroches@atmel.com>
>> > >
>> > > Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> >
>> > 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.
>

Thanks, applied for next!

Kind regards
Uffe