diff mbox

mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

Message ID 1415678573-6093-1-git-send-email-addy.ke@rock-chips.com
State New, archived
Headers show

Commit Message

addy ke Nov. 11, 2014, 4:02 a.m. UTC
SD2.0 cards need vqmmc and vmmc to be the same.
But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.
So if we set vmmc 3.3V in dt table, we will get error information as follows:

[   17.785398] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
50000000Hz, actual 50000000HZ div = 0)
[   17.795175] mmc1: new high speed SDHC card at address e624
[   17.801283] mmcblk1: mmc1:e624 SU08G 7.40 GiB
[   17.816033]  mmcblk1: p1
[   17.839318] mmcblk1: error -110 sending status command, retrying
[   17.845363] mmcblk1: error -115 sending stop command, original cmd
response 0x900, card status 0x800b00
[   17.854758] mmcblk1: error -84 transferring data, sector 32, nr 24,
cmd response 0x900, card status 0xb00
[   17.864328] mmcblk1: retrying using single block read
[   17.873647] mmcblk1: error -110 sending status command, retrying
[   17.879660] mmcblk1: error -84 transferring data, sector 44, nr 12,
cmd response 0x900, card status 0x0
[   17.889051] end_request: I/O error, dev mmcblk1, sector 44
[   17.895594] Buffer I/O error on device mmcblk1, logical block 5
[   17.902484] mmcblk1: error -110 sending status command, retrying
[   17.908498] mmcblk1: error -84 transferring data, sector 50, nr 6,
cmd response 0x900, card status 0x0
[   17.917802] end_request: I/O error, dev mmcblk1, sector 50
[   17.924984] Buffer I/O error on device mmcblk1, logical block 6
[   18.431258] mmc_host mmc1: Timeout sending command (cmd 0x200000 arg
0x0 status 0x80200000)

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
 drivers/mmc/host/dw_mmc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Ulf Hansson Nov. 11, 2014, 8:52 a.m. UTC | #1
On 11 November 2014 05:02, Addy Ke <addy.ke@rock-chips.com> wrote:
> SD2.0 cards need vqmmc and vmmc to be the same.

No, that's not correct.

If I remember the spec correctly, the bus signal threshold is 0.75 * VDD.

> But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.

I guess you want to do that to save as much power as possible.

> So if we set vmmc 3.3V in dt table, we will get error information as follows:
>
> [   17.785398] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
> 50000000Hz, actual 50000000HZ div = 0)
> [   17.795175] mmc1: new high speed SDHC card at address e624
> [   17.801283] mmcblk1: mmc1:e624 SU08G 7.40 GiB
> [   17.816033]  mmcblk1: p1
> [   17.839318] mmcblk1: error -110 sending status command, retrying
> [   17.845363] mmcblk1: error -115 sending stop command, original cmd
> response 0x900, card status 0x800b00
> [   17.854758] mmcblk1: error -84 transferring data, sector 32, nr 24,
> cmd response 0x900, card status 0xb00
> [   17.864328] mmcblk1: retrying using single block read
> [   17.873647] mmcblk1: error -110 sending status command, retrying
> [   17.879660] mmcblk1: error -84 transferring data, sector 44, nr 12,
> cmd response 0x900, card status 0x0
> [   17.889051] end_request: I/O error, dev mmcblk1, sector 44
> [   17.895594] Buffer I/O error on device mmcblk1, logical block 5
> [   17.902484] mmcblk1: error -110 sending status command, retrying
> [   17.908498] mmcblk1: error -84 transferring data, sector 50, nr 6,
> cmd response 0x900, card status 0x0
> [   17.917802] end_request: I/O error, dev mmcblk1, sector 50
> [   17.924984] Buffer I/O error on device mmcblk1, logical block 6
> [   18.431258] mmc_host mmc1: Timeout sending command (cmd 0x200000 arg
> 0x0 status 0x80200000)
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index b4c3044..a8b70b5 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>          */
>         uhs = mci_readl(host, UHS_REG);
>         if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> -               min_uv = 2700000;
> -               max_uv = 3600000;
> +               /* try pick the exact same voltage as vmmc for vqmmc */

This seems like a generic SD protocol issue.

Should we maybe provide some helper function from the mmc core, which
in principle take the negotiated card->ocr into account while
calculating the signal voltage level. Typically min_uv should be 0.75
x (card->ocr), for these cases.

Comments?

