linux-kernel - Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

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] [day] [month] [year] [list]

Message-ID: <CADz5_g+gh33k9hKi9=1wd=qc-QoOGyGVnkfkT2QoE6cYnHjLKw@mail.gmail.com>
Date:	Wed, 20 Feb 2013 17:34:51 +0800
From:	Kevin Liu <keyuan.liu@...il.com>
To:	Ulf Hansson <ulf.hansson@...aro.org>
Cc:	Marek Szyprowski <m.szyprowski@...sung.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Guennadi Liakhovetski <g.liakhovetski@....de>,
	linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Liam Girdwood <lrg@...com>, Chris Ball <cjb@...top.org>
Subject: Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on
 regulators aware of voltage value

2013/2/14 Ulf Hansson <ulf.hansson@...aro.org>:
> On 14 February 2013 09:05, Marek Szyprowski <m.szyprowski@...sung.com> wrote:
>>
>> On 2/13/2013 12:35 PM, Mark Brown wrote:
>>>
>>> On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
>>> > On Wed, 13 Feb 2013, Marek Szyprowski wrote:
>>>
>>> > > BTW, mmc_regulator_get_ocrmask() won't work with continuous range
>>> > > regulators.
>>>
>>> > This seems like a problem, that has to be fixed...
>>>
>>> Indeed, what's the issue?
>>
>>
>> There are probably 2 issues:
>>
>> 1. mmc_regulator_get_ocrmask() works only with regulators which support
>> regulator_count_voltages() and regulator_list_voltage(). Recently support
>> for
>> continuous regulators have been merged. Such regulators doesn't provide
>> regulator_list_voltage() method, but are able to change/set voltage to the
>> given value. I agree that they are not very common, so right now we can
>> probably ignore them until the first board, which uses them arrives.
>>
>> 2. The second issue might be related to the testing of precise voltage
>> values
>> in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c driver
>> has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
>> ("mmc: sdhci: Use regulator min/max voltage range according to spec"), but I
>> don't know MMC core code to judge if ocr mask is used for exact voltage
>> checking or only for checking the voltage ranges. However someone with good
>> mmc subsystem knowledge should check it.
>>
>
> Not 100% sure what your problem relates too here, but I am aware of an
> issue for how the mmc protocol layer are handling ocr_masks. Let me
> try to describe it here:
>
> 1. During "card init" mmc_power_up will be called for telling the host
> driver to provide power to the card. The level of voltage will be set
> to "ocr_avail" which means the highest supported voltage by the host.
> 2. At the protocol layer the card init sequence tries to negotiate to
> lowest possible ocr value from what the card and the host together
> supports. Once done, the ocr mask value will be cached into the host
> struct.
> 3. The host will informed about the new ocr mask from the protocol
> layer with mmc_select_voltage and it somewhere here the problems
> starts. No host are actually changing the voltage level at this state
> (MMC_POWER_ON) which is correct since it would likely mean violation
> of the spec. At the same time the protocol layer still believes the
> host has switched to operate at the new voltage level.

According to the spec, only during the initialization procedure, the
host is not allowed to change the operating voltage range.
In fact, mmc_select_voltage is called before init start in
mmc_attach_sd/mmc_attach_mmc/mmc_attach_sdio.
So voltage level should can be changed during MMC_POWER_ON.
sdhci.c did this way and worked well. But I noticed some other hosts
didn't change voltage for MMC_POWER_ON.
I think it should be changed.

> 4. So the host and the protocol layer are out of sync with regards to
> the ocr mask, which is why the cached ocr_mask in the host struct is
> reset when doing mmc_power_off. Otherwise the suspend/resume sequence
> would have been broken.
>

But there indeed exist issues that the host->ocr doesn't restore after
resume back and operation voltage use the highest voltage.
Just submitted below patch to fix this. Please help to review.
"[PATCH] mmc: core: restore ocr and operation voltage in resume"

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