linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] regulator: core: Don't use regulators as supplies until the parent is bound
@ 2017-01-10 11:01 Jon Hunter
  2017-01-10 16:57 ` Mark Brown
  0 siblings, 1 reply; 3+ messages in thread
From: Jon Hunter @ 2017-01-10 11:01 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown; +Cc: linux-kernel, linux-tegra, Jon Hunter

When regulators are successfully registered, we check to see if the
regulator is a supply for any other registered regulator and if so
add the new regulator as the supply for the existing regulator(s).

Some devices, such as Power Management ICs, may register a series of
regulators when probed and there are cases where one of the regulators
may fail to register and defer the probing of the parent device. In this
case any successfully registered regulators would be unregistered so
that they can be re-registered at some time later when the probe is
attempted again. However, if one of the regulators that was registered
was added as a supply to another registered regulator (that did not
belong to the same parent device), then this supply regulator was
unregister again because the parent device is probe deferred, then a
regulator could be holding an invalid reference to a supply regulator
that has been unregistered. This will lead to a system crash if that
regulator is then used.

Although it would be possible to check when unregistering a regulator
if any other regulator in the system is using it as a supply, it still
may not be possible to remove it as a supply if this other regulator is
in use. Therefore, fix this by preventing any regulator from adding
another regulator as a supply if the parent device for the supply
regulator has not been bound. This will allow a parent device that is
registering regulators to be probe deferred and ensure that none of the
regulators it has registered are used as supplies for any other
regulator.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 04baac9a165b..8adadcdfed1c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1553,6 +1553,11 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		}
 	}
 
+	if (r->dev.parent && !device_is_bound(r->dev.parent)) {
+		put_device(&r->dev);
+		return -EPROBE_DEFER;
+	}
+
 	/* Recursively resolve the supply of the supply */
 	ret = regulator_resolve_supply(r);
 	if (ret < 0) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] regulator: core: Don't use regulators as supplies until the parent is bound
  2017-01-10 11:01 [PATCH] regulator: core: Don't use regulators as supplies until the parent is bound Jon Hunter
@ 2017-01-10 16:57 ` Mark Brown
  2017-01-11 16:57   ` Jon Hunter
  0 siblings, 1 reply; 3+ messages in thread
From: Mark Brown @ 2017-01-10 16:57 UTC (permalink / raw)
  To: Jon Hunter; +Cc: Liam Girdwood, linux-kernel, linux-tegra

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

On Tue, Jan 10, 2017 at 11:01:09AM +0000, Jon Hunter wrote:

> +++ b/drivers/regulator/core.c
> @@ -1553,6 +1553,11 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
>  		}
>  	}
>  
> +	if (r->dev.parent && !device_is_bound(r->dev.parent)) {
> +		put_device(&r->dev);
> +		return -EPROBE_DEFER;
> +	}
> +

I realised earlier that this will trigger when the supply and the
consumer are part of the same PMIC which will cause issues as that's a
fairly common setup and won't be resolved by deferring.  Can you please
modify this to only defer if the parent is not the same as the parent of
the regulator we're currently resolving?  Sorry, should've thought of
this yesterday.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] regulator: core: Don't use regulators as supplies until the parent is bound
  2017-01-10 16:57 ` Mark Brown
@ 2017-01-11 16:57   ` Jon Hunter
  0 siblings, 0 replies; 3+ messages in thread
From: Jon Hunter @ 2017-01-11 16:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Liam Girdwood, linux-kernel, linux-tegra


On 10/01/17 16:57, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Tue, Jan 10, 2017 at 11:01:09AM +0000, Jon Hunter wrote:
> 
>> +++ b/drivers/regulator/core.c
>> @@ -1553,6 +1553,11 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
>>  		}
>>  	}
>>  
>> +	if (r->dev.parent && !device_is_bound(r->dev.parent)) {
>> +		put_device(&r->dev);
>> +		return -EPROBE_DEFER;
>> +	}
>> +
> 
> I realised earlier that this will trigger when the supply and the
> consumer are part of the same PMIC which will cause issues as that's a
> fairly common setup and won't be resolved by deferring.  Can you please
> modify this to only defer if the parent is not the same as the parent of
> the regulator we're currently resolving?  Sorry, should've thought of
> this yesterday.

OK, no problem. Will send out an update shortly.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-01-11 16:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10 11:01 [PATCH] regulator: core: Don't use regulators as supplies until the parent is bound Jon Hunter
2017-01-10 16:57 ` Mark Brown
2017-01-11 16:57   ` Jon Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).