linux-kernel - Re: [PATCH v2] mmc: sdhci: apply voltage range check only for non-fixed regulators

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Nov 2012 15:09:36 +0100
From:	Marek Szyprowski <m.szyprowski@...sung.com>
To:	Chris Ball <cjb@...top.org>
Cc:	linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Liam Girdwood <lrg@...com>
Subject: Re: [PATCH v2] mmc: sdhci: apply voltage range check only for
 non-fixed regulators

Hello,

On 11/13/2012 2:45 PM, Chris Ball wrote:
> Hi Marek,
>
> On Tue, Nov 13 2012, Marek Szyprowski wrote:
> > Fixed regulators cannot change their voltage, so disable all voltage
> > range checking for them, otherwise the driver fails to operate with
> > fixed regulators. Up to now it worked only by luck, because
> > regulator_is_supported_voltage() function returned incorrect values.
> > Commit "regulator: fix voltage check in regulator_is_supported_voltage()"
> > fixed that function and now additional check is needed for fixed
> > regulators.
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski@...sung.com>
> > ---
> >  drivers/mmc/host/sdhci.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index c7851c0..6f6534e 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -2923,7 +2923,7 @@ int sdhci_add_host(struct sdhci_host *host)
> >  		regulator_enable(host->vmmc);
> >
> >  #ifdef CONFIG_REGULATOR
> > -	if (host->vmmc) {
> > +	if (host->vmmc && regulator_count_voltages(host->vmmc) > 1) {
> >  		ret = regulator_is_supported_voltage(host->vmmc, 3300000,
> >  			3300000);
> >  		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
>
> Thanks for the longer explanation.  I'm still missing something, though;
> what's wrong with running the check as it was with the new regulator code?
> (I haven't tried it yet.)
>
> #ifdef CONFIG_REGULATOR
>          if (host->vmmc) {
>                  ret = regulator_is_supported_voltage(host->vmmc, 3300000,
>                          3300000);
>                  if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
>                          caps[0] &= ~SDHCI_CAN_VDD_330;
>                  ret = regulator_is_supported_voltage(host->vmmc, 3000000,
>                          3000000);
>                  if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
>                          caps[0] &= ~SDHCI_CAN_VDD_300;
>                  ret = regulator_is_supported_voltage(host->vmmc, 1800000,
>                          1800000);
>                  if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
>                          caps[0] &= ~SDHCI_CAN_VDD_180;
>          }
> #endif /* CONFIG_REGULATOR */
>
> The point is to remove unsupported voltages, so if someone sets up a
> fixed regulator at 3300000, all of the other caps are disabled.  Why
> wouldn't that work without this change, and how are we supposed to
> remove those caps on a fixed regulator after your patchset?
>
> Thanks, sorry if I'm missing something obvious,

On our boards eMMC is connected to fixed 2.8V regulator, what results in
clearing all available voltages and fail. The same situation is when one
enable dummy regulator and try to use sdhci with it. My patch fixes this
and restores sdhci to working state as it was before (before fixing
regulator regulator_is_supported_voltage() function and earlier when
MMC_BROKEN_VOLATGE capability was used).

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center


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