[PATCH] regulator: s2mps11: Remove unused set_voltage_time_sel

[PATCH] regulator: s2mps11: Remove unused set_voltage_time_sel

[Krzysztof Kozlowski]

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(-)

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,
 };
 
-- 
1.9.1

Re: [PATCH] regulator: s2mps11: Remove unused set_voltage_time_sel

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] regulator: s2mps11: Remove unused set_voltage_time_sel

[Krzysztof Kozlowski]

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

Re: [PATCH] regulator: s2mps11: Remove unused set_voltage_time_sel

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]