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

Groups > linux.kernel > #1373425 > unrolled thread

[PATCH 1/5] regulator: core: Resolve supply earlier

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/5] regulator: core: Resolve supply earlier Thierry Reding <thierry.reding@gmail.com> - 2016-04-07 16:30 +0200
    [PATCH 5/5] regulator: as3722: Constify regulator ops Thierry Reding <thierry.reding@gmail.com> - 2016-04-07 16:30 +0200
      Applied "regulator: as3722: Constify regulator ops" to the regulator tree Mark Brown <broonie@kernel.org> - 2016-04-11 18:20 +0200
    [PATCH 2/5] regulator: core: Use parent voltage from the supply when bypassed Thierry Reding <thierry.reding@gmail.com> - 2016-04-07 16:30 +0200
      Applied "regulator: core: Use parent voltage from the supply when bypassed" to the regulator tree Mark Brown <broonie@kernel.org> - 2016-04-12 08:40 +0200
    Re: [PATCH 1/5] regulator: core: Resolve supply earlier Jon Hunter <jonathanh@nvidia.com> - 2016-04-11 13:00 +0200
      Re: [PATCH 1/5] regulator: core: Resolve supply earlier Thierry Reding <thierry.reding@gmail.com> - 2016-04-11 13:50 +0200
        Re: [PATCH 1/5] regulator: core: Resolve supply earlier Jon Hunter <jonathanh@nvidia.com> - 2016-04-11 14:20 +0200
          Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 14:50 +0200
          Re: [PATCH 1/5] regulator: core: Resolve supply earlier Javier Martinez Canillas <javier@osg.samsung.com> - 2016-04-11 16:10 +0200
        Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 15:00 +0200
          Re: [PATCH 1/5] regulator: core: Resolve supply earlier Thierry Reding <thierry.reding@gmail.com> - 2016-04-11 15:10 +0200
            Re: [PATCH 1/5] regulator: core: Resolve supply earlier Javier Martinez Canillas <javier@osg.samsung.com> - 2016-04-11 15:50 +0200
              Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 16:00 +0200
                Re: [PATCH 1/5] regulator: core: Resolve supply earlier Thierry Reding <thierry.reding@gmail.com> - 2016-04-11 16:10 +0200
                  Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 16:40 +0200
                    Re: [PATCH 1/5] regulator: core: Resolve supply earlier Thierry Reding <thierry.reding@gmail.com> - 2016-04-11 16:50 +0200
                      Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 18:00 +0200
            Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 16:00 +0200
    Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 16:10 +0200
      Re: [PATCH 1/5] regulator: core: Resolve supply earlier Thierry Reding <thierry.reding@gmail.com> - 2016-04-11 16:20 +0200
        Re: [PATCH 1/5] regulator: core: Resolve supply earlier Mark Brown <broonie@kernel.org> - 2016-04-11 16:20 +0200

Page 1 of 2  [1] 2  Next page →

#1373425 — [PATCH 1/5] regulator: core: Resolve supply earlier

From: Thierry Reding <treding@nvidia.com>

Subsequent patches will need access to the parent supply from within the
set_machine_constraints() function to properly implement bypass mode. If
the parent supply hasn't been resolved by that time the voltage can't be
queried.

Also, by making sure the supply is resolved early most of the changes in
set_machine_constraints() don't have to be undone if resolution fails.

Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/regulator/core.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2786d251b1cc..cc0333a79924 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3972,18 +3972,27 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	dev_set_drvdata(&rdev->dev, rdev);
 
+	if (init_data && init_data->supply_regulator)
+		rdev->supply_name = init_data->supply_regulator;
+	else if (regulator_desc->supply_name)
+		rdev->supply_name = regulator_desc->supply_name;
+
+	/*
+	 * set_machine_constraints() needs the supply to be resolved in order
+	 * to support querying the current voltage in bypass mode. Resolve it
+	 * here to more easily handle deferred probing.
+	 */
+	ret = regulator_resolve_supply(rdev);
+	if (ret < 0)
+		goto scrub;
+
 	/* set regulator constraints */
 	if (init_data)
 		constraints = &init_data->constraints;
 
 	ret = set_machine_constraints(rdev, constraints);
 	if (ret < 0)
