From: ulf.hansson@linaro.org (Ulf Hansson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc()
Date: Mon, 16 Mar 2015 15:05:59 +0100 [thread overview]
Message-ID: <CAPDyKFq_WMJfqNGuxwN7xPmhv2X6-+G1zXpdD6zYDkDe7-Jo2A@mail.gmail.com> (raw)
In-Reply-To: <1426112117-18220-2-git-send-email-dianders@chromium.org>
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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2015-03-16 14:05 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-11 22:15 [PATCH v4 1/4] mmc: dw_mmc: Don't try to enable the CD until we're sure we're not deferring Doug Anderson
2015-03-11 22:15 ` [PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc() Doug Anderson
2015-03-16 14:05 ` Ulf Hansson [this message]
2015-03-16 15:12 ` Doug Anderson
2015-03-17 10:23 ` Ulf Hansson
2015-03-17 10:38 ` Mark Brown
2015-03-17 11:28 ` Ulf Hansson
2015-03-19 4:09 ` Doug Anderson
2015-03-19 11:14 ` Ulf Hansson
2015-03-19 11:36 ` Mark Brown
2015-03-20 10:55 ` Ulf Hansson
2015-03-20 11:28 ` Mark Brown
2015-04-07 20:05 ` Doug Anderson
2015-04-08 11:28 ` Mark Brown
2015-03-11 22:15 ` [PATCH v4 3/4] mmc: dw_mmc: Use mmc_regulator_set_vqmmc in start_signal_voltage_switch Doug Anderson
2015-03-11 22:15 ` [PATCH v4 4/4] ARM: dts: Specify VMMC and VQMMC on rk3288-evb Doug Anderson
2015-03-11 23:30 ` Heiko Stuebner
2015-04-07 21:37 ` Heiko Stübner
2015-03-13 11:32 ` [PATCH v4 1/4] mmc: dw_mmc: Don't try to enable the CD until we're sure we're not deferring Jaehoon Chung
2015-03-13 12:10 ` Heiko Stuebner
2015-03-16 2:09 ` Jaehoon Chung
2015-03-27 5:55 ` Jaehoon Chung
2015-03-27 15:46 ` Doug Anderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPDyKFq_WMJfqNGuxwN7xPmhv2X6-+G1zXpdD6zYDkDe7-Jo2A@mail.gmail.com \
--to=ulf.hansson@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).