[PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc()

[Doug Anderson]

Ulf,

On Mon, Mar 16, 2015 at 7:05 AM, Ulf Hansson <ulf.hansson at linaro.org> wrote:
>> +       switch (ios->signal_voltage) {
>> +       case MMC_SIGNAL_VOLTAGE_120:
>> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
>> +                       1200000, 100000);
>
> Is 1V the lowest possible value? How did you get to that?

I think you've added a zero in your mind and not realized that I'm
calling regulator_set_voltage_tol() here and in other calls.  Please
read the above as:

* Try to set the voltage to exactly 1,200,000 uV (1.2V).
* If you can't get 1.2V exactly, a tolerance ("tol") of 100,000 uV
(.1V) is OK.
* In other words, 1.1V - 1.3V are OK, but aim for 1.2V


>> +       case MMC_SIGNAL_VOLTAGE_180:
>> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
>> +                       1800000, 100000);
>
> Is 1V the lowest possible value? How did you get to that?

Again, check my zeros.  This should be 1.7 - 1.9V, aiming for 1.8V.


>> +       case MMC_SIGNAL_VOLTAGE_330:
>> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
>> +                       regulator_get_voltage(mmc->supply.vmmc), 300000);
>
> Why 3V? Shouldn't it be 2.7V? How will else those SoC that for example
> supports 2.9V only work?

This will get us within .3V of whatever vmmc is.  If vmmc is 3.3V, it
will allow vqmmc of 3.0V - 3.6V.

This _seems_ sane to me and given any sane system design we should be
fine here, I think.  I can't see someone designing a system where
vqmmc was not within .3V of vmmc, can you?  If we think someone will
actually build a system where vmmc is 3.3V and vqmmc can't go higher
than 2.7V then we'll either need to increase the tolerance here or add
a new asymmetric system call like my original patches did.

>>  int mmc_regulator_get_supply(struct mmc_host *mmc);
>
> One more thought,s as for the vmmc regulator we have a
> "regulator_enabled" member in the mmc_host. Should we add a similar
> member for vqmmc? That would prevent host drivers from keeping track
> of this state themselves.

Yeah, that does sound nice.  Are you suggesting that I modify this
patch or submit a new one.  Let me know.


-Doug