Hello Krzysztof, On 03/27/2016 08:54 PM, Krzysztof Kozlowski wrote: > The buck9 regulator of S2MPS11 PMIC lacked minimal selector for linear > mapping. The mapping starts from 0x40 (3 V). > This patch is a real fix since the the SD error goes away and the regulator driver probes correctly now. > This buck9 provides power to other regulators, including LDO13 and LDO19 > which supply the MMC2 (SD card). > I think it's worth mentioning that this is the case for the Exynos5422 Odroid XU{3,4} boards. I mean, the regulators can be used for anything but are those boards that use LDO19 as the SD card supply. In fact, I wonder if the subject line shouldn't be changed to something like: "regulator: s2mps11: Fix invalid min selector and voltages num for buck9" since the real problem is that the .linear_min_sel and .n_voltages are wrong. The fact that the SD was failing on Odroids is just a consequence of that (although I agree that this information should be part of the commit message). > 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@xxxxxxxxxxxxxxx> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> > > --- > > 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/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c > index d24e2c783dc5..caeefc38ac47 100644 > --- a/drivers/regulator/s2mps11.c > +++ b/drivers/regulator/s2mps11.c > @@ -324,6 +324,23 @@ static struct regulator_ops s2mps11_buck_ops = { > .enable_mask = S2MPS11_ENABLE_MASK \ > } > > +#define regulator_desc_s2mps11_buck9 { \ > + .name = "BUCK9", \ > + .id = S2MPS11_BUCK9, \ > + .ops = &s2mps11_buck_ops, \ > + .type = REGULATOR_VOLTAGE, \ > + .owner = THIS_MODULE, \ > + .min_uV = MIN_3000_MV, \ > + .uV_step = STEP_25_MV, \ > + .linear_min_sel = S2MPS11_BUCK9_MIN_VSEL, \ I don't have a datasheet for this PMIC but I wonder if buck9 is the only buck regulator whose minimal register value is != 0. If that's not the case, it would be good to fix the descriptions for all other regulators. > + .n_voltages = S2MPS11_BUCK9_N_VOLTAGES, \ > + .ramp_delay = S2MPS11_RAMP_DELAY, \ > + .vsel_reg = S2MPS11_REG_B9CTRL2, \ > + .vsel_mask = S2MPS11_BUCK_VSEL_MASK, \ > + .enable_reg = S2MPS11_REG_B9CTRL1, \ > + .enable_mask = S2MPS11_ENABLE_MASK \ > +} > + > static const struct regulator_desc s2mps11_regulators[] = { > regulator_desc_s2mps11_ldo(1, STEP_25_MV), > regulator_desc_s2mps11_ldo(2, STEP_50_MV), > @@ -371,7 +388,7 @@ static const struct regulator_desc s2mps11_regulators[] = { > regulator_desc_s2mps11_buck6_10(6, MIN_600_MV, STEP_6_25_MV), > regulator_desc_s2mps11_buck6_10(7, MIN_600_MV, STEP_6_25_MV), > regulator_desc_s2mps11_buck6_10(8, MIN_600_MV, STEP_6_25_MV), > - regulator_desc_s2mps11_buck6_10(9, MIN_3000_MV, STEP_25_MV), Maybe the regulator_desc_s2mps11_buck6_10() define should be renamed? Since it's no longer true that can be used for buck6-10, so the name is misleading now. Patch looks good to me though and as I said it fixes the issue so: Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx> Tested-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html