Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1236313 > unrolled thread

[PATCH 1/6] Revert "regulator: core: Handle full constraints systems when resolving supplies"

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/6] Revert "regulator: core: Handle full constraints systems when resolving supplies" Sascha Hauer <s.hauer@pengutronix.de> - 2015-09-30 16:00 +0200
    Re: [PATCH 1/6] Revert "regulator: core: Handle full constraints  systems when resolving supplies" Mark Brown <broonie@kernel.org> - 2015-09-30 20:10 +0200
      Re: [PATCH 1/6] Revert "regulator: core: Handle full constraints  systems when resolving supplies" Sascha Hauer <s.hauer@pengutronix.de> - 2015-09-30 20:50 +0200
        Re: [PATCH 1/6] Revert "regulator: core: Handle full constraints  systems when resolving supplies" Mark Brown <broonie@kernel.org> - 2015-09-30 21:30 +0200

#1236313 — [PATCH 1/6] Revert "regulator: core: Handle full constraints systems when resolving supplies"

This reverts commit 9f7e25edb1575a6d2363dc003f9cc09d840657e2.

When a regulator A is registered and is supplied by regulator B which is
not yet registered then a regulator_get on regulator A will set the As
supply to the dummy regulator. This is not correct, we should return
-EPROBE_DEFER instead as done without this patch.
Of course reverting this patch brings back the issue it fixed, so this
is not a solution, but what is the correct solution?

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/regulator/core.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7849187..bd9db70 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1403,13 +1403,9 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 			return 0;
 		}
 
-		if (have_full_constraints()) {
-			r = dummy_regulator_rdev;
-		} else {
-			dev_err(dev, "Failed to resolve %s-supply for %s\n",
-				rdev->supply_name, rdev->desc->name);
-			return -EPROBE_DEFER;
-		}
+		dev_err(dev, "Failed to resolve %s-supply for %s\n",
+			rdev->supply_name, rdev->desc->name);
+		return -EPROBE_DEFER;
 	}
 
 	/* Recursively resolve the supply of the supply */
-- 
2.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1236605 — Re: [PATCH 1/6] Revert "regulator: core: Handle full constraints systems when resolving supplies"

[Multipart message — attachments visible in raw view] — view raw

On Wed, Sep 30, 2015 at 03:57:49PM +0200, Sascha Hauer wrote:
> This reverts commit 9f7e25edb1575a6d2363dc003f9cc09d840657e2.
> 
> When a regulator A is registered and is supplied by regulator B which is
> not yet registered then a regulator_get on regulator A will set the As
> supply to the dummy regulator. This is not correct, we should return
> -EPROBE_DEFER instead as done without this patch.

What makes you say this is not correct?  In a system with fully
specified supplies if we fail to resolve the supply we know that no
supply will ever appear and so substitute in the dummy on the assumption
that there is a supply with no software control.

[toc] | [prev] | [next] | [standalone]

#1236646 — Re: [PATCH 1/6] Revert "regulator: core: Handle full constraints systems when resolving supplies"

On Wed, Sep 30, 2015 at 07:02:13PM +0100, Mark Brown wrote:
> On Wed, Sep 30, 2015 at 03:57:49PM +0200, Sascha Hauer wrote:
> > This reverts commit 9f7e25edb1575a6d2363dc003f9cc09d840657e2.
> > 
> > When a regulator A is registered and is supplied by regulator B which is
> > not yet registered then a regulator_get on regulator A will set the As
> > supply to the dummy regulator. This is not correct, we should return
> > -EPROBE_DEFER instead as done without this patch.
> 
> What makes you say this is not correct?  In a system with fully
> specified supplies if we fail to resolve the supply we know that no
> supply will ever appear and so substitute in the dummy on the assumption
> that there is a supply with no software control.

The situation I'm in is:

		gpc: gpc@020dc000 {
			compatible = "fsl,imx6q-gpc";
			pu-supply = <&reg_pu>;
		};

		reg_pu: regulator-vddpu@140 {
			compatible = "fsl,anatop-regulator";
			vin-supply = <&vddsoc_reg>;
		};

		...

		pmic@58 {
			compatible = "dlg,da9063";
			regulators {
				...
				vddsoc_reg: bcore2 {
					regulator-min-microvolt = <730000>;
					regulator-max-microvolt = <1380000>;
					regulator-always-on;
				};
			};
		};

During boot the fsl,imx6q-gpc is probed and tries to get its regulator.
fsl,anatop-regulator is already registered, the dlg,da9063 is not.  The
call to regulator_get tries to resolve the supply using
regulator_resolve_supply. regulator_resolve_supply then calls
regulator_dev_lookup which returns NULL because the supply (vddsoc_reg)
is specified in the device tree but not yet present. &ret is correctly
filled with -EPROBE_DEFER in this case, but this is ignored. Instead due
to 9f7e25edb157 the dummy regulator is used. This case worked before
9f7e25edb157.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1236693 — Re: [PATCH 1/6] Revert "regulator: core: Handle full constraints systems when resolving supplies"

[Multipart message — attachments visible in raw view] — view raw

On Wed, Sep 30, 2015 at 08:48:12PM +0200, Sascha Hauer wrote:

> During boot the fsl,imx6q-gpc is probed and tries to get its regulator.
> fsl,anatop-regulator is already registered, the dlg,da9063 is not.  The
> call to regulator_get tries to resolve the supply using
> regulator_resolve_supply. regulator_resolve_supply then calls
> regulator_dev_lookup which returns NULL because the supply (vddsoc_reg)
> is specified in the device tree but not yet present. &ret is correctly
> filled with -EPROBE_DEFER in this case, but this is ignored. Instead due
> to 9f7e25edb157 the dummy regulator is used. This case worked before
> 9f7e25edb157.

Ah, I see - yes that's a bug.  Untested patch incoming, let me know if
that works.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web