diff mbox

[v2,3/8] mmc: core: Add mmc_regulator_set_vqmmc()

Message ID 1443622064-14362-4-git-send-email-heiko@sntech.de (mailing list archive)
State New, archived
Headers show

Commit Message

Heiko Stuebner Sept. 30, 2015, 2:07 p.m. UTC
From: Douglas Anderson <dianders@chromium.org>

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.
  This is done in two steps. At first we try to find a VQMMC within
  a 0.3V tolerance of VMMC and if this is not supported by the
  supplying regulator we try to find a suitable voltage within the
  whole 2.7V-3.6V area of the spec.

* The two step approach is currently necessary, as the used
  regulator_set_voltage_triplet(min, target, max) uses a simple
  implementation that just tries two basic steps:
	regulator_set_voltage(target, max);
	regulator_set_voltage(min, target);
  So with only one step with 2.7-3.6V borders, if a suitable voltage
  is a bit below VMMC, we would directly get the lowest 2.7V
  which some boards (like Rockchips) don't like at all.

* 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: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/core/core.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |  7 +++++
 2 files changed, 81 insertions(+)

Comments

Ulf Hansson Sept. 30, 2015, 2:42 p.m. UTC | #1
On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> From: Douglas Anderson <dianders@chromium.org>
>
> 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.
>   This is done in two steps. At first we try to find a VQMMC within
>   a 0.3V tolerance of VMMC and if this is not supported by the
>   supplying regulator we try to find a suitable voltage within the
>   whole 2.7V-3.6V area of the spec.
>
> * The two step approach is currently necessary, as the used
>   regulator_set_voltage_triplet(min, target, max) uses a simple
>   implementation that just tries two basic steps:
>         regulator_set_voltage(target, max);
>         regulator_set_voltage(min, target);
>   So with only one step with 2.7-3.6V borders, if a suitable voltage
>   is a bit below VMMC, we would directly get the lowest 2.7V
>   which some boards (like Rockchips) don't like at all.
>
> * 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: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

This looks good to me!

Once all are happy with the patches, can we take the mmc patches via
my mmc tree or does it all have to go together?

Kind regards
Uffe

> ---
>  drivers/mmc/core/core.c  | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h |  7 +++++
>  2 files changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 0520064..363cc98 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1437,6 +1437,80 @@ 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 min_uV, int target_uV,
> +                                                 int max_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(regulator, min_uV, max_uV))
> +               return -EINVAL;
> +
> +       return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
> +                                            max_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.
> + * If this is not possible we'll try the full 2.7-3.6V of the spec.
> + *
> + * 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)
> +{
> +       int volt, min_uV, max_uV;
> +
> +       /* 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,
> +                                               1100000, 1200000, 1300000);
> +       case MMC_SIGNAL_VOLTAGE_180:
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               1700000, 1800000, 1950000);
> +       case MMC_SIGNAL_VOLTAGE_330:
> +               volt = regulator_get_voltage(mmc->supply.vmmc);
> +               if (volt < 0)
> +                       return volt;
> +
> +               min_uV = max(volt - 300000, 2700000);
> +               max_uV = min(volt + 300000, 3600000);
> +
> +               /*
> +                * Due to a limitation in the current implementation of
> +                * regulator_set_voltage_triplet() which is taking the lowest
> +                * voltage possible if below the target, search for a suitable
> +                * voltage in two steps and try to stay close to vmmc
> +                * with a 0.3V tolerance at first.
> +                */
> +               if (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               min_uV, volt, max_uV))
> +                       return 0;
> +
> +               return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
> +                                               2700000, volt, 3600000);
> +       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 83b81fd..a2a78eb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -423,6 +423,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)
>  {
> @@ -435,6 +436,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);
> --
> 2.5.1
>
Heiko Stuebner Sept. 30, 2015, 2:55 p.m. UTC | #2
Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> > From: Douglas Anderson <dianders@chromium.org>
> > 
> > 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.
> >   This is done in two steps. At first we try to find a VQMMC within
> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >   supplying regulator we try to find a suitable voltage within the
> >   whole 2.7V-3.6V area of the spec.
> > 
> > * The two step approach is currently necessary, as the used
> > 
> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >   
> >   implementation that just tries two basic steps:
> >         regulator_set_voltage(target, max);
> >         regulator_set_voltage(min, target);
> >   
> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >   which some boards (like Rockchips) don't like at all.
> > 
> > * 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: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> This looks good to me!