> +               if (!IS_ERR(mmc->supply.vmmc)) {
> +                       min_uv = regulator_get_voltage(mmc->supply.vmmc);
> +                       max_uv = min_uv;
> +               } else {
> +                       min_uv = 2700000;
> +                       max_uv = 3600000;
> +               }
>                 uhs &= ~v18;
>         } else {
>                 min_uv = 1700000;
> --
> 1.8.3.2
>
>

Kind regards
Uffe
Doug Anderson Nov. 12, 2014, 6:04 p.m. UTC | #2
Ulf,

On Tue, Nov 11, 2014 at 12:52 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 11 November 2014 05:02, Addy Ke <addy.ke@rock-chips.com> wrote:
>> SD2.0 cards need vqmmc and vmmc to be the same.
>
> No, that's not correct.
>
> If I remember the spec correctly, the bus signal threshold is 0.75 * VDD.

As usual, I will first state my utter lack of knowledge of all things mmc.

Now that's out of the way, on two separate board with two separate
SoCs I've heard stories of cards that don't work when there's a big
gap between vmmc and vqmmc.

If my memory serves, previously I heard of problems with vmmc=3.3V and
vqmmc=2.8V.  That means there were problems with .85 * VDD.  Certainly
Addy seems to have a card that has problems with vmmc=3.3V and
vqmmc=2.7V (but worked with vmmc=3.3V and vqmmc=2.8V).  That is .82 *
VDD.

I have no idea if these old cards are "to spec", but they exist and it
would be nice to support them.

It seems like the absolute safest thing would be to try to keep vmmc
and vqmmc matching if possible, especially during card probe.  Once
voltage negotiation happened then the vqmmc could go down.


>> But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.
>
> I guess you want to do that to save as much power as possible.

I don't think it's Addy wanting it, I think it's the regulator framework.

If a regulator is current 1.8V and you request 2.7 - 3.3V, the
framework needs to pick one of those voltages.  I believe it will pick
2.7V.

...so I think we get into trouble only when the 2.0 card is plugged in
after a UHS card has negotiated down the voltage, but I could be
wrong.  Maybe Addy can clarify.


>> @@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>          */
>>         uhs = mci_readl(host, UHS_REG);
>>         if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
>> -               min_uv = 2700000;
>> -               max_uv = 3600000;
>> +               /* try pick the exact same voltage as vmmc for vqmmc */
>
> This seems like a generic SD protocol issue.
>
> Should we maybe provide some helper function from the mmc core, which
> in principle take the negotiated card->ocr into account while
> calculating the signal voltage level. Typically min_uv should be 0.75
> x (card->ocr), for these cases.

Yes, if there are ways to make the solution more generic I would
certainly support that.

-Doug
addy ke Nov. 13, 2014, 2:19 a.m. UTC | #3
On 2014/11/13 02:04, Doug Anderson wrote:
> Ulf,
> 
> On Tue, Nov 11, 2014 at 12:52 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 11 November 2014 05:02, Addy Ke <addy.ke@rock-chips.com> wrote:
>>> SD2.0 cards need vqmmc and vmmc to be the same.
>>
>> No, that's not correct.
>>
>> If I remember the spec correctly, the bus signal threshold is 0.75 * VDD.
> 
> As usual, I will first state my utter lack of knowledge of all things mmc.
> 
> Now that's out of the way, on two separate board with two separate
> SoCs I've heard stories of cards that don't work when there's a big
> gap between vmmc and vqmmc.
> 
> If my memory serves, previously I heard of problems with vmmc=3.3V and
> vqmmc=2.8V.  That means there were problems with .85 * VDD.  Certainly
> Addy seems to have a card that has problems with vmmc=3.3V and
> vqmmc=2.7V (but worked with vmmc=3.3V and vqmmc=2.8V).  That is .82 *
> VDD.
> 
> I have no idea if these old cards are "to spec", but they exist and it
> would be nice to support them.
> 
> It seems like the absolute safest thing would be to try to keep vmmc
> and vqmmc matching if possible, especially during card probe.  Once
> voltage negotiation happened then the vqmmc could go down.
> 
> 
>>> But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.
>>
>> I guess you want to do that to save as much power as possible.
> 
> I don't think it's Addy wanting it, I think it's the regulator framework.
> 
> If a regulator is current 1.8V and you request 2.7 - 3.3V, the
> framework needs to pick one of those voltages.  I believe it will pick
> 2.7V.
> 
> ...so I think we get into trouble only when the 2.0 card is plugged in
> after a UHS card has negotiated down the voltage, but I could be
> wrong.  Maybe Addy can clarify.
> 
Sure
If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
and card can be identified. But if UHS card is pulgged in first, the vqmmc will be down to 1.8v.

when sd2.0 card is pulgged in, mmc core will call dw_mci_switch_voltage to change vqmmc to 3.3v
(MMC_SINGLE_VOTAGE_330). So vqmmc will be set 2.7v, if we request 2.7-3.6v.

But vmmc is always 3.3v,becuase it be set min_volt = max_volt = 3.3v in dt tables.

So the result:
vmmc = 3.3v and vqmmc = 2.7v, and sd2.0 card is failed to identify in my test.

> 
>>> @@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>>          */
>>>         uhs = mci_readl(host, UHS_REG);
>>>         if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
>>> -               min_uv = 2700000;
>>> -               max_uv = 3600000;
>>> +               /* try pick the exact same voltage as vmmc for vqmmc */
>>
>> This seems like a generic SD protocol issue.
>>
>> Should we maybe provide some helper function from the mmc core, which
>> in principle take the negotiated card->ocr into account while
>> calculating the signal voltage level. Typically min_uv should be 0.75
>> x (card->ocr), for these cases.
> 
> Yes, if there are ways to make the solution more generic I would
> certainly support that.
> 
> -Doug
> 
> 
>
Ulf Hansson Nov. 21, 2014, 12:06 p.m. UTC | #4
[...]

> Sure
> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,

That can't be right. mmc_power_up() should trigger
dw_mci_switch_voltage() to be invoked.

> and card can be identified. But if UHS card is pulgged in first, the vqmmc will be down to 1.8v.
>
> when sd2.0 card is pulgged in, mmc core will call dw_mci_switch_voltage to change vqmmc to 3.3v
> (MMC_SINGLE_VOTAGE_330). So vqmmc will be set 2.7v, if we request 2.7-3.6v.
>
> But vmmc is always 3.3v,becuase it be set min_volt = max_volt = 3.3v in dt tables.
>
> So the result:
> vmmc = 3.3v and vqmmc = 2.7v, and sd2.0 card is failed to identify in my test.

Hmm. I wonder why it works the first time? Could it be that your
regulator have voltage ramp up time that isn't properly considered?

Kind regards
Uffe
Jaehoon Chung Nov. 21, 2014, 12:29 p.m. UTC | #5
On 11/21/2014 09:06 PM, Ulf Hansson wrote:
> [...]
> 
>> Sure
>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
> 
> That can't be right. mmc_power_up() should trigger
> dw_mci_switch_voltage() to be invoked.

Since dw_mci_switch_voltage() is invoked, voltage is changed from 1.8v to 2.7v (minimum value 2.7-3.6v), isn't?

> 
>> and card can be identified. But if UHS card is pulgged in first, the vqmmc will be down to 1.8v.
>>
>> when sd2.0 card is pulgged in, mmc core will call dw_mci_switch_voltage to change vqmmc to 3.3v
>> (MMC_SINGLE_VOTAGE_330). So vqmmc will be set 2.7v, if we request 2.7-3.6v.
>>
>> But vmmc is always 3.3v,becuase it be set min_volt = max_volt = 3.3v in dt tables.

vmmc is fixed voltage?

>>
>> So the result:
>> vmmc = 3.3v and vqmmc = 2.7v, and sd2.0 card is failed to identify in my test.
> 
> Hmm. I wonder why it works the first time? Could it be that your
> regulator have voltage ramp up time that isn't properly considered?

if oscilloscope can use, can we know more exactly?

Best Regards,
Jaehoon Chung
> 
> Kind regards
> Uffe
> --
> 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
>
Doug Anderson Nov. 21, 2014, 5:42 p.m. UTC | #6
Ulf,

On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]
>
>> Sure
>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>
> That can't be right. mmc_power_up() should trigger
> dw_mci_switch_voltage() to be invoked.

Hmmm, I think you're right.  Addy: can you double check if it's only
the 2nd card for you?  I was thinking that if a regulator is currently
3.3V and you request 2.7 - 3.3V the regulator framework will treat
that as a noop.  ...but that definitely doesn't appear to be the case.
When I boot up the first time even with no SD card plugged in, I see
this at bootup:

[    3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV

...showing that it started at 3.3V.  Then I see:

$ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
/sys/class/regulator/regulator.16/name:vccio_sd
/sys/class/regulator/regulator.16/microvolts:2700000

...so it is certainly getting changed even with no card plugged in.


BTW: I don't actually have one of these failing cards--all of mine
work.  Addy, do you know the make and model of the card you have that
fails?


-Doug
Doug Anderson Nov. 21, 2014, 9:04 p.m. UTC | #7
Hi,

On Fri, Nov 21, 2014 at 9:42 AM, Doug Anderson <dianders@chromium.org> wrote:
> Ulf,
>
> On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> [...]
>>
>>> Sure
>>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>>
>> That can't be right. mmc_power_up() should trigger
>> dw_mci_switch_voltage() to be invoked.
>
> Hmmm, I think you're right.  Addy: can you double check if it's only
> the 2nd card for you?  I was thinking that if a regulator is currently
> 3.3V and you request 2.7 - 3.3V the regulator framework will treat
> that as a noop.  ...but that definitely doesn't appear to be the case.
> When I boot up the first time even with no SD card plugged in, I see
> this at bootup:
>
> [    3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV
>
> ...showing that it started at 3.3V.  Then I see:
>
> $ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
> /sys/class/regulator/regulator.16/name:vccio_sd
> /sys/class/regulator/regulator.16/microvolts:2700000
>
> ...so it is certainly getting changed even with no card plugged in.
>
>
> BTW: I don't actually have one of these failing cards--all of mine
> work.  Addy, do you know the make and model of the card you have that
> fails?

Just as a bit of a followup, I did some more digging...

1. It looks as if we now have a bit of "opposite" logic for vmmc vs.
vqmmc.  In mmc_power_up() I see that it sets the initial voltage as:

  host->ios.vdd = fls(ocr) - 1;

That actually means that we're going to pick the maximum voltage for
vmmc (of the supported voltages).  For vqmmc dw_mmc is using the
regulator framework which (as described in my previous message) will
pick the minimum.

2. Several people I've talked to have expressed concerns that our
minimum value is 2.7V.  Apparently that's really on the edge and makes
EEs a little nervous.  The quick sample of cards sitting on my desk
shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.


Both of the above make me feel like dw_mmc should try its best to pick
a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
That also happens to make us work exactly like hosts where vmmc and
vqmmc are supplied by the same supply.


-Doug
Ulf Hansson Nov. 24, 2014, 1:29 p.m. UTC | #8
On 21 November 2014 at 22:04, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Fri, Nov 21, 2014 at 9:42 AM, Doug Anderson <dianders@chromium.org> wrote:
>> Ulf,
>>
>> On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> [...]
>>>
>>>> Sure
>>>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>>>
>>> That can't be right. mmc_power_up() should trigger
>>> dw_mci_switch_voltage() to be invoked.
>>
>> Hmmm, I think you're right.  Addy: can you double check if it's only
>> the 2nd card for you?  I was thinking that if a regulator is currently
>> 3.3V and you request 2.7 - 3.3V the regulator framework will treat
>> that as a noop.  ...but that definitely doesn't appear to be the case.
>> When I boot up the first time even with no SD card plugged in, I see
>> this at bootup:
>>
>> [    3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV
>>
>> ...showing that it started at 3.3V.  Then I see:
>>
>> $ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
>> /sys/class/regulator/regulator.16/name:vccio_sd
>> /sys/class/regulator/regulator.16/microvolts:2700000
>>
>> ...so it is certainly getting changed even with no card plugged in.
>>
>>
>> BTW: I don't actually have one of these failing cards--all of mine
>> work.  Addy, do you know the make and model of the card you have that
>> fails?
>
> Just as a bit of a followup, I did some more digging...
>
> 1. It looks as if we now have a bit of "opposite" logic for vmmc vs.
> vqmmc.  In mmc_power_up() I see that it sets the initial voltage as:
>
>   host->ios.vdd = fls(ocr) - 1;

That's because we would like to supports as many cards as possible.
The policy is based upon that some cards may not support lower
voltages, but most will support higher.

>
> That actually means that we're going to pick the maximum voltage for
> vmmc (of the supported voltages).  For vqmmc dw_mmc is using the
> regulator framework which (as described in my previous message) will
> pick the minimum.

Correct. I have thought this has been inside spec and choosing the
lower value would be preferred to lower power consumption. Maybe we
needs to re-visit this one more time.

Here are some of the interesting sections in the eMMC spec:
10.3.3 Power supply Voltages
"The VCCQ must be defined at equal to or less than VCC".

10.5 Bus signal levels
Push-pull mode:
Voh = 0.75 * VCCQ. (Do note, its VCCQ not VCC).

Summary eMMC: VCCQ must be less and VCC, we should be inside spec.

From SD spec:
6.6.1 Threshold Level for High Voltage Range
Voh = 0.75 * VDD.

In worst case scenario, VDD = 3.6V and VIO = 2.7V. That gives as the
factor of 0.75, thus we are inside spec but without margins.

>
> 2. Several people I've talked to have expressed concerns that our
> minimum value is 2.7V.  Apparently that's really on the edge and makes
> EEs a little nervous.  The quick sample of cards sitting on my desk
> shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.

0x00ff8000 states what values of VDD levels the device supports. Not VIO.

>
>
> Both of the above make me feel like dw_mmc should try its best to pick
> a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
> That also happens to make us work exactly like hosts where vmmc and
> vqmmc are supplied by the same supply.

I do see your point. And I agree that it would be nice to achieve
something like this.

The question is how to do this. For sure, we need to involve the mmc
core to handle this correctly.

Kind regards
Uffe
addy ke Nov. 25, 2014, 2:38 a.m. UTC | #9
On Fri, Nov 24, 2014 at 9:29 PM, Ulf Hansson <ulf.hansson@linaro.org> 
wrote:

> On 21 November 2014 at 22:04, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Fri, Nov 21, 2014 at 9:42 AM, Doug Anderson <dianders@chromium.org> wrote:
>>> Ulf,
>>>
>>> On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> [...]
>>>>
>>>>> Sure
>>>>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>>>> That can't be right. mmc_power_up() should trigger
>>>> dw_mci_switch_voltage() to be invoked.
>>> Hmmm, I think you're right.  Addy: can you double check if it's only
>>> the 2nd card for you?  I was thinking that if a regulator is currently
>>> 3.3V and you request 2.7 - 3.3V the regulator framework will treat
>>> that as a noop.  ...but that definitely doesn't appear to be the case.
>>> When I boot up the first time even with no SD card plugged in, I see
>>> this at bootup:
>>>
>>> [    3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV
>>>
>>> ...showing that it started at 3.3V.  Then I see:
>>>
>>> $ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
>>> /sys/class/regulator/regulator.16/name:vccio_sd
>>> /sys/class/regulator/regulator.16/microvolts:2700000
>>>
>>> ...so it is certainly getting changed even with no card plugged in.
>>>
>>>
>>> BTW: I don't actually have one of these failing cards--all of mine
>>> work.  Addy, do you know the make and model of the card you have that
>>> fails?
>> Just as a bit of a followup, I did some more digging...
>>
>> 1. It looks as if we now have a bit of "opposite" logic for vmmc vs.
>> vqmmc.  In mmc_power_up() I see that it sets the initial voltage as:
>>
>>    host->ios.vdd = fls(ocr) - 1;
> That's because we would like to supports as many cards as possible.
> The policy is based upon that some cards may not support lower
> voltages, but most will support higher.
>
>> That actually means that we're going to pick the maximum voltage for
>> vmmc (of the supported voltages).  For vqmmc dw_mmc is using the
>> regulator framework which (as described in my previous message) will
>> pick the minimum.
> Correct. I have thought this has been inside spec and choosing the
> lower value would be preferred to lower power consumption. Maybe we
> needs to re-visit this one more time.
>
> Here are some of the interesting sections in the eMMC spec:
> 10.3.3 Power supply Voltages
> "The VCCQ must be defined at equal to or less than VCC".
>
> 10.5 Bus signal levels
> Push-pull mode:
> Voh = 0.75 * VCCQ. (Do note, its VCCQ not VCC).
>
> Summary eMMC: VCCQ must be less and VCC, we should be inside spec.
>
> >From SD spec:
> 6.6.1 Threshold Level for High Voltage Range
> Voh = 0.75 * VDD.
>
> In worst case scenario, VDD = 3.6V and VIO = 2.7V. That gives as the
> factor of 0.75, thus we are inside spec but without margins.
* From eMMC4.5 spec:
   1. (VDDF)vcc: Supply voltage for flash memory,  which is  2.7v -- 3.3v
   2. (VDD)vccq: Supply voltage for memory controller, which is  1.7v -- 
1.95v  and 2,7v -- 3.6v

* And from RK3288 datasheet:
   Digtial GPIO Power(SDMMC0_VDD --> vccq) is 3.0v -- 3.6v and 1.62v - 1.98v

So I think:
3.3v:  (2.7v < vccq < 3.6v)   &&  (3.0v < vccq < 3.6v)  ==> (3.0v < vccq 
< 3.6v)
1.8v:  (1.7v < vccq < 1.95v)  && (1.62v < vccq < 1.98v) ==> (1.7v < vccq 
< 1.95v)

and (2.7v < vcc < 3.3v)

* And according to our hardware engineer:
   All of supply voltage must have +/- 10% cushion.

* And we have found in some worse card that there is 200mv voltage 
collapse when these card is insert.

So I think the best resolution is that vcc and vccq is configurable int 
dt table.

>> 2. Several people I've talked to have expressed concerns that our
>> minimum value is 2.7V.  Apparently that's really on the edge and makes
>> EEs a little nervous.  The quick sample of cards sitting on my desk
>> shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.
> 0x00ff8000 states what values of VDD levels the device supports. Not VIO.
>
>>
>> Both of the above make me feel like dw_mmc should try its best to pick
>> a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
>> That also happens to make us work exactly like hosts where vmmc and
>> vqmmc are supplied by the same supply.
> I do see your point. And I agree that it would be nice to achieve
> something like this.
>
> The question is how to do this. For sure, we need to involve the mmc
> core to handle this correctly.
>
> Kind regards
> Uffe
>
>
>
Doug Anderson Nov. 25, 2014, 5:29 a.m. UTC | #10
Ulf,

On Mon, Nov 24, 2014 at 5:29 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> 2. Several people I've talked to have expressed concerns that our
>> minimum value is 2.7V.  Apparently that's really on the edge and makes
>> EEs a little nervous.  The quick sample of cards sitting on my desk
>> shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.
>
> 0x00ff8000 states what values of VDD levels the device supports. Not VIO.

Yup, I was just pointing out that possibly others were trying to get a
little bit of margin (not going all the way down to 2.7V) too.

>> Both of the above make me feel like dw_mmc should try its best to pick
>> a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
>> That also happens to make us work exactly like hosts where vmmc and
>> vqmmc are supplied by the same supply.
>
> I do see your point. And I agree that it would be nice to achieve
> something like this.
>
> The question is how to do this. For sure, we need to involve the mmc
> core to handle this correctly.

You could add a function to the core that we could call from
dw_mci_switch_voltage() and it would do all the work except trying to
set the UHS register.  That would certainly make it easy.  That could
try to set the highest voltage that is <= vmmc when we're at
MMC_SIGNAL_VOLTAGE_330.

-Doug
Doug Anderson Nov. 25, 2014, 5:36 a.m. UTC | #11
Addy,

On Mon, Nov 24, 2014 at 6:38 PM, Addy <addy.ke@rock-chips.com> wrote:
>> In worst case scenario, VDD = 3.6V and VIO = 2.7V. That gives as the
>> factor of 0.75, thus we are inside spec but without margins.
>
> * From eMMC4.5 spec:
>   1. (VDDF)vcc: Supply voltage for flash memory,  which is  2.7v -- 3.3v
>   2. (VDD)vccq: Supply voltage for memory controller, which is  1.7v --
> 1.95v  and 2,7v -- 3.6v
>
> * And from RK3288 datasheet:
>   Digtial GPIO Power(SDMMC0_VDD --> vccq) is 3.0v -- 3.6v and 1.62v - 1.98v
>
> So I think:
> 3.3v:  (2.7v < vccq < 3.6v)   &&  (3.0v < vccq < 3.6v)  ==> (3.0v < vccq <
> 3.6v)
> 1.8v:  (1.7v < vccq < 1.95v)  && (1.62v < vccq < 1.98v) ==> (1.7v < vccq <
> 1.95v)
>
> and (2.7v < vcc < 3.3v)
>
> * And according to our hardware engineer:
>   All of supply voltage must have +/- 10% cushion.
>
> * And we have found in some worse card that there is 200mv voltage collapse
> when these card is insert.
>
> So I think the best resolution is that vcc and vccq is configurable int dt
> table.

Ah, interesting.  ...so what we really need to be able to do is to say
that the regulator we for vqmmc have supports the ranges 3.0V - 3.3V
and 1.7V - 1.95V but not anything in between 1.95V ad 3.0V.  I have no
idea how to express that in the regulator framework.

Technically you could take the IO Voltage Domains code (responsible
for choosing the 1.8V range or the 3.3V range) and have it communicate
the requirements to the regulator framework if you could figure out
how to communicate them.


...of course if you implemented my suggestion of keeping vqmmc as the
highest voltage <= vmmc then maybe the whole point is moot and we
don't have to figure it out.  Just make sure that vmmc never goes
below 3.0V.


-Doug
Alexandru Stan Nov. 25, 2014, 9:11 p.m. UTC | #12
From what I understand... High speed SD cards have 1.8V regulators
inside them(sourced by vmmc (what we power the SD card with)). So in
terms of the SD card IO pins they will either use exactly vmmc or
1.8V. It doesn't make sense for vqmmc (the voltage we use to power the
AP block connected to the SD cards) to be anything but exactly equal
to vmmc or 1.8V. If vqmmc differs from vmmc(or 1.8V, depending on
mode) by more than a little (~100-200mV), both up or down, you start
getting leaks into the input protection diodes of the pins(either the
AP or the SD card) which is a pretty BAD thing (you're essentially
powering the sd card through the IO pins, or the SD card is powering
the IO block on the AP).

Alexandru Stan (amstan)

On Mon, Nov 24, 2014 at 9:36 PM, Doug Anderson <dianders@chromium.org> wrote:
> Addy,
>
> On Mon, Nov 24, 2014 at 6:38 PM, Addy <addy.ke@rock-chips.com> wrote:
>>> In worst case scenario, VDD = 3.6V and VIO = 2.7V. That gives as the
>>> factor of 0.75, thus we are inside spec but without margins.
>>
>> * From eMMC4.5 spec:
>>   1. (VDDF)vcc: Supply voltage for flash memory,  which is  2.7v -- 3.3v
>>   2. (VDD)vccq: Supply voltage for memory controller, which is  1.7v --
>> 1.95v  and 2,7v -- 3.6v
>>
>> * And from RK3288 datasheet:
>>   Digtial GPIO Power(SDMMC0_VDD --> vccq) is 3.0v -- 3.6v and 1.62v - 1.98v
>>
>> So I think:
>> 3.3v:  (2.7v < vccq < 3.6v)   &&  (3.0v < vccq < 3.6v)  ==> (3.0v < vccq <
>> 3.6v)
>> 1.8v:  (1.7v < vccq < 1.95v)  && (1.62v < vccq < 1.98v) ==> (1.7v < vccq <
>> 1.95v)
>>
>> and (2.7v < vcc < 3.3v)
>>
>> * And according to our hardware engineer:
>>   All of supply voltage must have +/- 10% cushion.
>>
>> * And we have found in some worse card that there is 200mv voltage collapse
>> when these card is insert.
>>
>> So I think the best resolution is that vcc and vccq is configurable int dt
>> table.
>
> Ah, interesting.  ...so what we really need to be able to do is to say
> that the regulator we for vqmmc have supports the ranges 3.0V - 3.3V
> and 1.7V - 1.95V but not anything in between 1.95V ad 3.0V.  I have no
> idea how to express that in the regulator framework.
>
> Technically you could take the IO Voltage Domains code (responsible
> for choosing the 1.8V range or the 3.3V range) and have it communicate
> the requirements to the regulator framework if you could figure out
> how to communicate them.
>
>
> ...of course if you implemented my suggestion of keeping vqmmc as the
> highest voltage <= vmmc then maybe the whole point is moot and we
> don't have to figure it out.  Just make sure that vmmc never goes
> below 3.0V.
>
>
> -Doug
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b4c3044..a8b70b5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1163,8 +1163,14 @@  static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 	 */
 	uhs = mci_readl(host, UHS_REG);
 	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
-		min_uv = 2700000;
-		max_uv = 3600000;
+		/* try pick the exact same voltage as vmmc for vqmmc */
+		if (!IS_ERR(mmc->supply.vmmc)) {
+			min_uv = regulator_get_voltage(mmc->supply.vmmc);
+			max_uv = min_uv;
+		} else {
+			min_uv = 2700000;
+			max_uv = 3600000;
+		}
 		uhs &= ~v18;
 	} else {
 		min_uv = 1700000;