[PATCH v2] regulator: fixed: support deferred probe for DT GPIOs

[PATCH v2] regulator: fixed: support deferred probe for DT GPIOs

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

of_get_named_gpio() needs the driver hosting the GPIO that the DT
property references to have been probed. Detect this specific failure,
and defer the probe of the whole regulator until this API can complete.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: Simplify the patch by returning ERR_PTR()s instead of a separate
defer_probe value.
---
 drivers/regulator/fixed.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 8bda365..b8c3a91 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -61,11 +61,11 @@ of_get_fixed_voltage_config(struct device *dev)
 	config = devm_kzalloc(dev, sizeof(struct fixed_voltage_config),
 								 GFP_KERNEL);
 	if (!config)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	config->init_data = of_get_regulator_init_data(dev, dev->of_node);
 	if (!config->init_data)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	init_data = config->init_data;
 	init_data->constraints.apply_uV = 0;
@@ -76,13 +76,26 @@ of_get_fixed_voltage_config(struct device *dev)
 	} else {
 		dev_err(dev,
 			 "Fixed regulator specified with variable voltages\n");
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (init_data->constraints.boot_on)
 		config->enabled_at_boot = true;
 
 	config->gpio = of_get_named_gpio(np, "gpio", 0);
+	/*
+	 * of_get_named_gpio() currently returns ENODEV rather than
+	 * EPROBE_DEFER. This code attempts to be compatible with both
+	 * for now; the ENODEV check can be removed once the API is fixed.
+	 * of_get_named_gpio() doesn't differentiate between a missing
+	 * property (which would be fine here, since the GPIO is optional)
+	 * and some other error. Patches have been posted for both issues.
+	 * Once they are check in, we should replace this with:
+	 * if (config->gpio < 0 && config->gpio != -ENOENT)
+	 */
+	if ((config->gpio == -ENODEV) || (config->gpio == -EPROBE_DEFER))
+		return ERR_PTR(-EPROBE_DEFER);
+
 	delay = of_get_property(np, "startup-delay-us", NULL);
 	if (delay)
 		config->startup_delay = be32_to_cpu(*delay);
@@ -172,10 +185,13 @@ static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev)
 	struct regulator_config cfg = { };
 	int ret;
 
-	if (pdev->dev.of_node)
+	if (pdev->dev.of_node) {
 		config = of_get_fixed_voltage_config(&pdev->dev);
-	else
+		if (IS_ERR(config))
+			return PTR_ERR(config);
+	} else {
 		config = pdev->dev.platform_data;
+	}
 
 	if (!config)
 		return -ENOMEM;
-- 
1.7.0.4

Re: [PATCH v2] regulator: fixed: support deferred probe for DT GPIOs

[Mark Brown]

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

On Fri, Jun 29, 2012 at 10:33:15AM -0600, Stephen Warren wrote:

> of_get_named_gpio() needs the driver hosting the GPIO that the DT
> property references to have been probed. Detect this specific failure,
> and defer the probe of the whole regulator until this API can complete.

I've applied this but...

> +	/*
> +	 * of_get_named_gpio() currently returns ENODEV rather than
> +	 * EPROBE_DEFER. This code attempts to be compatible with both
> +	 * for now; the ENODEV check can be removed once the API is fixed.

...this just seems rubbish, why aren't we just fixing the device tree
code and why are we doing this at the device tree level rather than as a
general gpiolib thing?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] regulator: fixed: support deferred probe for DT GPIOs

[Stephen Warren]

On 07/01/2012 12:23 PM, Mark Brown wrote:
> On Fri, Jun 29, 2012 at 10:33:15AM -0600, Stephen Warren wrote:
> 
>> of_get_named_gpio() needs the driver hosting the GPIO that the
>> DT property references to have been probed. Detect this specific
>> failure, and defer the probe of the whole regulator until this
>> API can complete.
> 
> I've applied this but...

Thanks.

>> +	/* +	 * of_get_named_gpio() currently returns ENODEV rather
>> than +	 * EPROBE_DEFER. This code attempts to be compatible with
>> both +	 * for now; the ENODEV check can be removed once the API
>> is fixed.
> 
> ...this just seems rubbish, why aren't we just fixing the device
> tree code and why are we doing this at the device tree level rather
> than as a general gpiolib thing?

It is being fixed in the DT code:

https://lkml.org/lkml/2012/6/18/468

However, it seemed best to make the regulator code work both before
and after that patch.

This issue can't be fixed only in gpio_request(); of_get_named_gpio()
needs the GPIO driver to be probed/registered to even be able to parse
the GPIO ID out of the DT; it makes a call into the GPIO driver as
part of the parsing. So, the DT code will always have to handle
deferred probe, as well as gpio_request() (for the case where the GPIO
ID came from platform data and hence there was no earlier point where
deferred probed would be detected).

Re: [PATCH v2] regulator: fixed: support deferred probe for DT GPIOs

[Mark Brown]

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

On Mon, Jul 02, 2012 at 09:40:06AM -0600, Stephen Warren wrote:
> On 07/01/2012 12:23 PM, Mark Brown wrote:

> > ...this just seems rubbish, why aren't we just fixing the device
> > tree code and why are we doing this at the device tree level rather
> > than as a general gpiolib thing?

> It is being fixed in the DT code:

> https://lkml.org/lkml/2012/6/18/468

Is that patch actually being applied (especially with Grant's free time
issues)?  I've still got the patch sitting in my inbox since it wasn't
clear what was going on.

> However, it seemed best to make the regulator code work both before
> and after that patch.

I'd be kind of inclined to just leave it broken to be honest rather than
having a huge comment and dodgy code.

> This issue can't be fixed only in gpio_request(); of_get_named_gpio()
> needs the GPIO driver to be probed/registered to even be able to parse
> the GPIO ID out of the DT; it makes a call into the GPIO driver as

Oh, ick.  :/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] regulator: fixed: support deferred probe for DT GPIOs

[Stephen Warren]

On 07/02/2012 10:03 AM, Mark Brown wrote:
> On Mon, Jul 02, 2012 at 09:40:06AM -0600, Stephen Warren wrote:
>> On 07/01/2012 12:23 PM, Mark Brown wrote:
> 
>>> ...this just seems rubbish, why aren't we just fixing the
>>> device tree code and why are we doing this at the device tree
>>> level rather than as a general gpiolib thing?
> 
>> It is being fixed in the DT code:
> 
>> https://lkml.org/lkml/2012/6/18/468
> 
> Is that patch actually being applied (especially with Grant's free
> time issues)?  I've still got the patch sitting in my inbox since
> it wasn't clear what was going on.

It sounds like Rob Herring will be helping out on this for a few
weeks; see:

https://lkml.org/lkml/2012/7/2/3
"Re: SPI, GPIO, and DT maintainer ship (cry for help)"

I cc'd Grant and Rob to make sure.

>> However, it seemed best to make the regulator code work both
>> before and after that patch.
> 
> I'd be kind of inclined to just leave it broken to be honest rather
> than having a huge comment and dodgy code.

Hmmm. That would have made it kinda hard to use:-(