| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFq_WMJfqNGuxwN7xPmhv2X6-+G1zXpdD6zYDkDe7-Jo2A@mail.gmail.com>
Date: Mon, 16 Mar 2015 15:05:59 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Doug Anderson <dianders@...omium.org>
Cc: Heiko Stuebner <heiko@...ech.de>,
Jaehoon Chung <jh80.chung@...sung.com>,
Seungwon Jeon <tgih.jun@...sung.com>,
Mark Brown <broonie@...nel.org>,
Alexandru Stan <amstan@...omium.org>,
Alim Akhtar <alim.akhtar@...sung.com>,
Sonny Rao <sonnyrao@...omium.org>,
Andrew Bresticker <abrestic@...omium.org>,
Addy Ke <addy.ke@...k-chips.com>,
Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Chris Ball <chris@...ntf.net>,
Johan Rudholm <johan.rudholm@...s.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Tim Kryger <tim.kryger@...il.com>,
Andrew Gabbasov <andrew_gabbasov@...tor.com>,
Sascha Hauer <s.hauer@...gutronix.de>,
linux-mmc <linux-mmc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc()
On 11 March 2015 at 23:15, Doug Anderson <dianders@...omium.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@...omium.org>
> Reviewed-by: Andrew Bresticker <abrestic@...omium.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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists