linux-kernel - Re: [PATCH] regulator: s2mps11: Fix invalid minimal selector for buck9 supplying SD card

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]

Message-id: <56FA499F.2060206@samsung.com>
Date:	Tue, 29 Mar 2016 18:23:43 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Lee Jones <lee.jones@...aro.org>
Cc:	Sangbeom Kim <sbkim73@...sung.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, linux-mmc@...r.kernel.org,
	Javier Martinez Canillas <javier@....samsung.com>,
	Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>,
	stable@...r.kernel.org
Subject: Re: [PATCH] regulator: s2mps11: Fix invalid minimal selector for buck9
 supplying SD card

On 29.03.2016 18:08, Lee Jones wrote:
> On Mon, 28 Mar 2016, Krzysztof Kozlowski wrote:
> 
>> The buck9 regulator of S2MPS11 PMIC lacked minimal selector for linear
>> mapping. The mapping starts from 0x40 (3 V).
>>
>> This buck9 provides power to other regulators, including LDO13 and LDO19
>> which supply the MMC2 (SD card).
>>
>> Bootloader initializes the regulator with value of 0xff (5 V) which is
>> outside of supported voltage range. When (during boot) constraints to
>> buck9 were applied, the driver wrote value counting from 0x00, not 0x40.
>> Effectively driver set lower voltage than required leading to SD card
>> detection errors on Odroid XU3/XU4:
>> 	mmc1: card never left busy state
>> 	mmc1: error -110 whilst initialising SD card
>>
>> Fixes: cb74685ecb39 ("regulator: s2mps11: Add samsung s2mps11 regulator driver")
>> Cc: <stable@...r.kernel.org>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@...sung.com>
>>
>> ---
>>
>> The issue can be reproduced on next-20160324 with
>> bae4fdc88d7f7dda1 (regulator: core: Ensure we are at least in bounds
>> for our constraints).
>> ---
>>  drivers/regulator/s2mps11.c         | 19 ++++++++++++++++++-
>>  include/linux/mfd/samsung/s2mps11.h |  9 +++++++++
>>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> [...]
> 
>> diff --git a/include/linux/mfd/samsung/s2mps11.h b/include/linux/mfd/samsung/s2mps11.h
>> index b288965e8101..3937a932bfe0 100644
>> --- a/include/linux/mfd/samsung/s2mps11.h
>> +++ b/include/linux/mfd/samsung/s2mps11.h
>> @@ -173,10 +173,19 @@ enum s2mps11_regulators {
>>  
>>  #define S2MPS11_LDO_VSEL_MASK	0x3F
>>  #define S2MPS11_BUCK_VSEL_MASK	0xFF
>> +#define S2MPS11_BUCK9_MIN_VSEL	0x40
>>  #define S2MPS11_ENABLE_MASK	(0x03 << S2MPS11_ENABLE_SHIFT)
>>  #define S2MPS11_ENABLE_SHIFT	0x06
>>  #define S2MPS11_LDO_N_VOLTAGES	(S2MPS11_LDO_VSEL_MASK + 1)
>>  #define S2MPS11_BUCK_N_VOLTAGES (S2MPS11_BUCK_VSEL_MASK + 1)
>> +/*
>> + * Buck9 supports only 32 voltages (values from 0x40 to 0x5F) but bootloader
>> + * initializes the register with value of 0xff so when probing this would
>> + * cause a failure (Odroid XU3):
>> + *	vdd_2.8v_ldo: failed to get the current voltage(-22)
>> + * Instead pretend we support up to 0xff (5 V).
>> + */
>> +#define S2MPS11_BUCK9_N_VOLTAGES	192
> 
> Err... NACK.
> 
> Please go and fix the bootloader instead of hacking the kernel.

The change is not needed and was an effect of my inaccurate
understanding of device and driver behaviour. In v2 there is no such
statement and changes. Instead driver properly uses the mask without
touching other parts of register.

Best regards,
Krzysztof