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

[PATCH] 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.

Note that of_get_named_gpio() currently returns -ENODEV in this case,
but a patch has been sent to make it return -EPROBE_DEFER. Hence, this
patch checks both so that it works with/without the other patch.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/regulator/fixed.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 8bda365..1acd60a 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -51,13 +51,15 @@ struct fixed_voltage_data {
  * alloc fails.
  */
 static struct fixed_voltage_config *
-of_get_fixed_voltage_config(struct device *dev)
+of_get_fixed_voltage_config(struct device *dev, bool *defer_probe)
 {
 	struct fixed_voltage_config *config;
 	struct device_node *np = dev->of_node;
 	const __be32 *delay;
 	struct regulator_init_data *init_data;
 
+	*defer_probe = false;
+
 	config = devm_kzalloc(dev, sizeof(struct fixed_voltage_config),
 								 GFP_KERNEL);
 	if (!config)
@@ -83,6 +85,11 @@ of_get_fixed_voltage_config(struct device *dev)
 		config->enabled_at_boot = true;
 
 	config->gpio = of_get_named_gpio(np, "gpio", 0);
+	if ((config->gpio == -ENODEV) || (config->gpio == -EPROBE_DEFER)) {
+		*defer_probe = true;
+		return NULL;
+	}
+
 	delay = of_get_property(np, "startup-delay-us", NULL);
 	if (delay)
 		config->startup_delay = be32_to_cpu(*delay);
@@ -167,15 +174,19 @@ static struct regulator_ops fixed_voltage_ops = {
 
 static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev)
 {
+	bool defer_probe;
 	struct fixed_voltage_config *config;
 	struct fixed_voltage_data *drvdata;
 	struct regulator_config cfg = { };
 	int ret;
 
-	if (pdev->dev.of_node)
-		config = of_get_fixed_voltage_config(&pdev->dev);
-	else
+	if (pdev->dev.of_node) {
+		config = of_get_fixed_voltage_config(&pdev->dev, &defer_probe);
+		if (defer_probe)
+			return -EPROBE_DEFER;
+	} else {
 		config = pdev->dev.platform_data;
+	}
 
 	if (!config)
 		return -ENOMEM;
-- 
1.7.0.4

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

[Mark Brown]

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

On Thu, Jun 28, 2012 at 04:31:32PM -0600, Stephen Warren wrote:

>  static struct fixed_voltage_config *
> -of_get_fixed_voltage_config(struct device *dev)
> +of_get_fixed_voltage_config(struct device *dev, bool *defer_probe)

This is pretty contorted, we should just be able to pass the return
value back more directly and of course ideally gpiolib would be doing
the -EPROBE_DEFER for us anyway (I did send a patch for this, Grant
didn't apply it due to a mostly unrelated issue in the current probe
deferral implementation).  Or just defer if we don't get a config passed
back or something.

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

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

[Stephen Warren]

On 06/28/2012 07:25 PM, Mark Brown wrote:
> On Thu, Jun 28, 2012 at 04:31:32PM -0600, Stephen Warren wrote:
> 
>> static struct fixed_voltage_config * 
>> -of_get_fixed_voltage_config(struct device *dev) 
>> +of_get_fixed_voltage_config(struct device *dev, bool
>> *defer_probe)
> 
> This is pretty contorted, we should just be able to pass the
> return value back more directly and of course ideally gpiolib would
> be doing the -EPROBE_DEFER for us anyway (I did send a patch for
> this, Grant didn't apply it due to a mostly unrelated issue in the
> current probe deferral implementation).  Or just defer if we don't
> get a config passed back or something.

I did consider making of_get_fixed_voltage_config() return a result
code, but then it needs some other way of returning the pointer, so
that seemed just as convoluted. Oh, I suppose it could use ERR_PTR()
to do that; that'd be nice and simple. Would that do?

Re: gpiolib doing it: How is that possible? of_get_named_gpio()
certainly can return -EPROBE_DEFER, but the caller would still need to
check it. The ideal case might be to just do:

ret = gpio_request(of_get_named_gpio(...));
if (ret)
    return ret;

and have gpio_request pass -EPROBE_DEFER from input to output.

i.e. only check the gpio_request() result code, not the
of_get_named_gpio() result code, and rely on gpio_request() to do
validation later.

But gpio_request takes an unsigned int, so the error code wouldn't
survive the translation:-(

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

[Mark Brown]

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

On Fri, Jun 29, 2012 at 09:30:51AM -0600, Stephen Warren wrote:

> I did consider making of_get_fixed_voltage_config() return a result
> code, but then it needs some other way of returning the pointer, so
> that seemed just as convoluted. Oh, I suppose it could use ERR_PTR()
> to do that; that'd be nice and simple. Would that do?

Meh, or just have the caller pass in the platform data rather than
allocate it in the function.

> Re: gpiolib doing it: How is that possible? of_get_named_gpio()
> certainly can return -EPROBE_DEFER, but the caller would still need to
> check it. The ideal case might be to just do:

> ret = gpio_request(of_get_named_gpio(...));
> if (ret)
>     return ret;

> and have gpio_request pass -EPROBE_DEFER from input to output.

Even better just have gpio_request() just return -EPROBE_DEFER instead
of -ENODEV.  There is no sane case where you'd request a GPIO you didn't
have a reasonable idea was going to be registered at some point (or
could be with appropriate system configuration) so we should just assume
that might happen.

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