-		goto scrub;
-
-	if (init_data && init_data->supply_regulator)
-		rdev->supply_name = init_data->supply_regulator;
-	else if (regulator_desc->supply_name)
-		rdev->supply_name = regulator_desc->supply_name;
+		goto tumble;
 
 	/* add consumers devices */
 	if (init_data) {
@@ -4010,7 +4019,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 unset_supplies:
 	unset_regulator_supplies(rdev);
+tumble:
+	if (rdev->supply) {
+		if (_regulator_is_enabled(rdev))
+			regulator_disable(rdev->supply);
 
+		_regulator_put(rdev->supply);
+	}
 scrub:
 	regulator_ena_gpio_free(rdev);
 	device_unregister(&rdev->dev);
-- 
2.8.0

[toc] | [next] | [standalone]

#1373428 — [PATCH 5/5] regulator: as3722: Constify regulator ops

From: Thierry Reding <treding@nvidia.com>

A const pointer to regulator ops is stored in regulator descriptors. The
operations never need to be modified, so define them as const as a hint
to the compiler that they can go into .rodata.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/regulator/as3722-regulator.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/as3722-regulator.c b/drivers/regulator/as3722-regulator.c
index 35a7c647f6a9..66337e12719b 100644
--- a/drivers/regulator/as3722-regulator.c
+++ b/drivers/regulator/as3722-regulator.c
@@ -372,7 +372,7 @@ static int as3722_ldo_set_current_limit(struct regulator_dev *rdev,
 			AS3722_LDO_ILIMIT_MASK, reg);
 }
 
-static struct regulator_ops as3722_ldo0_ops = {
+static const struct regulator_ops as3722_ldo0_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -383,7 +383,7 @@ static struct regulator_ops as3722_ldo0_ops = {
 	.set_current_limit = as3722_ldo_set_current_limit,
 };
 
-static struct regulator_ops as3722_ldo0_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo0_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -415,7 +415,7 @@ static int as3722_ldo3_get_current_limit(struct regulator_dev *rdev)
 	return 150000;
 }
 
-static struct regulator_ops as3722_ldo3_ops = {
+static const struct regulator_ops as3722_ldo3_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -425,14 +425,14 @@ static struct regulator_ops as3722_ldo3_ops = {
 	.get_current_limit = as3722_ldo3_get_current_limit,
 };
 
-static struct regulator_ops as3722_ldo3_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo3_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_current_limit = as3722_ldo3_get_current_limit,
 };
 
-static struct regulator_ops as3722_ldo6_ops = {
+static const struct regulator_ops as3722_ldo6_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -446,7 +446,7 @@ static struct regulator_ops as3722_ldo6_ops = {
 	.set_bypass = regulator_set_bypass_regmap,
 };
 
-static struct regulator_ops as3722_ldo6_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo6_extcntrl_ops = {
 	.map_voltage = regulator_map_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
@@ -463,7 +463,7 @@ static const struct regulator_linear_range as3722_ldo_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1725000, 0x40, 0x7F, 25000),
 };
 
