[PATCH 0/2] mmc: core: power-on sdio card correctly.

[PATCH 0/2] mmc: core: power-on sdio card correctly.

[NeilBrown]

These two patches replace my previous patch
  [PATCH 2/2] mmc: core: reset sdio card properly on resume.

The only change is to use host->card->ocr instead of
host->ocr_avail, as recommend by Ulf.

These restore my libertas  wifi device properly on system
resume.

(I'm still hoping for a reply to my other patch:
   [PATCH 1/2] mmc: core: allow a reset gpio to be configured.
)

Thanks,
NeilBrown

---

NeilBrown (2):
      mmc: core: use card->ocr when negotiating voltage setting in mmc_sdio_power_restore
      mmc: core: reset sdio card properly on resume.


 drivers/mmc/core/sdio.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--
Signature

[PATCH 1/2] mmc: core: use card->ocr when negotiating voltage setting in mmc_sdio_power_restore

[NeilBrown]

As we are restoring power to a known card, it makes sense to use
the 'ocr' value known for the card rather than the generic one
for the host interface.
This matches the use of card->ocr passed to mmc_power_up in
mmc_sdio_runtime_resume  (just before mmc_sdio_power_restore is
called), and the value passed to mmc_sdio_init_card() a little
later in mmc_sdio_power_restore().

Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/core/sdio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index e636d9e99e4a..3c0f07961fab 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -1028,7 +1028,7 @@ static int mmc_sdio_power_restore(struct mmc_host *host)
 
 	sdio_reset(host);
 	mmc_go_idle(host);
-	mmc_send_if_cond(host, host->ocr_avail);
+	mmc_send_if_cond(host, host->card->ocr);
 
 	ret = mmc_send_io_op_cond(host, 0, NULL);
 	if (ret)

[PATCH 2/2] mmc: core: reset sdio card properly on resume.

[NeilBrown]

mmc_sdio_power_restore() calls
	mmc_send_if_cond(host, host->card->ocr);

	ret = mmc_send_io_op_cond(host, 0, NULL);

between mmc_go_idle() and mmc_sdio_init_card().
mmc_sdio_resume() needs to as well, else my libertas sdio wifi
device doesn't resume properly from suspend.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/core/sdio.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 3c0f07961fab..da927c61bf47 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -981,8 +981,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) {
 		sdio_reset(host);
 		mmc_go_idle(host);
-		err = mmc_sdio_init_card(host, host->card->ocr, host->card,
-					mmc_card_keep_power(host));
+		mmc_send_if_cond(host, host->card->ocr);
+		err = mmc_send_io_op_cond(host, 0, NULL);
+		if (!err)
+			err = mmc_sdio_init_card(host, host->card->ocr,
+						 host->card,
+						 mmc_card_keep_power(host));
 	} else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
 		/* We may have switched to 1-bit mode during suspend */
 		err = sdio_enable_4bit_bus(host->card);

Re: [PATCH 0/2] mmc: core: power-on sdio card correctly.

[Ulf Hansson]

On 12 November 2014 04:55, NeilBrown <neilb@suse.de> wrote:
> These two patches replace my previous patch
>   [PATCH 2/2] mmc: core: reset sdio card properly on resume.
>
> The only change is to use host->card->ocr instead of
> host->ocr_avail, as recommend by Ulf.
>
> These restore my libertas  wifi device properly on system
> resume.
>
> (I'm still hoping for a reply to my other patch:
>    [PATCH 1/2] mmc: core: allow a reset gpio to be configured.

Working on a reply. Get back to you soon. :-)

> )
>
> Thanks,
> NeilBrown
>
> ---
>
> NeilBrown (2):
>       mmc: core: use card->ocr when negotiating voltage setting in mmc_sdio_power_restore
>       mmc: core: reset sdio card properly on resume.

Thanks! Applied for next!

Kind regards
Uffe

>
>
>  drivers/mmc/core/sdio.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> --
> Signature
>

[PATCH 0/2] mmc: improve power-on for sdio wifi card.

[NeilBrown]

These two patches improve power-on management for my 'libertas'
wifi card.

