Re: Warnings for invalid VDD (sdhci-s3c)

[Adrian Hunter]

On 28/03/16 09:07, Krzysztof Kozlowski wrote:
> On 24.03.2016 22:21, Adrian Hunter wrote:
>> On 24/03/16 15:11, Adrian Hunter wrote:
>>> On 24/03/16 10:42, Krzysztof Kozlowski wrote:
>>>> On 24.03.2016 17:24, Jisheng Zhang wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, 24 Mar 2016 17:09:27 +0900 Jaehoon Chung wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 03/24/2016 04:58 PM, Jisheng Zhang wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Thu, 24 Mar 2016 16:28:56 +0900 Krzysztof Kozlowski wrote:
>>>>>>>   
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> After 918f4cbd4340 ("mmc: sdhci: restore behavior when setting VDD via
>>>>>>>> external regulator") On Trats2 board I see warnings for invalid VDD
>>>>>>>> value (2.8V):
>>>>>>>>
>>>>>>>> [    3.119656] ------------[ cut here ]------------
>>>>>>>> [    3.119666] WARNING: CPU: 3 PID: 90 at
>>>>>>>> ../drivers/mmc/host/sdhci.c:1234 sdhci_do_set_ios+0x4cc/0x5e0
>>>>>>>> [    3.119669] mmc0: Invalid vdd 0x10  
>>>>>>>
>>>>>>> Per my understanding, the wrong vdd indicates a wrong ocr, what's the voltage of
>>>>>>> this host's vmmc regulator?  
>>>>>>
>>>>>> As i know, it's fixed-voltage with gpio on trats2. It's 2.8V.
>>>>>> I didn't check this entirely..need to check ocr value.
>>>>>>
>>>>>
>>>>> I may know the reason. the vmmc is 2.8v, then mmc_regulator_get_supply() convert
>>>>> the value to a ocr as 0x10. The key here is that the 2.8v is invalid in SDHCI
>>>>> case and isn't accepted by current sdhci driver.
>>>>
>>>> Yeah, I already wrote that. It is the part of the warning and my email.
>>>> Our regulator is fixed at 2.8 which is 0x10. :)
>>>>
>>>>> I dunno the elegant solution to handle this case, let's wait for sdhci maintainers
>>>>> idea.
>>>>
>>>> Hmm...
>>>
>>> I haven't tested it, but what about this:
>>
>> And now with checkpatch complaints fixed:
>>
>> From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> Date: Thu, 24 Mar 2016 14:29:24 +0200
>> Subject: [PATCH V2] mmc: sdhci: Fix regression setting power on Trats2 board
>>
>> Several commits relating to setting power have been introducing
>> problems by putting driver-specific rules into generic SDHCI code.
>>
>> Fix by adding a 'set_power' callback and restoring the default
>> behaviour prior to commit 918f4cbd4340 ("mmc: sdhci: restore
>> behavior when setting VDD via external regulator").  The desired
> 
> s/behavior/behaviour/

It seems checkpatch insists the commit message must be identical to the
original, so I am leaving it that way.

> 
>> behaviour of that commit is gotten by having sdhci-pxav3 provide
>> its own set_power callback.
>>
>> Reported-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
>> Fixes: 918f4cbd4340 ("mmc: sdhci: restore behavior when setting VDD...)
>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx # v4.5+
>> ---
>>  drivers/mmc/host/sdhci-pxav3.c | 17 +++++++++++++++++
>>  drivers/mmc/host/sdhci.c       | 38 ++++++++++++++++++++++++++++++--------
>>  drivers/mmc/host/sdhci.h       |  4 ++++
>>  3 files changed, 51 insertions(+), 8 deletions(-)
>>
> 
> Works for me (sdhci-s3c, Exynos4412 on Trats2 board):
> Tested-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>

I made a V3 which has a couple of minor changes for pxav3_set_power()
and consequently have not added the Reviewed/Tested-by Jisheng Zhang.