-static struct regulator_ops as3722_ldo_ops = {
+static const struct regulator_ops as3722_ldo_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -475,7 +475,7 @@ static struct regulator_ops as3722_ldo_ops = {
 	.set_current_limit = as3722_ldo_set_current_limit,
 };
 
-static struct regulator_ops as3722_ldo_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo_extcntrl_ops = {
 	.map_voltage = regulator_map_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
@@ -641,7 +641,7 @@ static const struct regulator_linear_range as3722_sd2345_ranges[] = {
 	REGULATOR_LINEAR_RANGE(2650000, 0x71, 0x7F, 50000),
 };
 
-static struct regulator_ops as3722_sd016_ops = {
+static const struct regulator_ops as3722_sd016_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -655,7 +655,7 @@ static struct regulator_ops as3722_sd016_ops = {
 	.set_mode = as3722_sd_set_mode,
 };
 
-static struct regulator_ops as3722_sd016_extcntrl_ops = {
+static const struct regulator_ops as3722_sd016_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.map_voltage = regulator_map_voltage_linear,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
@@ -666,7 +666,7 @@ static struct regulator_ops as3722_sd016_extcntrl_ops = {
 	.set_mode = as3722_sd_set_mode,
 };
 
-static struct regulator_ops as3722_sd2345_ops = {
+static const struct regulator_ops as3722_sd2345_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -678,7 +678,7 @@ static struct regulator_ops as3722_sd2345_ops = {
 	.set_mode = as3722_sd_set_mode,
 };
 
-static struct regulator_ops as3722_sd2345_extcntrl_ops = {
+static const struct regulator_ops as3722_sd2345_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear_range,
 	.map_voltage = regulator_map_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -785,7 +785,7 @@ static int as3722_regulator_probe(struct platform_device *pdev)
 	struct as3722_regulator_config_data *reg_config;
 	struct regulator_dev *rdev;
 	struct regulator_config config = { };
-	struct regulator_ops *ops;
+	const struct regulator_ops *ops;
 	int id;
 	int ret;
 
-- 
2.8.0

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

#1376149 — Applied "regulator: as3722: Constify regulator ops" to the regulator tree

The patch

   regulator: as3722: Constify regulator ops

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 162c5a368fb788f040e9c51a1251ac36d80bff32 Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Thu, 7 Apr 2016 16:22:39 +0200
Subject: [PATCH] regulator: as3722: Constify regulator ops

A const pointer to regulator ops is stored in regulator descriptors. The
operations never need to be modified, so define them as const as a hint
to the compiler that they can go into .rodata.

Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/as3722-regulator.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/as3722-regulator.c b/drivers/regulator/as3722-regulator.c
index 35a7c647f6a9..66337e12719b 100644
--- a/drivers/regulator/as3722-regulator.c
+++ b/drivers/regulator/as3722-regulator.c
@@ -372,7 +372,7 @@ static int as3722_ldo_set_current_limit(struct regulator_dev *rdev,
 			AS3722_LDO_ILIMIT_MASK, reg);
 }
 
-static struct regulator_ops as3722_ldo0_ops = {
+static const struct regulator_ops as3722_ldo0_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -383,7 +383,7 @@ static struct regulator_ops as3722_ldo0_ops = {
 	.set_current_limit = as3722_ldo_set_current_limit,
 };
 
-static struct regulator_ops as3722_ldo0_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo0_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -415,7 +415,7 @@ static int as3722_ldo3_get_current_limit(struct regulator_dev *rdev)
 	return 150000;
 }
 
-static struct regulator_ops as3722_ldo3_ops = {
+static const struct regulator_ops as3722_ldo3_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -425,14 +425,14 @@ static struct regulator_ops as3722_ldo3_ops = {
 	.get_current_limit = as3722_ldo3_get_current_limit,
 };
 
-static struct regulator_ops as3722_ldo3_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo3_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_current_limit = as3722_ldo3_get_current_limit,
 };
 
-static struct regulator_ops as3722_ldo6_ops = {
+static const struct regulator_ops as3722_ldo6_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -446,7 +446,7 @@ static struct regulator_ops as3722_ldo6_ops = {
 	.set_bypass = regulator_set_bypass_regmap,
 };
 
-static struct regulator_ops as3722_ldo6_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo6_extcntrl_ops = {
 	.map_voltage = regulator_map_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
@@ -463,7 +463,7 @@ static const struct regulator_linear_range as3722_ldo_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1725000, 0x40, 0x7F, 25000),
 };
 
-static struct regulator_ops as3722_ldo_ops = {
+static const struct regulator_ops as3722_ldo_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -475,7 +475,7 @@ static struct regulator_ops as3722_ldo_ops = {
 	.set_current_limit = as3722_ldo_set_current_limit,
 };
 
-static struct regulator_ops as3722_ldo_extcntrl_ops = {
+static const struct regulator_ops as3722_ldo_extcntrl_ops = {
 	.map_voltage = regulator_map_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
@@ -641,7 +641,7 @@ static const struct regulator_linear_range as3722_sd2345_ranges[] = {
 	REGULATOR_LINEAR_RANGE(2650000, 0x71, 0x7F, 50000),
 };
 
-static struct regulator_ops as3722_sd016_ops = {
+static const struct regulator_ops as3722_sd016_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -655,7 +655,7 @@ static struct regulator_ops as3722_sd016_ops = {
 	.set_mode = as3722_sd_set_mode,
 };
 
-static struct regulator_ops as3722_sd016_extcntrl_ops = {
+static const struct regulator_ops as3722_sd016_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.map_voltage = regulator_map_voltage_linear,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
@@ -666,7 +666,7 @@ static struct regulator_ops as3722_sd016_extcntrl_ops = {
 	.set_mode = as3722_sd_set_mode,
 };
 
-static struct regulator_ops as3722_sd2345_ops = {
+static const struct regulator_ops as3722_sd2345_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -678,7 +678,7 @@ static struct regulator_ops as3722_sd2345_ops = {
 	.set_mode = as3722_sd_set_mode,
 };
 
-static struct regulator_ops as3722_sd2345_extcntrl_ops = {
+static const struct regulator_ops as3722_sd2345_extcntrl_ops = {
 	.list_voltage = regulator_list_voltage_linear_range,
 	.map_voltage = regulator_map_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -785,7 +785,7 @@ static int as3722_regulator_probe(struct platform_device *pdev)
 	struct as3722_regulator_config_data *reg_config;
 	struct regulator_dev *rdev;
 	struct regulator_config config = { };
-	struct regulator_ops *ops;
+	const struct regulator_ops *ops;
 	int id;
 	int ret;
 
-- 
2.8.0.rc3

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

#1373430 — [PATCH 2/5] regulator: core: Use parent voltage from the supply when bypassed

From: Mark Brown <broonie@kernel.org>

When a regulator is in bypass mode it is functioning as a switch
returning the voltage set in the regulator will not give the voltage
being output by the regulator as it's just passing through its supply.
This means that when we are getting the voltage from a regulator we
should check to see if it is in bypass mode and if it is we should
report the voltage from the supply rather than that which is set on the
regulator.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
[treding@nvidia.com: return early for bypass mode]
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/regulator/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cc0333a79924..23c8c4c86389 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3118,6 +3118,20 @@ EXPORT_SYMBOL_GPL(regulator_sync_voltage);
 static int _regulator_get_voltage(struct regulator_dev *rdev)
 {
 	int sel, ret;
+	bool bypassed;
+
+	if (rdev->desc->ops->get_bypass) {
+		ret = rdev->desc->ops->get_bypass(rdev, &bypassed);
+		if (ret < 0)
+			return ret;
+		if (bypassed) {
+			/* if bypassed the regulator must have a supply */
+			if (!rdev->supply)
+				return -EINVAL;
+
+			return _regulator_get_voltage(rdev->supply->rdev);
+		}
+	}
 
 	if (rdev->desc->ops->get_voltage_sel) {
 		sel = rdev->desc->ops->get_voltage_sel(rdev);
-- 
2.8.0

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

#1376502 — Applied "regulator: core: Use parent voltage from the supply when bypassed" to the regulator tree

The patch

   regulator: core: Use parent voltage from the supply when bypassed

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From fef95019016ac10e250d2c67a3c97af5797e3938 Mon Sep 17 00:00:00 2001
From: Mark Brown <broonie@kernel.org>
Date: Thu, 7 Apr 2016 16:22:36 +0200
Subject: [PATCH] regulator: core: Use parent voltage from the supply when
 bypassed

When a regulator is in bypass mode it is functioning as a switch
returning the voltage set in the regulator will not give the voltage
being output by the regulator as it's just passing through its supply.
This means that when we are getting the voltage from a regulator we
should check to see if it is in bypass mode and if it is we should
report the voltage from the supply rather than that which is set on the
regulator.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
[treding@nvidia.com: return early for bypass mode]
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e0b764284773..990fd7b3da7d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3109,6 +3109,20 @@ EXPORT_SYMBOL_GPL(regulator_sync_voltage);
 static int _regulator_get_voltage(struct regulator_dev *rdev)
 {
 	int sel, ret;
+	bool bypassed;
+
+	if (rdev->desc->ops->get_bypass) {
+		ret = rdev->desc->ops->get_bypass(rdev, &bypassed);
+		if (ret < 0)
+			return ret;
+		if (bypassed) {
+			/* if bypassed the regulator must have a supply */
+			if (!rdev->supply)
+				return -EINVAL;
+
+			return _regulator_get_voltage(rdev->supply->rdev);
+		}
+	}
 
 	if (rdev->desc->ops->get_voltage_sel) {
 		sel = rdev->desc->ops->get_voltage_sel(rdev);
-- 
2.8.0.rc3

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

#1375687

Hi Thierry,

On 07/04/16 15:22, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Subsequent patches will need access to the parent supply from within the
> set_machine_constraints() function to properly implement bypass mode. If
> the parent supply hasn't been resolved by that time the voltage can't be
> queried.
> 
> Also, by making sure the supply is resolved early most of the changes in
> set_machine_constraints() don't have to be undone if resolution fails.
> 
> Suggested-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/regulator/core.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 2786d251b1cc..cc0333a79924 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3972,18 +3972,27 @@ regulator_register(const struct regulator_desc *regulator_desc,
>  
>  	dev_set_drvdata(&rdev->dev, rdev);
>  
> +	if (init_data && init_data->supply_regulator)
> +		rdev->supply_name = init_data->supply_regulator;
> +	else if (regulator_desc->supply_name)
> +		rdev->supply_name = regulator_desc->supply_name;
> +
> +	/*
> +	 * set_machine_constraints() needs the supply to be resolved in order
> +	 * to support querying the current voltage in bypass mode. Resolve it
> +	 * here to more easily handle deferred probing.
> +	 */
> +	ret = regulator_resolve_supply(rdev);
> +	if (ret < 0)
> +		goto scrub;
> +

Thanks for sending this. However, I think that calling
regulator_resolve_supply() can cause a deadlock, because the
regulator_list_mutex is held at this point and
regulator_resolve_supply() calls regulator_dev_lookup() which may try to
request the mutex again.

So may be we need to move this call after the call to
regulator_of_get_init_data() before we acquire the mutex.

Also, if we add this call, then I am wondering if we still need ...

	class_for_each_device(&regulator_class, NULL, NULL,
			      regulator_register_resolve_supply);

Cheers
Jon

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

#1375745

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

On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote:
> Hi Thierry,
> 
> On 07/04/16 15:22, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Subsequent patches will need access to the parent supply from within the
> > set_machine_constraints() function to properly implement bypass mode. If
> > the parent supply hasn't been resolved by that time the voltage can't be
> > queried.
> > 
> > Also, by making sure the supply is resolved early most of the changes in
> > set_machine_constraints() don't have to be undone if resolution fails.
> > 
> > Suggested-by: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/regulator/core.c | 27 +++++++++++++++++++++------
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> > index 2786d251b1cc..cc0333a79924 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
> > @@ -3972,18 +3972,27 @@ regulator_register(const struct regulator_desc *regulator_desc,
> >  
> >  	dev_set_drvdata(&rdev->dev, rdev);
> >  
> > +	if (init_data && init_data->supply_regulator)
> > +		rdev->supply_name = init_data->supply_regulator;
> > +	else if (regulator_desc->supply_name)
> > +		rdev->supply_name = regulator_desc->supply_name;
> > +
> > +	/*
> > +	 * set_machine_constraints() needs the supply to be resolved in order
> > +	 * to support querying the current voltage in bypass mode. Resolve it
> > +	 * here to more easily handle deferred probing.
> > +	 */
> > +	ret = regulator_resolve_supply(rdev);
> > +	if (ret < 0)
> > +		goto scrub;
> > +
> 
> Thanks for sending this. However, I think that calling
> regulator_resolve_supply() can cause a deadlock, because the
> regulator_list_mutex is held at this point and
> regulator_resolve_supply() calls regulator_dev_lookup() which may try to
> request the mutex again.

True... I never encountered that case in my testing. I'm not sure
exactly why, though.

> So may be we need to move this call after the call to
> regulator_of_get_init_data() before we acquire the mutex.

I don't think that'll work. regulator_resolve_supply() depends on some
operations performed much later (such as rdev->dev.parent being set).

Perhaps moving the locking of the regulator_list_mutex down instead
could work. It seems to me like the first place where it would need to
be held is set_machine_constraints().

> Also, if we add this call, then I am wondering if we still need ...
> 
> 	class_for_each_device(&regulator_class, NULL, NULL,
> 			      regulator_register_resolve_supply);

Possibly not. That line was introduced to hook up existing orphan
regulators with their parents when they were registered, but I guess
since we now always defer probe if a parent isn't registered yet the
line would become a no-op.

Thierry

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

#1375770

On 11/04/16 12:46, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote:
>> Hi Thierry,
>>
>> On 07/04/16 15:22, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Subsequent patches will need access to the parent supply from within the
>>> set_machine_constraints() function to properly implement bypass mode. If
>>> the parent supply hasn't been resolved by that time the voltage can't be
>>> queried.
>>>
>>> Also, by making sure the supply is resolved early most of the changes in
>>> set_machine_constraints() don't have to be undone if resolution fails.
>>>
>>> Suggested-by: Mark Brown <broonie@kernel.org>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  drivers/regulator/core.c | 27 +++++++++++++++++++++------
>>>  1 file changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index 2786d251b1cc..cc0333a79924 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -3972,18 +3972,27 @@ regulator_register(const struct regulator_desc *regulator_desc,
>>>  
>>>  	dev_set_drvdata(&rdev->dev, rdev);
>>>  
>>> +	if (init_data && init_data->supply_regulator)
>>> +		rdev->supply_name = init_data->supply_regulator;
>>> +	else if (regulator_desc->supply_name)
>>> +		rdev->supply_name = regulator_desc->supply_name;
>>> +
>>> +	/*
>>> +	 * set_machine_constraints() needs the supply to be resolved in order
>>> +	 * to support querying the current voltage in bypass mode. Resolve it
>>> +	 * here to more easily handle deferred probing.
>>> +	 */
>>> +	ret = regulator_resolve_supply(rdev);
>>> +	if (ret < 0)
>>> +		goto scrub;
>>> +
>>
>> Thanks for sending this. However, I think that calling
>> regulator_resolve_supply() can cause a deadlock, because the
>> regulator_list_mutex is held at this point and
>> regulator_resolve_supply() calls regulator_dev_lookup() which may try to
>> request the mutex again.
> 
> True... I never encountered that case in my testing. I'm not sure
> exactly why, though.

I believe that you may see it on Tegra114 [0], however, that was the
only tegra board I have seen a deadlock here in the past.

>> So may be we need to move this call after the call to
>> regulator_of_get_init_data() before we acquire the mutex.
> 
> I don't think that'll work. regulator_resolve_supply() depends on some
> operations performed much later (such as rdev->dev.parent being set).

Hmmm ... yes I was not sure if there was something else needed.

> Perhaps moving the locking of the regulator_list_mutex down instead
> could work. It seems to me like the first place where it would need to
> be held is set_machine_constraints().

Yes either that or we add a variable to regulator_resolve_supply() and
regulator_dev_lookup() that indicates if the mutex is already held.
Moving the acquistion of mutex would be best/cleaner if that is ok.

>> Also, if we add this call, then I am wondering if we still need ...
>>
>> 	class_for_each_device(&regulator_class, NULL, NULL,
>> 			      regulator_register_resolve_supply);
> 
> Possibly not. That line was introduced to hook up existing orphan
> regulators with their parents when they were registered, but I guess
> since we now always defer probe if a parent isn't registered yet the
> line would become a no-op.

OK. I added Javier to the thread as he added this so whatever we propose
hopefully he can test as well.

Cheers
Jon

[0] http://marc.info/?l=linux-tegra&m=145935416701022&w=2

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

#1375791

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

On Mon, Apr 11, 2016 at 01:19:26PM +0100, Jon Hunter wrote:
> On 11/04/16 12:46, Thierry Reding wrote:

> > Perhaps moving the locking of the regulator_list_mutex down instead
> > could work. It seems to me like the first place where it would need to
> > be held is set_machine_constraints().

> Yes either that or we add a variable to regulator_resolve_supply() and
> regulator_dev_lookup() that indicates if the mutex is already held.

No, that sort of conditional locking is horrible and error prone.

> Moving the acquistion of mutex would be best/cleaner if that is ok.

Yes, we need to reorganize the locking.

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

#1375983

Hello,

On 04/11/2016 08:19 AM, Jon Hunter wrote:
> 
> On 11/04/16 12:46, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote:
>>> Hi Thierry,
>>>
>>> On 07/04/16 15:22, Thierry Reding wrote:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> Subsequent patches will need access to the parent supply from within the
>>>> set_machine_constraints() function to properly implement bypass mode. If
>>>> the parent supply hasn't been resolved by that time the voltage can't be
>>>> queried.
>>>>
>>>> Also, by making sure the supply is resolved early most of the changes in
>>>> set_machine_constraints() don't have to be undone if resolution fails.
>>>>
>>>> Suggested-by: Mark Brown <broonie@kernel.org>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>  drivers/regulator/core.c | 27 +++++++++++++++++++++------
>>>>  1 file changed, 21 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>>> index 2786d251b1cc..cc0333a79924 100644
>>>> --- a/drivers/regulator/core.c
>>>> +++ b/drivers/regulator/core.c
>>>> @@ -3972,18 +3972,27 @@ regulator_register(const struct regulator_desc *regulator_desc,
>>>>  
>>>>  	dev_set_drvdata(&rdev->dev, rdev);
>>>>  
>>>> +	if (init_data && init_data->supply_regulator)
>>>> +		rdev->supply_name = init_data->supply_regulator;
>>>> +	else if (regulator_desc->supply_name)
>>>> +		rdev->supply_name = regulator_desc->supply_name;
>>>> +
>>>> +	/*
>>>> +	 * set_machine_constraints() needs the supply to be resolved in order
>>>> +	 * to support querying the current voltage in bypass mode. Resolve it
>>>> +	 * here to more easily handle deferred probing.
>>>> +	 */
>>>> +	ret = regulator_resolve_supply(rdev);
>>>> +	if (ret < 0)
>>>> +		goto scrub;
>>>> +
>>>
>>> Thanks for sending this. However, I think that calling
>>> regulator_resolve_supply() can cause a deadlock, because the
>>> regulator_list_mutex is held at this point and
>>> regulator_resolve_supply() calls regulator_dev_lookup() which may try to
>>> request the mutex again.
>>
>> True... I never encountered that case in my testing. I'm not sure
>> exactly why, though.
> 
> I believe that you may see it on Tegra114 [0], however, that was the
> only tegra board I have seen a deadlock here in the past.
>

I guess Thierry didn't see that error because it only happens on platforms
that do legacy (non-DT) regulators lookup. For OF registered regulators,
the lookup logic doesn't grab the regulator list mutex.

That was in fact why I didn't notice that issue introduced by my patch that
later was fixed by Jon.
 
>>> So may be we need to move this call after the call to
>>> regulator_of_get_init_data() before we acquire the mutex.
>>
>> I don't think that'll work. regulator_resolve_supply() depends on some
>> operations performed much later (such as rdev->dev.parent being set).
> 
> Hmmm ... yes I was not sure if there was something else needed.
> 
>> Perhaps moving the locking of the regulator_list_mutex down instead
>> could work. It seems to me like the first place where it would need to
>> be held is set_machine_constraints().
> 
> Yes either that or we add a variable to regulator_resolve_supply() and
> regulator_dev_lookup() that indicates if the mutex is already held.
> Moving the acquistion of mutex would be best/cleaner if that is ok.
> 
>>> Also, if we add this call, then I am wondering if we still need ...
>>>
>>> 	class_for_each_device(&regulator_class, NULL, NULL,
>>> 			      regulator_register_resolve_supply);
>>
>> Possibly not. That line was introduced to hook up existing orphan
>> regulators with their parents when they were registered, but I guess
>> since we now always defer probe if a parent isn't registered yet the
>> line would become a no-op.
> 
> OK. I added Javier to the thread as he added this so whatever we propose
> hopefully he can test as well.
>

Sure, I'll be able to test the patches on the platform where I had issues
that motivated that change. But as mentioned in the other email, I think
this patch will cause regressions on other platforms due moving the supply
resolution to registration again.

> Cheers
> Jon
> 
> [0] http://marc.info/?l=linux-tegra&m=145935416701022&w=2
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

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

#1375810

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

On Mon, Apr 11, 2016 at 01:46:12PM +0200, Thierry Reding wrote:
> On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote:

> > Also, if we add this call, then I am wondering if we still need ...
> > 
> > 	class_for_each_device(&regulator_class, NULL, NULL,
> > 			      regulator_register_resolve_supply);

> Possibly not. That line was introduced to hook up existing orphan
> regulators with their parents when they were registered, but I guess
> since we now always defer probe if a parent isn't registered yet the
> line would become a no-op.

That then takes us right the way back to the original problem where
people we're getting upset at the number of probe deferrals they were
seeing and more importantly we didn't have any way of sorting out
dependencies within a single PMIC if the parents weren't registered
before their children.

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

#1375819

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

On Mon, Apr 11, 2016 at 01:58:14PM +0100, Mark Brown wrote:
> On Mon, Apr 11, 2016 at 01:46:12PM +0200, Thierry Reding wrote:
> > On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote:
> 
> > > Also, if we add this call, then I am wondering if we still need ...
> > > 
> > > 	class_for_each_device(&regulator_class, NULL, NULL,
> > > 			      regulator_register_resolve_supply);
> 
> > Possibly not. That line was introduced to hook up existing orphan
> > regulators with their parents when they were registered, but I guess
> > since we now always defer probe if a parent isn't registered yet the
> > line would become a no-op.
> 
> That then takes us right the way back to the original problem where
> people we're getting upset at the number of probe deferrals they were
> seeing and more importantly we didn't have any way of sorting out
> dependencies within a single PMIC if the parents weren't registered
> before their children.

Isn't that usually solved by making each regulator of a PMIC a separate
device (platform device, typically, for MFD devices? That way each of
them is probed separately allowing the dependency cycle to be broken.

Thierry

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

#1375928

[adding Bjorn Andersson to cc list]

Hello,

On 04/11/2016 09:09 AM, Thierry Reding wrote:
> On Mon, Apr 11, 2016 at 01:58:14PM +0100, Mark Brown wrote:
>> On Mon, Apr 11, 2016 at 01:46:12PM +0200, Thierry Reding wrote:
>>> On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote:
>>
>>>> Also, if we add this call, then I am wondering if we still need ...
>>>>
>>>> 	class_for_each_device(&regulator_class, NULL, NULL,
>>>> 			      regulator_register_resolve_supply);
>>
>>> Possibly not. That line was introduced to hook up existing orphan
>>> regulators with their parents when they were registered, but I guess
>>> since we now always defer probe if a parent isn't registered yet the
>>> line would become a no-op.
>>
>> That then takes us right the way back to the original problem where
>> people we're getting upset at the number of probe deferrals they were
>> seeing and more importantly we didn't have any way of sorting out
>> dependencies within a single PMIC if the parents weren't registered
>> before their children.
> 
> Isn't that usually solved by making each regulator of a PMIC a separate
> device (platform device, typically, for MFD devices? That way each of
> them is probed separately allowing the dependency cycle to be broken.
>

IIRC the problem was that in some systems 2 PMICs can have circular
dependencies. That is, PMIC A can have as input supply a regulator
from PMIC B and PMIC B can have as input supply a regulator from A.

So the parent supply resolution was moved from regulator_register to
regulator_get in commit 6261b06de565 ("regulator: Defer lookup of
supply to regulator_get").

That way, regulators could be registered out of order and the supply
looked up only on get. The side effect of that change was that only
regulators that are get will resolve their parent supplies and that
is why commit 5e3ca2b349b1 ("regulator: Try to resolve regulators
supplies on registration") was needed for regulators that don't have
a client driver that looks them up (usually the always-on ones).

The latter commit tries to resolve the parent supply on registration
but it's only enforced on get to allow out or order registration of
parent supplies.

Now $SUBJECT will break the use case for Bjorn's commit AFAIU.

> Thierry
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

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

#1375943

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

On Mon, Apr 11, 2016 at 09:45:12AM -0400, Javier Martinez Canillas wrote:

> Now $SUBJECT will break the use case for Bjorn's commit AFAIU.

Yes, it'll break some systems.

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

#1375995

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

On Mon, Apr 11, 2016 at 02:57:15PM +0100, Mark Brown wrote:
> On Mon, Apr 11, 2016 at 09:45:12AM -0400, Javier Martinez Canillas wrote:
> 
> > Now $SUBJECT will break the use case for Bjorn's commit AFAIU.
> 
> Yes, it'll break some systems.

Okay, so how do we proceed here? Currently Jetson TK1 is broken because
bypass mode requires the parent to be available at probe time due to new
code that's now doing a regulator_get_voltage() during the initial call
to set_machine_constraints().

Perhaps to unbreak boards the original commit that caused this failure
fa93fd4ecc9c ("regulator: core: Ensure we are at least in bounds for our
constraints") should be reverted for now?

Thierry

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

#1376030

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

On Mon, Apr 11, 2016 at 04:07:30PM +0200, Thierry Reding wrote:

> Okay, so how do we proceed here? Currently Jetson TK1 is broken because
> bypass mode requires the parent to be available at probe time due to new
> code that's now doing a regulator_get_voltage() during the initial call
> to set_machine_constraints().

I think we should be doing what I'd expected this series to do and
looking up the supply as and when we need it when applying constraints.  
That will only affect systems where there is a practical issue which
should minimise the impact.  Long term we want a bigger refactoring but
I think we need to sort out what's going on with probe ordering in
general before we do that, that's part of the problem here - people
really aren't happy with deferral and for good reason.

> Perhaps to unbreak boards the original commit that caused this failure
> fa93fd4ecc9c ("regulator: core: Ensure we are at least in bounds for our
> constraints") should be reverted for now?

That breaks other boards that start up with out of spec variable voltage
regulators and therefore fail to probe some of the devices using those
regulators.

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

#1376041

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

On Mon, Apr 11, 2016 at 03:32:39PM +0100, Mark Brown wrote:
> On Mon, Apr 11, 2016 at 04:07:30PM +0200, Thierry Reding wrote:
> 
> > Okay, so how do we proceed here? Currently Jetson TK1 is broken because
> > bypass mode requires the parent to be available at probe time due to new
> > code that's now doing a regulator_get_voltage() during the initial call
> > to set_machine_constraints().
> 
> I think we should be doing what I'd expected this series to do and
> looking up the supply as and when we need it when applying constraints.  
> That will only affect systems where there is a practical issue which
> should minimise the impact.

I must have misinterpreted our discussion on IRC, then, because I
thought this was exactly what you had been expecting. =\ I'll go look
for my earlier patch and repost.

> Long term we want a bigger refactoring but
> I think we need to sort out what's going on with probe ordering in
> general before we do that, that's part of the problem here - people
> really aren't happy with deferral and for good reason.

What happened to correctness first? I thought we had at some point all
agreed that even if deferred probe wasn't perfect it would at least give
us correct results. And if all the code in place to properly establish
the dependencies we could rid ourselves of all the downsides at once if
ever we came up with a better alternative.

Thierry

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

#1376119

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

On Mon, Apr 11, 2016 at 04:49:02PM +0200, Thierry Reding wrote:
> On Mon, Apr 11, 2016 at 03:32:39PM +0100, Mark Brown wrote:

> > Long term we want a bigger refactoring but
> > I think we need to sort out what's going on with probe ordering in
> > general before we do that, that's part of the problem here - people
> > really aren't happy with deferral and for good reason.

> What happened to correctness first? I thought we had at some point all
> agreed that even if deferred probe wasn't perfect it would at least give
> us correct results. And if all the code in place to properly establish
> the dependencies we could rid ourselves of all the downsides at once if
> ever we came up with a better alternative.

This has never been completely correct since it predates deferred probe
in the first place and was originally relying on init ordering.  Trying
to use deferred probe unconditionally right now would mean rewriting the
registration section of almost every regulator driver which seems a
bigger and more error prone process better approached after the current
issues are resolved.  Without doing that it'd just be shuffling the
problem around again and I'm not convinced it's a good idea to rush such
a large change.  If we just defer in the cases where we have identified
a need to defer that takes a lot of the pressure off and reduces the
risks, a big part of why this is coming up is that we made a change that
affects all drivers so it seems better not to continue making such broad
changes in a hurry.  

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

#1375956

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

On Mon, Apr 11, 2016 at 03:09:14PM +0200, Thierry Reding wrote:
> On Mon, Apr 11, 2016 at 01:58:14PM +0100, Mark Brown wrote:

> > That then takes us right the way back to the original problem where
> > people we're getting upset at the number of probe deferrals they were
> > seeing and more importantly we didn't have any way of sorting out
> > dependencies within a single PMIC if the parents weren't registered
> > before their children.

> Isn't that usually solved by making each regulator of a PMIC a separate
> device (platform device, typically, for MFD devices? That way each of
> them is probed separately allowing the dependency cycle to be broken.

That's not something we've actually done and it's going to be an
enormous pain for driver authors without helpers to make the devices,
IIRC some systems are depending on this happening at the minute.  I can
also see the complaints starting to flood in about kicking off more
deferred probes.

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

#1375986

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

On Thu, Apr 07, 2016 at 04:22:35PM +0200, Thierry Reding wrote:

> +	/*
> +	 * set_machine_constraints() needs the supply to be resolved in order
> +	 * to support querying the current voltage in bypass mode. Resolve it
> +	 * here to more easily handle deferred probing.
> +	 */
> +	ret = regulator_resolve_supply(rdev);
> +	if (ret < 0)
> +		goto scrub;

This shouldn't be a hard dependency: most regulators won't be in bypass
mode or otherwise depend on their parents enough to need this.

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web