diff mbox

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

Message ID 1426112117-18220-2-git-send-email-dianders@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Doug Anderson March 11, 2015, 10:15 p.m. UTC
This adds logic to the MMC core to set VQMMC.  This is expected to be
called by MMC drivers like dw_mmc as part of (or instead of) their
start_signal_voltage_switch() callback.

A few notes:

* When setting the signal voltage to 3.3V we do our best to make VQMMC
  and VMMC match.  It's been reported that this makes some old cards
  happy since they were tested back in the day before UHS when VQMMC
  and VMMC were provided by the same regulator.  A nice side effect of
  this is that we don't end up on the hairy edge of VQMMC (2.7V),
  which some EEs claim is a little too close to the minimum for
  comfort.

* When setting the signal voltage to 1.8V or 1.2V we aim for that
  specific voltage instead of picking the lowest one in the range.

* We very purposely don't print errors in mmc_regulator_set_vqmmc().
  There are cases where the MMC core will try several different
  voltages and we don't want to pollute the logs.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
---
Changes in v4: None
Changes in v3: None
Changes in v2:
- Now use existing regulator_set_voltage_tol() function.

 drivers/mmc/core/core.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |  7 +++++++
 2 files changed, 58 insertions(+)

Comments

Ulf Hansson March 16, 2015, 2:05 p.m. UTC | #1
On 11 March 2015 at 23:15, Doug Anderson <dianders@chromium.org> wrote:
> This adds logic to the MMC core to set VQMMC.  This is expected to be
> called by MMC drivers like dw_mmc as part of (or instead of) their
> start_signal_voltage_switch() callback.
>
> A few notes:
>
> * When setting the signal voltage to 3.3V we do our best to make VQMMC
>   and VMMC match.  It's been reported that this makes some old cards
>   happy since they were tested back in the day before UHS when VQMMC
>   and VMMC were provided by the same regulator.  A nice side effect of
>   this is that we don't end up on the hairy edge of VQMMC (2.7V),
>   which some EEs claim is a little too close to the minimum for
>   comfort.
>
> * When setting the signal voltage to 1.8V or 1.2V we aim for that
>   specific voltage instead of picking the lowest one in the range.
>
> * We very purposely don't print errors in mmc_regulator_set_vqmmc().
>   There are cases where the MMC core will try several different
>   voltages and we don't want to pollute the logs.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
> ---
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Now use existing regulator_set_voltage_tol() function.
>
>  drivers/mmc/core/core.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h |  7 +++++++
>  2 files changed, 58 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 23f10f7..1d3b84e 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1394,6 +1394,57 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  }
>  EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
>
> +static int mmc_regulator_set_voltage_if_supported(struct regulator *regulator,
> +                                                 int new_uV, int tol_uV)
> +{
> +       /*
> +        * Check if supported first to avoid errors since we may try several
> +        * signal levels during power up and don't want to show errors.
> +        */
> +       if (!regulator_is_supported_voltage_tol(regulator, new_uV, tol_uV))
> +               return -EINVAL;
> +
> +       return regulator_set_voltage_tol(regulator, new_uV, tol_uV);
> +}
> +
> +/**
> + * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
> + *
> + * For 3.3V signaling, we try to match VQMMC to VMMC as closely as possible.
> + * That will match the behavior of old boards where VQMMC and VMMC were supplied
> + * by the same supply.  The Bus Operating conditions for 3.3V signaling in the
> + * SD card spec also define VQMMC in terms of VMMC.
> + *
> + * For 1.2V and 1.8V signaling we'll try to get as close as possible to the
> + * requested voltage.  This is definitely a good idea for UHS where there's a
> + * separate regulator on the card that's trying to make 1.8V and it's best if
> + * we match.
> + *
> + * This function is expected to be used by a controller's
> + * start_signal_voltage_switch() function.
> + */
> +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> +       /* If no vqmmc supply then we can't change the voltage */
> +       if (IS_ERR(mmc->supply.vqmmc))
> +               return -EINVAL;
> +
> +       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?

> +       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?

> +       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?

> +       default:
> +               return -EINVAL;
> +       }
> +}
> +EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
> +
>  #endif /* CONFIG_REGULATOR */
>
>  int mmc_regulator_get_supply(struct mmc_host *mmc)
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 0c8cbe5..edd7d59 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -416,6 +416,7 @@ int mmc_regulator_get_ocrmask(struct regulator *supply);
>  int mmc_regulator_set_ocr(struct mmc_host *mmc,
>                         struct regulator *supply,
>                         unsigned short vdd_bit);
> +int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
>  #else
>  static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
>  {
> @@ -428,6 +429,12 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  {
>         return 0;
>  }
> +
> +static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
> +                                         struct mmc_ios *ios)
> +{
> +       return -EINVAL;
> +}
>  #endif
>
>  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.

Kind regards
Uffe

> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> 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 March 16, 2015, 3:12 p.m. UTC | #2
Ulf,

On Mon, Mar 16, 2015 at 7:05 AM, Ulf Hansson <ulf.hansson@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
Ulf Hansson March 17, 2015, 10:23 a.m. UTC | #3
On 16 March 2015 at 16:12, Doug Anderson <dianders@chromium.org> wrote:
> Ulf,
>
> On Mon, Mar 16, 2015 at 7:05 AM, Ulf Hansson <ulf.hansson@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:

Hehe, you are absolutely right.

>
> * 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

So what happens in the case when 1.3V and 1.1V, but not 1.2V. Which
value will be used? Is that algorithm defined by the regulator core or
does it depend per regulator implementation?

>
>
>>> +       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.

I know about SoC that supports 3.4V vmmc and 2.9V vqmmc.

What I think we need is the option to have a policy here. We need to
allow voltage levels stated by the spec and at the same time try chose
the one best suited. That's not being accomplished here.

Moreover, I wonder whether it's okay (from spec perspective) to have
vqmmc at a higher voltage level than vmmc. I don't think that's
allowed, but I might be wrong.

>
>>>  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.

Yes, please add the option as well. It's seems like it will belongs to
this code.

>
>
> -Doug

Kind regards
Uffe
Mark Brown March 17, 2015, 10:38 a.m. UTC | #4
On Tue, Mar 17, 2015 at 11:23:33AM +0100, Ulf Hansson wrote:
> On 16 March 2015 at 16:12, Doug Anderson <dianders@chromium.org> wrote:

> > * 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

> So what happens in the case when 1.3V and 1.1V, but not 1.2V. Which
> value will be used? Is that algorithm defined by the regulator core or
> does it depend per regulator implementation?

It's done in the core.  It first tries to hit the target voltage to the
maximum (picking the lowest voltage in that range) then tries to pick
the lowest voltage to the target, though that's an implementation detail
and we really should be trying to get as close as possible to the
target.  We don't do that yet because it can be expensive to work out so
we do the current thing which is cheap and mostly good enough.
Ulf Hansson March 17, 2015, 11:28 a.m. UTC | #5
On 17 March 2015 at 11:38, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Mar 17, 2015 at 11:23:33AM +0100, Ulf Hansson wrote:
>> On 16 March 2015 at 16:12, Doug Anderson <dianders@chromium.org> wrote:
>
>> > * 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
>
>> So what happens in the case when 1.3V and 1.1V, but not 1.2V. Which
>> value will be used? Is that algorithm defined by the regulator core or
>> does it depend per regulator implementation?
>
> It's done in the core.  It first tries to hit the target voltage to the
> maximum (picking the lowest voltage in that range) then tries to pick
> the lowest voltage to the target, though that's an implementation detail
> and we really should be trying to get as close as possible to the
> target.  We don't do that yet because it can be expensive to work out so
> we do the current thing which is cheap and mostly good enough.

Okay, so that seems to work well for our 1.1V->1.3V case.

Thanks!

Kind regards
Uffe
Doug Anderson March 19, 2015, 4:09 a.m. UTC | #6
Ulf,

On Tue, Mar 17, 2015 at 3:23 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> 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.
>
> I know about SoC that supports 3.4V vmmc and 2.9V vqmmc.
>
> What I think we need is the option to have a policy here. We need to
> allow voltage levels stated by the spec and at the same time try chose
> the one best suited. That's not being accomplished here.
>
> Moreover, I wonder whether it's okay (from spec perspective) to have
> vqmmc at a higher voltage level than vmmc. I don't think that's
> allowed, but I might be wrong.

OK, so sounds like I need to add a regulator_set_voltage_tol2()
function that takes in an upper tolerance and a lower.  We can use the
same rough implementation in the core we have today (if Mark is OK
with that) with regulator_set_voltage_tol() but just allow it to be
asymmetric.

From what I see in the spec for 3.3V cards are supposed to react to a
high signal that is .625 * VDD - VDD + .3


I might not be able to get to this till next week, though...

-Doug
Ulf Hansson March 19, 2015, 11:14 a.m. UTC | #7
On 19 March 2015 at 05:09, Doug Anderson <dianders@chromium.org> wrote:
> Ulf,
>
> On Tue, Mar 17, 2015 at 3:23 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> 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.
>>
>> I know about SoC that supports 3.4V vmmc and 2.9V vqmmc.
>>
>> What I think we need is the option to have a policy here. We need to
>> allow voltage levels stated by the spec and at the same time try chose
>> the one best suited. That's not being accomplished here.
>>
>> Moreover, I wonder whether it's okay (from spec perspective) to have
>> vqmmc at a higher voltage level than vmmc. I don't think that's
>> allowed, but I might be wrong.
>
> OK, so sounds like I need to add a regulator_set_voltage_tol2()
> function that takes in an upper tolerance and a lower.  We can use the
> same rough implementation in the core we have today (if Mark is OK
> with that) with regulator_set_voltage_tol() but just allow it to be
> asymmetric.

Agree. Moreover we need that API to also pick the closest value to
target, when trying the range "target->minimum". I also believe it
would be good to allow both upper and lower limits to be zero.

If Mark disagrees with this approach, we will have to deal with all
the magic inside the mmc core layer. Very much similar how
mmc_regulator_get_ocrmask() is implemented.

Kind regards
Uffe

>
> From what I see in the spec for 3.3V cards are supposed to react to a
> high signal that is .625 * VDD - VDD + .3
>
>
> I might not be able to get to this till next week, though...
>
> -Doug
Mark Brown March 19, 2015, 11:36 a.m. UTC | #8
On Thu, Mar 19, 2015 at 12:14:11PM +0100, Ulf Hansson wrote:

> Agree. Moreover we need that API to also pick the closest value to
> target, when trying the range "target->minimum". I also believe it

The implementation *should* do that anyway, it's just not trivial to
implement in an efficient fashion with the current information we have
from drivers.

> would be good to allow both upper and lower limits to be zero.

The lower limit can be zero already though it isn't clear to me that
this is useful.  Setting an upper limit of zero seems nonsensical, an
upper limit that is lower than the lower limit isn't terribly obvious
and removing the upper limit isn't safe - it means that we'll happily
oversupply things which is a road to physical damage.
Ulf Hansson March 20, 2015, 10:55 a.m. UTC | #9
On 19 March 2015 at 12:36, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Mar 19, 2015 at 12:14:11PM +0100, Ulf Hansson wrote:
>
>> Agree. Moreover we need that API to also pick the closest value to
>> target, when trying the range "target->minimum". I also believe it
>
> The implementation *should* do that anyway, it's just not trivial to
> implement in an efficient fashion with the current information we have
> from drivers.

