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

Message-ID: <20160329094023.GD8659@x1>
Date:	Tue, 29 Mar 2016 10:40:23 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
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 Tue, 29 Mar 2016, Krzysztof Kozlowski wrote:

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

Very well.  Thanks for letting me know.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog