[PATCH] mmc: core: restore ocr and operation voltage in resume

[PATCH] mmc: core: restore ocr and operation voltage in resume

[Kevin Liu]

host->ocr has been reset in power off but not restored after power up
in resume. And operation voltage will be set to the highest after resume
back. This patch fix these two bugs.

Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/core/core.c  |    3 ++-
 include/linux/mmc/host.h |    1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 08a3cf2..b8c3d41 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1523,6 +1523,7 @@ void mmc_power_off(struct mmc_host *host)
 	 * Reset ocr mask to be the highest possible voltage supported for
 	 * this mmc host. This value will be used at next power up.
 	 */
+	host->ocr_bak = host->ocr;
 	host->ocr = 1 << (fls(host->ocr_avail) - 1);
 
 	if (!mmc_host_is_spi(host)) {
@@ -2666,7 +2667,7 @@ int mmc_resume_host(struct mmc_host *host)
 	if (host->bus_ops && !host->bus_dead) {
 		if (!mmc_card_keep_power(host)) {
 			mmc_power_up(host);
-			mmc_select_voltage(host, host->ocr);
+			host->ocr = mmc_select_voltage(host, host->ocr_bak);
 			/*
 			 * Tell runtime PM core we just powered up the card,
 			 * since it still believes the card is powered off.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index d6f20cc..6e355d1 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -309,6 +309,7 @@ struct mmc_host {
 
 	struct mmc_ios		ios;		/* current io bus settings */
 	u32			ocr;		/* the current OCR setting */
+	u32			ocr_bak;	/* save current OCR setting */
 
 	/* group bitfields together to minimize padding */
 	unsigned int		use_spi_crc:1;
-- 
1.7.9.5

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Prasanna NAVARATNA]

Kevin Liu <kliu5 <at> marvell.com> writes:

> 
> host->ocr has been reset in power off but not restored after power up
> in resume. And operation voltage will be set to the highest after resume
> back. This patch fix these two bugs.
> 
> Signed-off-by: Kevin Liu <kliu5 <at> marvell.com>
> ---
>  drivers/mmc/core/core.c  |    3 ++-
>  include/linux/mmc/host.h |    1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 08a3cf2..b8c3d41 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
>  <at>  <at>  -1523,6 +1523,7  <at>  <at>  void mmc_power_off(struct 
mmc_host *host)
>  	 * Reset ocr mask to be the highest possible voltage supported for
>  	 * this mmc host. This value will be used at next power up.
>  	 */
> +	host->ocr_bak = host->ocr;
>  	host->ocr = 1 << (fls(host->ocr_avail) - 1);
> 
>  	if (!mmc_host_is_spi(host)) {
>  <at>  <at>  -2666,7 +2667,7  <at>  <at>  int mmc_resume_host(struct 
mmc_host *host)
>  	if (host->bus_ops && !host->bus_dead) {
>  		if (!mmc_card_keep_power(host)) {
>  			mmc_power_up(host);
> -			mmc_select_voltage(host, host->ocr);
> +			host->ocr = mmc_select_voltage(host, host->ocr_bak);
>  			/*
>  			 * Tell runtime PM core we just powered up the card,
>  			 * since it still believes the card is powered off.
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index d6f20cc..6e355d1 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
>  <at>  <at>  -309,6 +309,7  <at>  <at>  struct mmc_host {
> 
>  	struct mmc_ios		ios;		/* current io bus settings 
*/
>  	u32			ocr;		/* the current OCR setting 
*/
> +	u32			ocr_bak;	/* save current OCR setting 
*/
> 
>  	/* group bitfields together to minimize padding */
>  	unsigned int		use_spi_crc:1;


Hello Kevin,

Any update on this patch? 

I have seen an issue where during initialization, core will get card's ocr 
and match with host ocr available and select the appropriate voltage (3.0V)
But during suspend, maximum supported voltage from host is saved back and 
when resuming back requesting for highest voltage of host (3.3V).
It doesn't care about card's OCR.

This patch fixes it. Looks valid for me.

Thanks & Regards,
Prasanna NAVARATNA

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Ulf Hansson]

On 4 April 2013 11:44, Prasanna NAVARATNA <prasanna.navaratna@gmail.com> wrote:
> Kevin Liu <kliu5 <at> marvell.com> writes:
>
>>
>> host->ocr has been reset in power off but not restored after power up
>> in resume. And operation voltage will be set to the highest after resume
>> back. This patch fix these two bugs.
>>
>> Signed-off-by: Kevin Liu <kliu5 <at> marvell.com>
>> ---
>>  drivers/mmc/core/core.c  |    3 ++-
>>  include/linux/mmc/host.h |    1 +
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 08a3cf2..b8c3d41 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>>  <at>  <at>  -1523,6 +1523,7  <at>  <at>  void mmc_power_off(struct
> mmc_host *host)
>>        * Reset ocr mask to be the highest possible voltage supported for
>>        * this mmc host. This value will be used at next power up.
>>        */
>> +     host->ocr_bak = host->ocr;
>>       host->ocr = 1 << (fls(host->ocr_avail) - 1);
>>
>>       if (!mmc_host_is_spi(host)) {
>>  <at>  <at>  -2666,7 +2667,7  <at>  <at>  int mmc_resume_host(struct
> mmc_host *host)
>>       if (host->bus_ops && !host->bus_dead) {
>>               if (!mmc_card_keep_power(host)) {
>>                       mmc_power_up(host);
>> -                     mmc_select_voltage(host, host->ocr);
>> +                     host->ocr = mmc_select_voltage(host, host->ocr_bak);
>>                       /*
>>                        * Tell runtime PM core we just powered up the card,
>>                        * since it still believes the card is powered off.
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index d6f20cc..6e355d1 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>>  <at>  <at>  -309,6 +309,7  <at>  <at>  struct mmc_host {
>>
>>       struct mmc_ios          ios;            /* current io bus settings
> */
>>       u32                     ocr;            /* the current OCR setting
> */
>> +     u32                     ocr_bak;        /* save current OCR setting
> */
>>
>>       /* group bitfields together to minimize padding */
>>       unsigned int            use_spi_crc:1;
>
>
> Hello Kevin,
>
> Any update on this patch?
>
> I have seen an issue where during initialization, core will get card's ocr
> and match with host ocr available and select the appropriate voltage (3.0V)

You may think this is done correct, but it is not. Host drivers does
in general not change voltage in MMC_POWER_ON state, which is what is
required if the new negotiated ocr mask shall be used.  According to
the mmc/sd spec voltage level changes requires a full power cycle
sequence which is not done from the protocol layer, and which is why
host drivers also should not consider changing voltages at
MMC_POWER_ON state.

Note that debugfs is then also not telling the truth about the voltage
level being used.

> But during suspend, maximum supported voltage from host is saved back and
> when resuming back requesting for highest voltage of host (3.3V).
> It doesn't care about card's OCR.

In practice, it is same voltage level that has been used all the time,
except for those host driver that do change levels in MMC_POWER_ON
mode.


Kind regards
Ulf Hansson

>
> This patch fixes it. Looks valid for me.
>
> Thanks & Regards,
> Prasanna NAVARATNA
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Prasanna NAVARATNA]

Ulf Hansson <ulf.hansson <at> linaro.org> writes:

> 
> On 4 April 2013 11:44, Prasanna NAVARATNA <prasanna.navaratna <at> 
gmail.com> wrote:
> > Kevin Liu <kliu5 <at> marvell.com> writes:
> >
> >>
> >> host->ocr has been reset in power off but not restored after power up
> >> in resume. And operation voltage will be set to the highest after 
resume
> >> back. This patch fix these two bugs.
> >>
> >> Signed-off-by: Kevin Liu <kliu5 <at> marvell.com>
> >> ---
> >>  drivers/mmc/core/core.c  |    3 ++-
> >>  include/linux/mmc/host.h |    1 +
> >>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> index 08a3cf2..b8c3d41 100644
> >> --- a/drivers/mmc/core/core.c
> >> +++ b/drivers/mmc/core/core.c
> >>  <at>  <at>  -1523,6 +1523,7  <at>  <at>  void mmc_power_off(struct
> > mmc_host *host)
> >>        * Reset ocr mask to be the highest possible voltage supported 
for
> >>        * this mmc host. This value will be used at next power up.
> >>        */
> >> +     host->ocr_bak = host->ocr;
> >>       host->ocr = 1 << (fls(host->ocr_avail) - 1);
> >>
> >>       if (!mmc_host_is_spi(host)) {
> >>  <at>  <at>  -2666,7 +2667,7  <at>  <at>  int mmc_resume_host(struct
> > mmc_host *host)
> >>       if (host->bus_ops && !host->bus_dead) {
> >>               if (!mmc_card_keep_power(host)) {
> >>                       mmc_power_up(host);
> >> -                     mmc_select_voltage(host, host->ocr);
> >> +                     host->ocr = mmc_select_voltage(host, host-
>ocr_bak);
> >>                       /*
> >>                        * Tell runtime PM core we just powered up the 
card,
> >>                        * since it still believes the card is powered 
off.
> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> >> index d6f20cc..6e355d1 100644
> >> --- a/include/linux/mmc/host.h
> >> +++ b/include/linux/mmc/host.h
> >>  <at>  <at>  -309,6 +309,7  <at>  <at>  struct mmc_host {
> >>
> >>       struct mmc_ios          ios;            /* current io bus 
settings
> > */
> >>       u32                     ocr;            /* the current OCR 
setting
> > */
> >> +     u32                     ocr_bak;        /* save current OCR 
setting
> > */
> >>
> >>       /* group bitfields together to minimize padding */
> >>       unsigned int            use_spi_crc:1;
> >
> >
> > Hello Kevin,
> >
> > Any update on this patch?
> >
> > I have seen an issue where during initialization, core will get card's 
ocr
> > and match with host ocr available and select the appropriate voltage 
(3.0V)
> 
> You may think this is done correct, but it is not. Host drivers does
> in general not change voltage in MMC_POWER_ON state, which is what is
> required if the new negotiated ocr mask shall be used.  According to
> the mmc/sd spec voltage level changes requires a full power cycle
> sequence which is not done from the protocol layer, and which is why
> host drivers also should not consider changing voltages at
> MMC_POWER_ON state.
> 
> Note that debugfs is then also not telling the truth about the voltage
> level being used.
> 
> > But during suspend, maximum supported voltage from host is saved back 
and
> > when resuming back requesting for highest voltage of host (3.3V).
> > It doesn't care about card's OCR.
> 
> In practice, it is same voltage level that has been used all the time,
> except for those host driver that do change levels in MMC_POWER_ON
> mode.
> 
> Kind regards
> Ulf Hansson
> 
> >
> > This patch fixes it. Looks valid for me.
> >
> > Thanks & Regards,
> > Prasanna NAVARATNA
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majordomo <at> vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
I agree host shouldn't change voltages during MMC_POWER_ON state, but if 
core calls set_ios, with different voltage for the regulator, then voltage 
will be changed accordingly.
Consider this case where I am observing the issue :-

During initialization, (struct mmc_host *host) "host->ocr_avail" is updated 
with, what IP supports. (Maximum up to 3.3V)

mmc core will query the card OCR status :-
err = mmc_send_app_op_cond(host, 0, &ocr);
and this ocr is used to select voltage
host->ocr = mmc_select_voltage(host, ocr);
In mmc_select_voltage,
ocr &= host->ocr_avail; // Fair consideration done
ios.vdd = ffs (ocr) - 1
So ideally, this will consider card's OCR and what host supports and will 
choose appropriate voltage.

But during suspend, in mmc_power_off, ocr mask is reseted to be the highest 
possible voltage supported for the mmc_host.
host->ocr = 1 << (fls(host->ocr_avail) - 1);  // which is 3.3V

While resuming back, in mmc_power_up, you'll restore back the highest 
supported voltage for this mmc_host and request to set for that value(using 
set_ios).
Then mmc_select_voltage is called with highest supported voltage of host and 
not with card's OCR. It discards the card's OCR completely! (it will use 
saved host->ocr during suspend, where max 3.3V is set)

Don't you think, it must consider the card's OCR too during resume as done 
during initialization?
Please correct, if any of my assumption is wrong.

Thanks & Regards,
Prasanna NAVARATNA

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Ulf Hansson]

On 4 April 2013 12:43, Prasanna NAVARATNA <prasanna.navaratna@gmail.com> wrote:
> Ulf Hansson <ulf.hansson <at> linaro.org> writes:
>
>>
>> On 4 April 2013 11:44, Prasanna NAVARATNA <prasanna.navaratna <at>
> gmail.com> wrote:
>> > Kevin Liu <kliu5 <at> marvell.com> writes:
>> >
>> >>
>> >> host->ocr has been reset in power off but not restored after power up
>> >> in resume. And operation voltage will be set to the highest after
> resume
>> >> back. This patch fix these two bugs.
>> >>
>> >> Signed-off-by: Kevin Liu <kliu5 <at> marvell.com>
>> >> ---
>> >>  drivers/mmc/core/core.c  |    3 ++-
>> >>  include/linux/mmc/host.h |    1 +
>> >>  2 files changed, 3 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> >> index 08a3cf2..b8c3d41 100644
>> >> --- a/drivers/mmc/core/core.c
>> >> +++ b/drivers/mmc/core/core.c
>> >>  <at>  <at>  -1523,6 +1523,7  <at>  <at>  void mmc_power_off(struct
>> > mmc_host *host)
>> >>        * Reset ocr mask to be the highest possible voltage supported
> for
>> >>        * this mmc host. This value will be used at next power up.
>> >>        */
>> >> +     host->ocr_bak = host->ocr;
>> >>       host->ocr = 1 << (fls(host->ocr_avail) - 1);
>> >>
>> >>       if (!mmc_host_is_spi(host)) {
>> >>  <at>  <at>  -2666,7 +2667,7  <at>  <at>  int mmc_resume_host(struct
>> > mmc_host *host)
>> >>       if (host->bus_ops && !host->bus_dead) {
>> >>               if (!mmc_card_keep_power(host)) {
>> >>                       mmc_power_up(host);
>> >> -                     mmc_select_voltage(host, host->ocr);
>> >> +                     host->ocr = mmc_select_voltage(host, host-
>>ocr_bak);
>> >>                       /*
>> >>                        * Tell runtime PM core we just powered up the
> card,
>> >>                        * since it still believes the card is powered
> off.
>> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> >> index d6f20cc..6e355d1 100644
>> >> --- a/include/linux/mmc/host.h
>> >> +++ b/include/linux/mmc/host.h
>> >>  <at>  <at>  -309,6 +309,7  <at>  <at>  struct mmc_host {
>> >>
>> >>       struct mmc_ios          ios;            /* current io bus
> settings
>> > */
>> >>       u32                     ocr;            /* the current OCR
> setting
>> > */
>> >> +     u32                     ocr_bak;        /* save current OCR
> setting
>> > */
>> >>
>> >>       /* group bitfields together to minimize padding */
>> >>       unsigned int            use_spi_crc:1;
>> >
>> >
>> > Hello Kevin,
>> >
>> > Any update on this patch?
>> >
>> > I have seen an issue where during initialization, core will get card's
> ocr
>> > and match with host ocr available and select the appropriate voltage
> (3.0V)
>>
>> You may think this is done correct, but it is not. Host drivers does
>> in general not change voltage in MMC_POWER_ON state, which is what is
>> required if the new negotiated ocr mask shall be used.  According to
>> the mmc/sd spec voltage level changes requires a full power cycle
>> sequence which is not done from the protocol layer, and which is why
>> host drivers also should not consider changing voltages at
>> MMC_POWER_ON state.
>>
>> Note that debugfs is then also not telling the truth about the voltage
>> level being used.
>>
>> > But during suspend, maximum supported voltage from host is saved back
> and
>> > when resuming back requesting for highest voltage of host (3.3V).
>> > It doesn't care about card's OCR.
>>
>> In practice, it is same voltage level that has been used all the time,
>> except for those host driver that do change levels in MMC_POWER_ON
>> mode.
>>
>> Kind regards
>> Ulf Hansson
>>
>> >
>> > This patch fixes it. Looks valid for me.
>> >
>> > Thanks & Regards,
>> > Prasanna NAVARATNA
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> > the body of a message to majordomo <at> vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo <at> vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> I agree host shouldn't change voltages during MMC_POWER_ON state, but if
> core calls set_ios, with different voltage for the regulator, then voltage
> will be changed accordingly.
> Consider this case where I am observing the issue :-
>
> During initialization, (struct mmc_host *host) "host->ocr_avail" is updated
> with, what IP supports. (Maximum up to 3.3V)
>
> mmc core will query the card OCR status :-
> err = mmc_send_app_op_cond(host, 0, &ocr);
> and this ocr is used to select voltage
> host->ocr = mmc_select_voltage(host, ocr);
> In mmc_select_voltage,
> ocr &= host->ocr_avail; // Fair consideration done
> ios.vdd = ffs (ocr) - 1
> So ideally, this will consider card's OCR and what host supports and will
> choose appropriate voltage.
>
> But during suspend, in mmc_power_off, ocr mask is reseted to be the highest
> possible voltage supported for the mmc_host.
> host->ocr = 1 << (fls(host->ocr_avail) - 1);  // which is 3.3V
>
> While resuming back, in mmc_power_up, you'll restore back the highest
> supported voltage for this mmc_host and request to set for that value(using
> set_ios).
> Then mmc_select_voltage is called with highest supported voltage of host and
> not with card's OCR. It discards the card's OCR completely! (it will use
> saved host->ocr during suspend, where max 3.3V is set)
>
> Don't you think, it must consider the card's OCR too during resume as done
> during initialization?
> Please correct, if any of my assumption is wrong.

You are definitely correct, the ocr mask must not be reset during
suspend. But, the next problem is much more complex to solve; mmc core
should tell the host driver to use the properly negotiated ocr mask to
set the voltage level. Both at initialization and at resume. This
patch does not solve anything in this regards.

Kind regards
Ulf Hansson

>
> Thanks & Regards,
> Prasanna NAVARATNA
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Prasanna NAVARATNA]

So you agree the ocr mask must not be reset during suspend.

The patch over here exactly addresses the problem which you stated :-
"mmc core should tell the host driver to use the properly negotiated ocr 
mask to set the voltage level, both at initialization and at resume"

The patch saves the ocr which is properly negotiated during initialization 
time and then restores it back during resume.

Will you please elaborate, what is wrong with this patch? Or are you trying 
to tell, this is not the proper way to do the same?

Thanks & Regards,
Prasanna NAVARATNA

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Prasanna NAVARATNA]

Prasanna NAVARATNA <prasanna.navaratna <at> gmail.com> writes:

> 
> So you agree the ocr mask must not be reset during suspend.
> 
> The patch over here exactly addresses the problem which you stated :-
> "mmc core should tell the host driver to use the properly negotiated ocr 
> mask to set the voltage level, both at initialization and at resume"
> 
> The patch saves the ocr which is properly negotiated during initialization 
> time and then restores it back during resume.
> 
> Will you please elaborate, what is wrong with this patch? Or are you 
trying 
> to tell, this is not the proper way to do the same?
> 
> Thanks & Regards,
> Prasanna NAVARATNA
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Hello,

I have suggested another patch which does the same thing in better way.
Please refer to the new post "mmc: core: negotiate ocr during resume"

Regards,
Prasanna NAVARATNA

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Ulf Hansson]

On 4 April 2013 16:43, Prasanna NAVARATNA <prasanna.navaratna@gmail.com> wrote:
> Prasanna NAVARATNA <prasanna.navaratna <at> gmail.com> writes:
>
>>
>> So you agree the ocr mask must not be reset during suspend.
>>
>> The patch over here exactly addresses the problem which you stated :-
>> "mmc core should tell the host driver to use the properly negotiated ocr
>> mask to set the voltage level, both at initialization and at resume"
>>
>> The patch saves the ocr which is properly negotiated during initialization
>> time and then restores it back during resume.
>>
>> Will you please elaborate, what is wrong with this patch? Or are you
> trying
>> to tell, this is not the proper way to do the same?

"host->ocr" is intended for keeping the negotiated ocr mask. Another
variable is not needed.

A proper patch would in principle remove the workaround, which means
the reset of the "host->ocr" in mmc_power_off is not done any more.

Exactly what you need to make this work, is as stated before a bit
more complicated.

>>
>> Thanks & Regards,
>> Prasanna NAVARATNA
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo <at> vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>
> Hello,
>
> I have suggested another patch which does the same thing in better way.
> Please refer to the new post "mmc: core: negotiate ocr during resume"
>
> Regards,
> Prasanna NAVARATNA
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Prasanna NAVARATNA]

Ulf Hansson <ulf.hansson <at> linaro.org> writes:

> "host->ocr" is intended for keeping the negotiated ocr mask. Another
> variable is not needed.
> 
> A proper patch would in principle remove the workaround, which means
> the reset of the "host->ocr" in mmc_power_off is not done any more.
> 
> Exactly what you need to make this work, is as stated before a bit
> more complicated.

Hello Ulf,

The work around is introduced by yourself :-
"At power off, reset OCR mask to be the highest possible voltage
supported for the current mmc host.

This solves the re-initialization during the power up sequence.
The voltage may have been decreased due to the card accepts a lower
voltage than the voltage used during the initialization sequence.
We need to reset the voltage to by the host highest possible value
since according to _specification_ the initialization must always be
done at high voltage"

So do you think, solution to current issue is to revert this patch? This 
will not violate the spec as you are mentioning above?
If you agree, i'll push a new patch by reverting this. Else consider another 
way in my new post "mmc: core: negotiate ocr during resume"
This patch will follow the spec and will also negotiate ocr during 
mmc_resume_host.

Thanks,
Prasanna NAVARATNA

Re: [PATCH] mmc: core: restore ocr and operation voltage in resume

[Ulf Hansson]

On 5 April 2013 08:07, Prasanna NAVARATNA <prasanna.navaratna@gmail.com> wrote:
> Ulf Hansson <ulf.hansson <at> linaro.org> writes:
>
>> "host->ocr" is intended for keeping the negotiated ocr mask. Another
>> variable is not needed.
>>
>> A proper patch would in principle remove the workaround, which means
>> the reset of the "host->ocr" in mmc_power_off is not done any more.
>>
>> Exactly what you need to make this work, is as stated before a bit
>> more complicated.
>
> Hello Ulf,
>
> The work around is introduced by yourself :-
> "At power off, reset OCR mask to be the highest possible voltage
> supported for the current mmc host.
>
> This solves the re-initialization during the power up sequence.
> The voltage may have been decreased due to the card accepts a lower
> voltage than the voltage used during the initialization sequence.
> We need to reset the voltage to by the host highest possible value
> since according to _specification_ the initialization must always be
> done at high voltage"
>
> So do you think, solution to current issue is to revert this patch? This
> will not violate the spec as you are mentioning above?

The solution is not to revert, but to make a proper patch which solves
the initialization issues and the suspend/resume issues.

If you just revert the patch you will break suspend/resume sequence
for a many SoC I believe.

> If you agree, i'll push a new patch by reverting this. Else consider another
> way in my new post "mmc: core: negotiate ocr during resume"
> This patch will follow the spec and will also negotiate ocr during
> mmc_resume_host.
>
> Thanks,
> Prasanna NAVARATNA
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html