very cool :-)


> Once all are happy with the patches, can we take the mmc patches via
> my mmc tree or does it all have to go together?

The clock changes of course only touch internals of the phase-clocks, so 
should have no problem going through another tree.

For the devicetree part I'm unsure. If the boards enable the tuning-related 
settings without the new voltage handling, 2.7V gets set on all Rockchip 
boards which doesn't work on those at all.

So either the dts patches would need to go into your tree, I would need a 
stable branch or we could simply postpone dts changes for the next cycle.


Heiko

> > ---
> > 
> >  drivers/mmc/core/core.c  | 74
> >  ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mmc/host.h |  7 +++++
> >  2 files changed, 81 insertions(+)
> > 
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 0520064..363cc98 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -1437,6 +1437,80 @@ 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 min_uV,
> > int target_uV, +                                                 int
> > max_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(regulator, min_uV, max_uV))
> > +               return -EINVAL;
> > +
> > +       return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
> > +                                            max_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.
> > + * If this is not possible we'll try the full 2.7-3.6V of the spec.
> > + *
> > + * 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)
> > +{
> > +       int volt, min_uV, max_uV;
> > +
> > +       /* 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, +              
> >                                 1100000, 1200000, 1300000); +       case
> > MMC_SIGNAL_VOLTAGE_180:
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 1700000, 1800000, 1950000); +       case
> > MMC_SIGNAL_VOLTAGE_330:
> > +               volt = regulator_get_voltage(mmc->supply.vmmc);
> > +               if (volt < 0)
> > +                       return volt;
> > +
> > +               min_uV = max(volt - 300000, 2700000);
> > +               max_uV = min(volt + 300000, 3600000);
> > +
> > +               /*
> > +                * Due to a limitation in the current implementation of
> > +                * regulator_set_voltage_triplet() which is taking the
> > lowest +                * voltage possible if below the target, search
> > for a suitable +                * voltage in two steps and try to stay
> > close to vmmc +                * with a 0.3V tolerance at first.
> > +                */
> > +               if
> > (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +            
> >                                   min_uV, volt, max_uV)) +               
> >        return 0;
> > +
> > +               return
> > mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc, +              
> >                                 2700000, volt, 3600000); +       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 83b81fd..a2a78eb 100644
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -423,6 +423,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)
> >  {
> > 
> > @@ -435,6 +436,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);
> > 
> > --
> > 2.5.1
Ulf Hansson Oct. 1, 2015, 9:54 a.m. UTC | #3
On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > From: Douglas Anderson <dianders@chromium.org>
>> >
>> > 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.
>> >   This is done in two steps. At first we try to find a VQMMC within
>> >   a 0.3V tolerance of VMMC and if this is not supported by the
>> >   supplying regulator we try to find a suitable voltage within the
>> >   whole 2.7V-3.6V area of the spec.
>> >
>> > * The two step approach is currently necessary, as the used
>> >
>> >   regulator_set_voltage_triplet(min, target, max) uses a simple
>> >
>> >   implementation that just tries two basic steps:
>> >         regulator_set_voltage(target, max);
>> >         regulator_set_voltage(min, target);
>> >
>> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
>> >   is a bit below VMMC, we would directly get the lowest 2.7V
>> >   which some boards (like Rockchips) don't like at all.
>> >
>> > * 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: Douglas Anderson <dianders@chromium.org>
>> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> This looks good to me!
>
> very cool :-)
>
>
>> Once all are happy with the patches, can we take the mmc patches via
>> my mmc tree or does it all have to go together?
>
> The clock changes of course only touch internals of the phase-clocks, so
> should have no problem going through another tree.

What happens if I take mmc and dt changes, wouldn't I need the clock
patches as well?

>
> For the devicetree part I'm unsure. If the boards enable the tuning-related
> settings without the new voltage handling, 2.7V gets set on all Rockchip
> boards which doesn't work on those at all.
>
> So either the dts patches would need to go into your tree, I would need a
> stable branch or we could simply postpone dts changes for the next cycle.
>

