| Message ID | 1460965378-5256-1-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Mon, Apr 18, 2016 at 09:42:58AM +0200, Krzysztof Kozlowski wrote: > On S2MPS11 and S2MPS14 devices the default implementation of > set_voltage_time_sel() for LDO regulators was not doing anything useful > because users did not provide ramp delay in Device Tree so the > set_voltage_time_sel() exited with status 0. This could be seen in > dmesg, e.g. on Odroid XU4: That's not the bug, the bug is that the driver is providing the operation without providing a non-zero value for it. The device tree configuration is to override what the driver is doing.
On 04/18/2016 11:57 AM, Mark Brown wrote: > On Mon, Apr 18, 2016 at 09:42:58AM +0200, Krzysztof Kozlowski wrote: > >> On S2MPS11 and S2MPS14 devices the default implementation of >> set_voltage_time_sel() for LDO regulators was not doing anything useful >> because users did not provide ramp delay in Device Tree so the >> set_voltage_time_sel() exited with status 0. This could be seen in >> dmesg, e.g. on Odroid XU4: > > That's not the bug, the bug is that the driver is providing the > operation without providing a non-zero value for it. The device tree > configuration is to override what the driver is doing. So your recommendation would be to set the ramp_delay value in regulator_desc structure? The problem is that value of 30 mv/us in datasheet looks untrustworthy (copied from other pages) and vendor kernel (which should be a reference) uses 12 mv/us. Anyway setting any value greater than 0 seems like better idea than just sticking to 0... Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 18, 2016 at 12:36:52PM +0200, Krzysztof Kozlowski wrote: > On 04/18/2016 11:57 AM, Mark Brown wrote: > > That's not the bug, the bug is that the driver is providing the > > operation without providing a non-zero value for it. The device tree > > configuration is to override what the driver is doing. > So your recommendation would be to set the ramp_delay value in > regulator_desc structure? Or at least fix the changelog to describe the problem more clearly. > The problem is that value of 30 mv/us in datasheet looks untrustworthy > (copied from other pages) and vendor kernel (which should be a > reference) uses 12 mv/us. Anyway setting any value greater than 0 seems > like better idea than just sticking to 0... Yup. Probably the more conservative value is going to be safer.
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c index 64a262f10845..437b1b3b26c9 100644 --- a/drivers/regulator/s2mps11.c +++ b/drivers/regulator/s2mps11.c @@ -246,7 +246,6 @@ static struct regulator_ops s2mps11_ldo_ops = { .disable = regulator_disable_regmap, .get_voltage_sel = regulator_get_voltage_sel_regmap, .set_voltage_sel = regulator_set_voltage_sel_regmap, - .set_voltage_time_sel = regulator_set_voltage_time_sel, }; static struct regulator_ops s2mps11_buck_ops = { @@ -606,7 +605,6 @@ static struct regulator_ops s2mps14_reg_ops = { .disable = regulator_disable_regmap, .get_voltage_sel = regulator_get_voltage_sel_regmap, .set_voltage_sel = regulator_set_voltage_sel_regmap, - .set_voltage_time_sel = regulator_set_voltage_time_sel, .set_suspend_disable = s2mps14_regulator_set_suspend_disable, };
On S2MPS11 and S2MPS14 devices the default implementation of set_voltage_time_sel() for LDO regulators was not doing anything useful because users did not provide ramp delay in Device Tree so the set_voltage_time_sel() exited with status 0. This could be seen in dmesg, e.g. on Odroid XU4: [ 1.486076] vdd_ldo9: ramp_delay not set [ 1.506875] vddq_mmc2: ramp_delay not set [ 1.523766] vdd_ldo15: ramp_delay not set [ 1.544702] vdd_sd: ramp_delay not set The datasheet for these devices is inconsistent and does not specify unambiguously the value of ramp delay for LDO. It mentions 30 mV/us in one timing diagram but then omits it completely in LDO regulator characteristics table (it is specified for bucks). To make more confusion, the vendor kernel for Galaxy S5 and Odroid XU3 uses sometimes value of 12 mV/us. Overall, since the ramp delay is not currently provided by DT and it is unclear what is the correct value anyway, the safest choice seems to be to remove the implementation of set_voltage_time_sel() for S2MPS11 and S2MPS14 LDOs thus getting rid of the warning. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/regulator/s2mps11.c | 2 -- 1 file changed, 2 deletions(-)