The APIs regulator_count_voltages() and regulator_list_voltage(), are
currently used from the mmc core to find out which voltages that is
supported (with 0.1V granularity). Then that information can be used
when trying to set a new voltage.

But I guess such a wrapper API is out of the question?

Anyway, I get the feeling that we will need to do the same for this case.

>
>> would be good to allow both upper and lower limits to be zero.
>
> The lower limit can be zero already though it isn't clear to me that
> this is useful.  Setting an upper limit of zero seems nonsensical, an
> upper limit that is lower than the lower limit isn't terribly obvious
> and removing the upper limit isn't safe - it means that we'll happily
> oversupply things which is a road to physical damage.

I am not sure I follow here. In the regulator_set_voltage_tol() you
can only specifiy one limit (tolerance?). What Dough proposed was to
add a new API which can have both a low tolerance value and a high
tolerance value.

Kind regards
Uffe
Mark Brown March 20, 2015, 11:28 a.m. UTC | #10
On Fri, Mar 20, 2015 at 11:55:50AM +0100, Ulf Hansson wrote:
> On 19 March 2015 at 12:36, Mark Brown <broonie@kernel.org> wrote:

> > The implementation *should* do that anyway, it's just not trivial to
> > implement in an efficient fashion with the current information we have
> > from drivers.

> The APIs regulator_count_voltages() and regulator_list_voltage(), are
> currently used from the mmc core to find out which voltages that is
> supported (with 0.1V granularity). Then that information can be used
> when trying to set a new voltage.

> But I guess such a wrapper API is out of the question?

> Anyway, I get the feeling that we will need to do the same for this case.

As previously discussed the problem is that there can be a *lot* of
voltages on a modern regulator with fine grained voltage steps and
tolerances are also used for things like cpufreq where we care about
performance.  We need something that doesn't require a linear scan of
possible values.

> >> would be good to allow both upper and lower limits to be zero.

> > The lower limit can be zero already though it isn't clear to me that
> > this is useful.  Setting an upper limit of zero seems nonsensical, an
> > upper limit that is lower than the lower limit isn't terribly obvious
> > and removing the upper limit isn't safe - it means that we'll happily
> > oversupply things which is a road to physical damage.

> I am not sure I follow here. In the regulator_set_voltage_tol() you
> can only specifiy one limit (tolerance?). What Dough proposed was to
> add a new API which can have both a low tolerance value and a high
> tolerance value.

Tolerances are not limits - when you say "limit" that sounds like a hard
value.  I can't see any reason why the code would prevent anyone setting
a tolerance of zero, though it should be rare that this is actually the
best thing to do.
Doug Anderson April 7, 2015, 8:05 p.m. UTC | #11
Hi,

On Fri, Mar 20, 2015 at 4:28 AM, Mark Brown <broonie@kernel.org> wrote:
> As previously discussed the problem is that there can be a *lot* of
> voltages on a modern regulator with fine grained voltage steps and
> tolerances are also used for things like cpufreq where we care about
> performance.  We need something that doesn't require a linear scan of
> possible values.

Finally getting back to this (sorry for the delay!).  I tried
modifying my patches to keep using the simple implementation of
regulator_set_voltage_tol() but to take two tolerances (lower and
upper).  ...but when I thought about it I decided it wasn't enough.  I
think that doing a proper implementation of
regulator_set_voltage_tol() is going to be a requirement for getting
the MMC core changes posted.

Specifically, I think the right implementation for the MMC core's 3.3V
signaling is something like this:

  /*
   * Bus operating conditions say that card should accept input
   * between (0.625 * VDD) and (VDD + 0.3), so we'll use those
   * as tolerances.
   */
  return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
       vmmc_voltage, vmmc_voltage * 375 / 1000, 300000);

Ulf says he has a board where vmmc is 3.4V and the max vqmmc is 2.9V.
If we think about that board, we'll end up calling
regulator_set_voltage_tol() with a lower/upper tolerance of 1.275V and
.3V.