Kind regards
Uffe
Heiko Stuebner Oct. 1, 2015, 10:21 a.m. UTC | #4
Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> >> > From: Douglas Anderson <dianders@chromium.org>
> >> > 
> >> > 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.
> >> >   This is done in two steps. At first we try to find a VQMMC within
> >> >   a 0.3V tolerance of VMMC and if this is not supported by the
> >> >   supplying regulator we try to find a suitable voltage within the
> >> >   whole 2.7V-3.6V area of the spec.
> >> > 
> >> > * The two step approach is currently necessary, as the used
> >> > 
> >> >   regulator_set_voltage_triplet(min, target, max) uses a simple
> >> >   
> >> >   implementation that just tries two basic steps:
> >> >         regulator_set_voltage(target, max);
> >> >         regulator_set_voltage(min, target);
> >> >   
> >> >   So with only one step with 2.7-3.6V borders, if a suitable voltage
> >> >   is a bit below VMMC, we would directly get the lowest 2.7V
> >> >   which some boards (like Rockchips) don't like at all.
> >> > 
> >> > * 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: Douglas Anderson <dianders@chromium.org>
> >> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> 
> >> This looks good to me!
> > 
> > very cool :-)
> > 
> >> Once all are happy with the patches, can we take the mmc patches via
> >> my mmc tree or does it all have to go together?
> > 
> > The clock changes of course only touch internals of the phase-clocks, so
> > should have no problem going through another tree.
> 
> What happens if I take mmc and dt changes, wouldn't I need the clock
> patches as well?

The API stays of course the same, only the degree to settings translation gets 
optimized, so I guess in the worst case you would get no good phase and thus 
fall back to non-highspeed modes - but the system would stay running.

But of course, if the clock maintainers could Ack the two clock patches and 
everything would stay together that would work even better :-)


Heiko

> 
> > For the devicetree part I'm unsure. If the boards enable the
> > tuning-related
> > settings without the new voltage handling, 2.7V gets set on all Rockchip
> > boards which doesn't work on those at all.
> > 
> > So either the dts patches would need to go into your tree, I would need a
> > stable branch or we could simply postpone dts changes for the next cycle.
> 
> Kind regards
> Uffe
Stephen Boyd Oct. 1, 2015, 5:35 p.m. UTC | #5
On 10/01, Heiko Stübner wrote:
> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> > On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
> > > The clock changes of course only touch internals of the phase-clocks, so
> > > should have no problem going through another tree.
> > 
> > What happens if I take mmc and dt changes, wouldn't I need the clock
> > patches as well?
> 
> The API stays of course the same, only the degree to settings translation gets 
> optimized, so I guess in the worst case you would get no good phase and thus 
> fall back to non-highspeed modes - but the system would stay running.
> 
> But of course, if the clock maintainers could Ack the two clock patches and 
> everything would stay together that would work even better :-)
> 

If Ulf doesn't want to take them we can apply them to clk tree.
Otherwise, you can have my acked-by on the clk patches.
Ulf Hansson Oct. 1, 2015, 9:05 p.m. UTC | #6
On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/01, Heiko Stübner wrote:
>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>> > On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>> > > Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>> > >> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>> > > The clock changes of course only touch internals of the phase-clocks, so
>> > > should have no problem going through another tree.
>> >
>> > What happens if I take mmc and dt changes, wouldn't I need the clock
>> > patches as well?
>>
>> The API stays of course the same, only the degree to settings translation gets
>> optimized, so I guess in the worst case you would get no good phase and thus
>> fall back to non-highspeed modes - but the system would stay running.
>>
>> But of course, if the clock maintainers could Ack the two clock patches and
>> everything would stay together that would work even better :-)
>>
>
> If Ulf doesn't want to take them we can apply them to clk tree.
> Otherwise, you can have my acked-by on the clk patches.

I don't mind picking up the clock patches. So I consider this as an
ack for both patch 1 and patch2, thanks.

Now, let's give Jaehoon some time to review the dw_mmc parts.

Kind regards
Uffe
Jaehoon Chung Oct. 2, 2015, 2:52 a.m. UTC | #7
On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 10/01, Heiko Stübner wrote:
>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> wrote:
>>>>> The clock changes of course only touch internals of the phase-clocks, so
>>>>> should have no problem going through another tree.
>>>>
>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>> patches as well?
>>>
>>> The API stays of course the same, only the degree to settings translation gets
>>> optimized, so I guess in the worst case you would get no good phase and thus
>>> fall back to non-highspeed modes - but the system would stay running.
>>>
>>> But of course, if the clock maintainers could Ack the two clock patches and
>>> everything would stay together that would work even better :-)
>>>
>>
>> If Ulf doesn't want to take them we can apply them to clk tree.
>> Otherwise, you can have my acked-by on the clk patches.
> 
> I don't mind picking up the clock patches. So I consider this as an
> ack for both patch 1 and patch2, thanks.
> 
> Now, let's give Jaehoon some time to review the dw_mmc parts.