On my board the card shares a regulator with a bluetooth device, so
turning off the regulator may not powercycle the card.  To get it to a
clean state it is necessary to hold the reset line down while enabling
the regulator.

So the first patch add a reset-gpio function for all sdio cards
which is configured through devicetree.

The sequence of commands sent to the sdio device for power-up differs
between runtime power resume (which works nicely) and system-suspend
resume (which doesn't).
The second patch add to calls to make these sequences the same and
allowed my device to work reliably after system suspend (though the
libertas driver needs a bit of work before it is completely reliable).

Thanks,
NeilBrown
---

NeilBrown (2):
      mmc: core: allow a reset gpio to be configured.
      mmc: core: reset sdio card properly on resume.


 Documentation/devicetree/bindings/mmc/mmc.txt |    3 +
 drivers/mmc/core/core.c                       |    3 +
 drivers/mmc/core/host.c                       |   12 ++++
 drivers/mmc/core/sdio.c                       |    8 ++-
 drivers/mmc/core/slot-gpio.c                  |   70 +++++++++++++++++++++++++
 include/linux/mmc/slot-gpio.h                 |    4 +
 6 files changed, 97 insertions(+), 3 deletions(-)

--
Signature

[PATCH 2/2] mmc: core: reset sdio card properly on resume.

[NeilBrown]

mmc_sdio_power_restore calls
	mmc_send_if_cond(host, host->ocr_avail);

	ret = mmc_send_io_op_cond(host, 0, NULL);

between mmc_go_idle() and  mmc_sdio_init_card().
mmc_sdio_resume needs to as well, else my libertas sdio wifi
device doesn't resume properly from suspend.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/mmc/core/sdio.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index e636d9e99e4a..3f069a6f448f 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -981,8 +981,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) {
 		sdio_reset(host);
 		mmc_go_idle(host);
-		err = mmc_sdio_init_card(host, host->card->ocr, host->card,
-					mmc_card_keep_power(host));
+		mmc_send_if_cond(host, host->ocr_avail);
+		err = mmc_send_io_op_cond(host, 0, NULL);
+		if (!err)
+			err = mmc_sdio_init_card(host, host->card->ocr,
+						 host->card,
+						 mmc_card_keep_power(host));
 	} else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
 		/* We may have switched to 1-bit mode during suspend */
 		err = sdio_enable_4bit_bus(host->card);

Re: [PATCH 2/2] mmc: core: reset sdio card properly on resume.

[Ulf Hansson]

On 8 November 2014 01:14, NeilBrown <neilb@suse.de> wrote:
> mmc_sdio_power_restore calls
>         mmc_send_if_cond(host, host->ocr_avail);
>
>         ret = mmc_send_io_op_cond(host, 0, NULL);
>
> between mmc_go_idle() and  mmc_sdio_init_card().
> mmc_sdio_resume needs to as well, else my libertas sdio wifi
> device doesn't resume properly from suspend.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/mmc/core/sdio.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index e636d9e99e4a..3f069a6f448f 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -981,8 +981,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) {
>                 sdio_reset(host);
>                 mmc_go_idle(host);
> -               err = mmc_sdio_init_card(host, host->card->ocr, host->card,
> -                                       mmc_card_keep_power(host));
> +               mmc_send_if_cond(host, host->ocr_avail);

/s /host->ocr_avail /host->card->ocr

I would expect that to work. I do realize that "host->ocr_avail" is
being used in the ->power_restore() callback, but I think that's wrong
as well. Could you maybe verify that changing to host->card->ocr works
in this path as well? That's would of course be a separate patch.

> +               err = mmc_send_io_op_cond(host, 0, NULL);
> +               if (!err)
> +                       err = mmc_sdio_init_card(host, host->card->ocr,
> +                                                host->card,
> +                                                mmc_card_keep_power(host));
>         } else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
>                 /* We may have switched to 1-bit mode during suspend */
>                 err = sdio_enable_4bit_bus(host->card);
>
>

Kind regards
Uffe

Re: [PATCH 2/2] mmc: core: reset sdio card properly on resume.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2518 bytes --]

On Mon, 10 Nov 2014 14:29:19 +0100 Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On 8 November 2014 01:14, NeilBrown <neilb@suse.de> wrote:
> > mmc_sdio_power_restore calls
> >         mmc_send_if_cond(host, host->ocr_avail);
> >
> >         ret = mmc_send_io_op_cond(host, 0, NULL);
> >
> > between mmc_go_idle() and  mmc_sdio_init_card().
> > mmc_sdio_resume needs to as well, else my libertas sdio wifi
> > device doesn't resume properly from suspend.
> >
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/mmc/core/sdio.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> > index e636d9e99e4a..3f069a6f448f 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -981,8 +981,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
> >         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) {
> >                 sdio_reset(host);
> >                 mmc_go_idle(host);
> > -               err = mmc_sdio_init_card(host, host->card->ocr, host->card,
> > -                                       mmc_card_keep_power(host));
> > +               mmc_send_if_cond(host, host->ocr_avail);
> 
> /s /host->ocr_avail /host->card->ocr
> 
> I would expect that to work. I do realize that "host->ocr_avail" is
> being used in the ->power_restore() callback, but I think that's wrong
> as well. Could you maybe verify that changing to host->card->ocr works
> in this path as well? That's would of course be a separate patch.

Hi Ulf,
 I made the substitution in mmc_sdio_resume and mmc_sdio_power_restore
and my sdio device continues to work, including after resume.

The  host->card->ocr value is 0x10000, the host->ocr_avail value is 0x18000.

If you like I'll send the two patches formally, but it might be a day or so.

Thanks,
NeilBrown


> 
> > +               err = mmc_send_io_op_cond(host, 0, NULL);
> > +               if (!err)
> > +                       err = mmc_sdio_init_card(host, host->card->ocr,
> > +                                                host->card,
> > +                                                mmc_card_keep_power(host));
> >         } else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
> >                 /* We may have switched to 1-bit mode during suspend */
> >                 err = sdio_enable_4bit_bus(host->card);
> >
> >
> 
> Kind regards
> Uffe


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH 2/2] mmc: core: reset sdio card properly on resume.

[Ulf Hansson]

On 11 November 2014 01:17, NeilBrown <neilb@suse.de> wrote:
> On Mon, 10 Nov 2014 14:29:19 +0100 Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>> On 8 November 2014 01:14, NeilBrown <neilb@suse.de> wrote:
>> > mmc_sdio_power_restore calls
>> >         mmc_send_if_cond(host, host->ocr_avail);
>> >
>> >         ret = mmc_send_io_op_cond(host, 0, NULL);
>> >
>> > between mmc_go_idle() and  mmc_sdio_init_card().
>> > mmc_sdio_resume needs to as well, else my libertas sdio wifi
>> > device doesn't resume properly from suspend.
>> >
>> > Signed-off-by: NeilBrown <neilb@suse.de>
>> > ---
>> >  drivers/mmc/core/sdio.c |    8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> > index e636d9e99e4a..3f069a6f448f 100644
>> > --- a/drivers/mmc/core/sdio.c
>> > +++ b/drivers/mmc/core/sdio.c
>> > @@ -981,8 +981,12 @@ static int mmc_sdio_resume(struct mmc_host *host)
>> >         if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) {
>> >                 sdio_reset(host);
>> >                 mmc_go_idle(host);
>> > -               err = mmc_sdio_init_card(host, host->card->ocr, host->card,
>> > -                                       mmc_card_keep_power(host));
>> > +               mmc_send_if_cond(host, host->ocr_avail);
>>
>> /s /host->ocr_avail /host->card->ocr
>>
>> I would expect that to work. I do realize that "host->ocr_avail" is
>> being used in the ->power_restore() callback, but I think that's wrong
>> as well. Could you maybe verify that changing to host->card->ocr works
>> in this path as well? That's would of course be a separate patch.
>
> Hi Ulf,
>  I made the substitution in mmc_sdio_resume and mmc_sdio_power_restore
> and my sdio device continues to work, including after resume.
>
> The  host->card->ocr value is 0x10000, the host->ocr_avail value is 0x18000.

That makes sense.

The host->card->ocr mask has been negotiated at initialization between
the host and the card.

>
> If you like I'll send the two patches formally, but it might be a day or so.

Please do.

Kind regards
Uffe