Extending the current simple implementation of
regulator_set_voltage_tol() to take an upper and lower, that will
translate to first trying to set the voltage to 3.4V - 3.7V, which
will fail.  We'll then try to set the voltage to 2.125V - 3.7V.
Presumably that will end up picking 2.125V, which is really non-ideal
compared to 2.9V and it seems likely to cause some cards to start
failing.

Mark: I know you said you were considering writing a better
regulator_set_voltage_tol() yourself, but I don't know if you've
already started work on it.

I'm expecting to maybe have time to take a crack at it in a few weeks
if you haven't already done it by then.

Thanks!

-Doug
Mark Brown April 8, 2015, 11:28 a.m. UTC | #12
On Tue, Apr 07, 2015 at 01:05:43PM -0700, Doug Anderson wrote:

> Mark: I know you said you were considering writing a better
> regulator_set_voltage_tol() yourself, but I don't know if you've
> already started work on it.

> I'm expecting to maybe have time to take a crack at it in a few weeks
> if you haven't already done it by then.

If there's nothing on the list I won't have done anything.
diff mbox

Patch

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 23f10f7..1d3b84e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1394,6 +1394,57 @@  int mmc_regulator_set_ocr(struct mmc_host *mmc,
 }
 EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
 
+static int mmc_regulator_set_voltage_if_supported(struct regulator *regulator,
+						  int new_uV, int tol_uV)
+{
+	/*
+	 * Check if supported first to avoid errors since we may try several
+	 * signal levels during power up and don't want to show errors.
+	 */
+	if (!regulator_is_supported_voltage_tol(regulator, new_uV, tol_uV))
+		return -EINVAL;
+
+	return regulator_set_voltage_tol(regulator, new_uV, tol_uV);
+}
+
+/**
+ * mmc_regulator_set_vqmmc - Set VQMMC as per the ios
+ *
+ * For 3.3V signaling, we try to match VQMMC to VMMC as closely as possible.
+ * That will match the behavior of old boards where VQMMC and VMMC were supplied
+ * by the same supply.  The Bus Operating conditions for 3.3V signaling in the
+ * SD card spec also define VQMMC in terms of VMMC.
+ *
+ * For 1.2V and 1.8V signaling we'll try to get as close as possible to the
+ * requested voltage.  This is definitely a good idea for UHS where there's a
+ * separate regulator on the card that's trying to make 1.8V and it's best if
+ * we match.
+ *
+ * This function is expected to be used by a controller's
+ * start_signal_voltage_switch() function.
+ */
+int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	/* If no vqmmc supply then we can't change the voltage */
+	if (IS_ERR(mmc->supply.vqmmc))
+		return -EINVAL;
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_120:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+			1200000, 100000);
+	case MMC_SIGNAL_VOLTAGE_180:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+			1800000, 100000);
+	case MMC_SIGNAL_VOLTAGE_330:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+			regulator_get_voltage(mmc->supply.vmmc), 300000);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
+
 #endif /* CONFIG_REGULATOR */
 
 int mmc_regulator_get_supply(struct mmc_host *mmc)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0c8cbe5..edd7d59 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -416,6 +416,7 @@  int mmc_regulator_get_ocrmask(struct regulator *supply);
 int mmc_regulator_set_ocr(struct mmc_host *mmc,
 			struct regulator *supply,
 			unsigned short vdd_bit);
+int mmc_regulator_set_vqmmc(struct mmc_host *mmc, struct mmc_ios *ios);
 #else
 static inline int mmc_regulator_get_ocrmask(struct regulator *supply)
 {
@@ -428,6 +429,12 @@  static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
 {
 	return 0;
 }
+
+static inline int mmc_regulator_set_vqmmc(struct mmc_host *mmc,
+					  struct mmc_ios *ios)
+{
+	return -EINVAL;
+}
 #endif
 
 int mmc_regulator_get_supply(struct mmc_host *mmc);