I will check other patches on today, if it's ok, i will apply at my repository.
Thanks for giving time! :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
>
Heiko Stuebner Oct. 2, 2015, 7:06 a.m. UTC | #8
Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
> > On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> On 10/01, Heiko Stübner wrote:
> >>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
> >>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
> >>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
> >>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> 
wrote:
> >>>>> The clock changes of course only touch internals of the phase-clocks,
> >>>>> so
> >>>>> should have no problem going through another tree.
> >>>> 
> >>>> What happens if I take mmc and dt changes, wouldn't I need the clock
> >>>> patches as well?
> >>> 
> >>> The API stays of course the same, only the degree to settings
> >>> translation gets optimized, so I guess in the worst case you would get
> >>> no good phase and thus fall back to non-highspeed modes - but the
> >>> system would stay running.
> >>> 
> >>> But of course, if the clock maintainers could Ack the two clock patches
> >>> and
> >>> everything would stay together that would work even better :-)
> >> 
> >> If Ulf doesn't want to take them we can apply them to clk tree.
> >> Otherwise, you can have my acked-by on the clk patches.
> > 
> > I don't mind picking up the clock patches. So I consider this as an
> > ack for both patch 1 and patch2, thanks.
> > 
> > Now, let's give Jaehoon some time to review the dw_mmc parts.
> 
> I will check other patches on today, if it's ok, i will apply at my
> repository. Thanks for giving time! :)

I think Ulf wanted to apply the whole series via the mmc tree directly, but I 
guess that is between you two to decide ;-)


Heiko
Jaehoon Chung Oct. 2, 2015, 7:53 a.m. UTC | #9
On 10/02/2015 04:06 PM, Heiko Stübner wrote:
> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> On 10/01, Heiko Stübner wrote:
>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de> 
> wrote:
>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>> so
>>>>>>> should have no problem going through another tree.
>>>>>>
>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>> patches as well?
>>>>>
>>>>> The API stays of course the same, only the degree to settings
>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>> system would stay running.
>>>>>
>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>> and
>>>>> everything would stay together that would work even better :-)
>>>>
>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>> Otherwise, you can have my acked-by on the clk patches.
>>>
>>> I don't mind picking up the clock patches. So I consider this as an
>>> ack for both patch 1 and patch2, thanks.
>>>
>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>
>> I will check other patches on today, if it's ok, i will apply at my
>> repository. Thanks for giving time! :)
> 
> I think Ulf wanted to apply the whole series via the mmc tree directly, but I 
> guess that is between you two to decide ;-)

If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?

Best Regards,
Jaehoon Chung

> 
> 
> Heiko
>
Ulf Hansson Oct. 2, 2015, 8:43 a.m. UTC | #10
On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 10/02/2015 04:06 PM, Heiko Stübner wrote:
>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>> On 10/01, Heiko Stübner wrote:
>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>> wrote:
>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>> so
>>>>>>>> should have no problem going through another tree.
>>>>>>>
>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>> patches as well?
>>>>>>
>>>>>> The API stays of course the same, only the degree to settings
>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>> system would stay running.
>>>>>>
>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>> and
>>>>>> everything would stay together that would work even better :-)
>>>>>
>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>
>>>> I don't mind picking up the clock patches. So I consider this as an
>>>> ack for both patch 1 and patch2, thanks.
>>>>
>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>
>>> I will check other patches on today, if it's ok, i will apply at my
>>> repository. Thanks for giving time! :)
>>
>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>> guess that is between you two to decide ;-)
>
> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?

Stephen already acked the clock patches from this thread.

Perhaps this time, it's easier if you ack the dw_mmc patches and I can
pick up the hole series? Does that work for you?

Kind regards
Uffe
Jaehoon Chung Oct. 2, 2015, 8:48 a.m. UTC | #11
On 10/02/2015 05:43 PM, Ulf Hansson wrote:
> On 2 October 2015 at 09:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 10/02/2015 04:06 PM, Heiko Stübner wrote:
>>> Am Freitag, 2. Oktober 2015, 11:52:00 schrieb Jaehoon Chung:
>>>> On 10/02/2015 06:05 AM, Ulf Hansson wrote:
>>>>> On 1 October 2015 at 19:35, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>>> On 10/01, Heiko Stübner wrote:
>>>>>>> Am Donnerstag, 1. Oktober 2015, 11:54:24 schrieb Ulf Hansson:
>>>>>>>> On 30 September 2015 at 16:55, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>>>>> Am Mittwoch, 30. September 2015, 16:42:05 schrieb Ulf Hansson:
>>>>>>>>>> On 30 September 2015 at 16:07, Heiko Stuebner <heiko@sntech.de>
>>> wrote:
>>>>>>>>> The clock changes of course only touch internals of the phase-clocks,
>>>>>>>>> so
>>>>>>>>> should have no problem going through another tree.
>>>>>>>>
>>>>>>>> What happens if I take mmc and dt changes, wouldn't I need the clock
>>>>>>>> patches as well?
>>>>>>>
>>>>>>> The API stays of course the same, only the degree to settings
>>>>>>> translation gets optimized, so I guess in the worst case you would get
>>>>>>> no good phase and thus fall back to non-highspeed modes - but the
>>>>>>> system would stay running.
>>>>>>>
>>>>>>> But of course, if the clock maintainers could Ack the two clock patches
>>>>>>> and
>>>>>>> everything would stay together that would work even better :-)
>>>>>>
>>>>>> If Ulf doesn't want to take them we can apply them to clk tree.
>>>>>> Otherwise, you can have my acked-by on the clk patches.
>>>>>
>>>>> I don't mind picking up the clock patches. So I consider this as an
>>>>> ack for both patch 1 and patch2, thanks.
>>>>>
>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>
>>>> I will check other patches on today, if it's ok, i will apply at my
>>>> repository. Thanks for giving time! :)
>>>
>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>> guess that is between you two to decide ;-)
>>
>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?
> 
> Stephen already acked the clock patches from this thread.
> 
> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
> pick up the hole series? Does that work for you?

I doesn't review dwmmc patches (Sorry), could you give me more time?
If you're ok, i will notice to you...then you can pick up all. :)

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
>
Ulf Hansson Oct. 2, 2015, 8:57 a.m. UTC | #12
[...]

>>>>>> Now, let's give Jaehoon some time to review the dw_mmc parts.
>>>>>
>>>>> I will check other patches on today, if it's ok, i will apply at my
>>>>> repository. Thanks for giving time! :)
>>>>
>>>> I think Ulf wanted to apply the whole series via the mmc tree directly, but I
>>>> guess that is between you two to decide ;-)
>>>
>>> If it will get Stephen's acked-by, i will pick up all after checked the dwmmc patches. how about?
>>
>> Stephen already acked the clock patches from this thread.
>>
>> Perhaps this time, it's easier if you ack the dw_mmc patches and I can
>> pick up the hole series? Does that work for you?
>
> I doesn't review dwmmc patches (Sorry), could you give me more time?
> If you're ok, i will notice to you...then you can pick up all. :)

Sure, no worries. Take your time!

Kind regards
Uffe
diff mbox

Patch

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0520064..363cc98 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1437,6 +1437,80 @@  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 min_uV, int target_uV,
+						  int max_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(regulator, min_uV, max_uV))
+		return -EINVAL;
+
+	return regulator_set_voltage_triplet(regulator, min_uV, target_uV,
+					     max_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.
+ * If this is not possible we'll try the full 2.7-3.6V of the spec.
+ *
+ * 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)
+{
+	int volt, min_uV, max_uV;
+
+	/* 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,
+						1100000, 1200000, 1300000);
+	case MMC_SIGNAL_VOLTAGE_180:
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						1700000, 1800000, 1950000);
+	case MMC_SIGNAL_VOLTAGE_330:
+		volt = regulator_get_voltage(mmc->supply.vmmc);
+		if (volt < 0)
+			return volt;
+
+		min_uV = max(volt - 300000, 2700000);
+		max_uV = min(volt + 300000, 3600000);
+
+		/*
+		 * Due to a limitation in the current implementation of
+		 * regulator_set_voltage_triplet() which is taking the lowest
+		 * voltage possible if below the target, search for a suitable
+		 * voltage in two steps and try to stay close to vmmc
+		 * with a 0.3V tolerance at first.
+		 */
+		if (!mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						min_uV, volt, max_uV))
+			return 0;
+
+		return mmc_regulator_set_voltage_if_supported(mmc->supply.vqmmc,
+						2700000, volt, 3600000);
+	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 83b81fd..a2a78eb 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -423,6 +423,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)
 {
@@ -435,6 +436,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);