[PATCH] regulator: core: fix _regulator_do_disable return value

[PATCH] regulator: core: fix _regulator_do_disable return value

[Marco Felsch]

Currently _regulator_do_disable returns 0 if either the ena_pin nor the
ops.disbale() isn't present. This assumes that the call was successful
but it wasn't because disbaling isn't supported at all.

The last case of the if-chain should return -EINVAL, because disabling
isn't supported by the driver as it is done already by
_regulator_do_enable.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bb1324f93143..69b4e096b632 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2311,6 +2311,8 @@ static int _regulator_do_disable(struct regulator_dev *rdev)
 		ret = rdev->desc->ops->disable(rdev);
 		if (ret != 0)
 			return ret;
+	} else {
+		return -EINVAL;
 	}
 
 	/* cares about last_off_jiffy only if off_on_delay is required by
-- 
2.18.0

Re: [PATCH] regulator: core: fix _regulator_do_disable return value

[Mark Brown]

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

On Fri, Jul 13, 2018 at 02:56:24PM +0200, Marco Felsch wrote:

> Currently _regulator_do_disable returns 0 if either the ena_pin nor the
> ops.disbale() isn't present. This assumes that the call was successful
> but it wasn't because disbaling isn't supported at all.

> The last case of the if-chain should return -EINVAL, because disabling
> isn't supported by the driver as it is done already by
> _regulator_do_enable.

This is fine - consumers shouldn't expect that a disable will cause
anything to actually get powered off, constraints or other consumers
might mean that the disable doesn't actually happen.  It's just the same
as a consumer with an always on flag.

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

Re: [PATCH] regulator: core: fix _regulator_do_disable return value

[Marco Felsch]

Hi Mark,

tanks for the feedback.

On 18-07-13 14:07, Mark Brown wrote:
> On Fri, Jul 13, 2018 at 02:56:24PM +0200, Marco Felsch wrote:
> 
> > Currently _regulator_do_disable returns 0 if either the ena_pin nor the
> > ops.disbale() isn't present. This assumes that the call was successful
> > but it wasn't because disbaling isn't supported at all.
> 
> > The last case of the if-chain should return -EINVAL, because disabling
> > isn't supported by the driver as it is done already by
> > _regulator_do_enable.
> 
> This is fine - consumers shouldn't expect that a disable will cause
> anything to actually get powered off, constraints or other consumers
> might mean that the disable doesn't actually happen.  It's just the same
> as a consumer with an always on flag.

Okay, I understand that the behaviour should be like the always-on
contrain. But now the behaviour of the core is like my v1 of
"Re-Enable support to disable switch regulators". It's like a 'simulated
off', which wasn't a good solution for you. The difference is, that the
'simulated off' is now made in the core.

Regards,
Marco

Re: [PATCH] regulator: core: fix _regulator_do_disable return value

[Mark Brown]

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

On Fri, Jul 13, 2018 at 05:48:54PM +0200, Marco Felsch wrote:
> On 18-07-13 14:07, Mark Brown wrote:

> > This is fine - consumers shouldn't expect that a disable will cause
> > anything to actually get powered off, constraints or other consumers
> > might mean that the disable doesn't actually happen.  It's just the same
> > as a consumer with an always on flag.

> Okay, I understand that the behaviour should be like the always-on
> contrain. But now the behaviour of the core is like my v1 of
> "Re-Enable support to disable switch regulators". It's like a 'simulated
> off', which wasn't a good solution for you. The difference is, that the
> 'simulated off' is now made in the core.

Right, there's a difference between what the core (which does actually
explicitly turn things on and off) sees and what the consumers (which
only increment and decrement reference counts which may happen to result
in something being turned off immediately but also might not) see.

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