[PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Nishanth Menon]

While attempting to try and get each of the regulator nodes of palmas
to be capable of using vin-supply, I noticed a few cleanups and
associated bug fixes.

The series is based on next-20160504 and is split up to aid
readability of the patch.

Tested on OMAP5-uevm and DRA7 platform.

Nishanth Menon (4):
  regulator: tps65917/palmas: Simplify multiple dereference of
    ddata->palmas_matches[idx]
  regulator: tps65917/palmas: Simplify multiple dereference of
    pdata->reg_init[idx]
  regulator: tps65917/palmas: Handle possible memory allocation failure
  regulator: tps65917/palmas: Simplify multiple dereference of
    match->of_node

 drivers/regulator/palmas-regulator.c | 62 +++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 29 deletions(-)

-- 
2.8.0

[PATCH 1/4] regulator: tps65917/palmas: Simplify multiple dereference of ddata->palmas_matches[idx]

[Nishanth Menon]

Converting dt to platform data logic involves picking up information
that is unique per regulator, however we can improve readability of
the code by dereferencing ddata->palmas_matches[idx] once in the loop.

While at it fix reuse of generic palmas_matches common variable
while reporting error for a specific regulator (which may be from
65917/palmas list).

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/regulator/palmas-regulator.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index c83e06b2cedb..41b4e94a8d7d 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1491,21 +1491,23 @@ static void palmas_dt_to_pdata(struct device *dev,
 	}
 
 	for (idx = 0; idx < ddata->max_reg; idx++) {
-		if (!ddata->palmas_matches[idx].init_data ||
-		    !ddata->palmas_matches[idx].of_node)
+		static struct of_regulator_match *match;
+
+		match = &ddata->palmas_matches[idx];
+
+		if (!match->init_data || !match->of_node)
 			continue;
 
-		pdata->reg_data[idx] = ddata->palmas_matches[idx].init_data;
+		pdata->reg_data[idx] = match->init_data;
 
 		pdata->reg_init[idx] = devm_kzalloc(dev,
 				sizeof(struct palmas_reg_init), GFP_KERNEL);
 
 		pdata->reg_init[idx]->warm_reset =
-			of_property_read_bool(ddata->palmas_matches[idx].of_node,
-					      "ti,warm-reset");
+			of_property_read_bool(match->of_node, "ti,warm-reset");
 
-		ret = of_property_read_u32(ddata->palmas_matches[idx].of_node,
-					   "ti,roof-floor", &prop);
+		ret = of_property_read_u32(match->of_node, "ti,roof-floor",
+					   &prop);
 		/* EINVAL: Property not found */
 		if (ret != -EINVAL) {
 			int econtrol;
@@ -1527,27 +1529,26 @@ static void palmas_dt_to_pdata(struct device *dev,
 					WARN_ON(1);
 					dev_warn(dev,
 						 "%s: Invalid roof-floor option: %u\n",
-					     palmas_matches[idx].name, prop);
+						 match->name, prop);
 					break;
 				}
 			}
 			pdata->reg_init[idx]->roof_floor = econtrol;
 		}
 
-		ret = of_property_read_u32(ddata->palmas_matches[idx].of_node,
-					   "ti,mode-sleep", &prop);
+		ret = of_property_read_u32(match->of_node, "ti,mode-sleep",
+					   &prop);
 		if (!ret)
 			pdata->reg_init[idx]->mode_sleep = prop;
 
-		ret = of_property_read_bool(ddata->palmas_matches[idx].of_node,
-					    "ti,smps-range");
+		ret = of_property_read_bool(match->of_node, "ti,smps-range");
 		if (ret)
 			pdata->reg_init[idx]->vsel =
 				PALMAS_SMPS12_VOLTAGE_RANGE;
 
 		if (idx == PALMAS_REG_LDO8)
 			pdata->enable_ldo8_tracking = of_property_read_bool(
-						ddata->palmas_matches[idx].of_node,
+						match->of_node,
 						"ti,enable-ldo8-tracking");
 	}
 
-- 
2.8.0

[PATCH 2/4] regulator: tps65917/palmas: Simplify multiple dereference of pdata->reg_init[idx]

[Nishanth Menon]

Converting dt to platform data logic involves picking up information
that is unique per regulator, however we can improve readability of
the code by allocating and referencing pdata->reg_init[idx] once in
the loop.

While at it, use sizeof(*pointer) when allocating pointer. This allows
for structure name changes with minimal code change.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/regulator/palmas-regulator.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 41b4e94a8d7d..3b9206224cd1 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1492,19 +1492,19 @@ static void palmas_dt_to_pdata(struct device *dev,
 
 	for (idx = 0; idx < ddata->max_reg; idx++) {
 		static struct of_regulator_match *match;
+		struct palmas_reg_init *rinit;
 
 		match = &ddata->palmas_matches[idx];
 
 		if (!match->init_data || !match->of_node)
 			continue;
 
+		rinit = devm_kzalloc(dev, sizeof(*rinit), GFP_KERNEL);
 		pdata->reg_data[idx] = match->init_data;
+		pdata->reg_init[idx] = rinit;
 
-		pdata->reg_init[idx] = devm_kzalloc(dev,
-				sizeof(struct palmas_reg_init), GFP_KERNEL);
-
-		pdata->reg_init[idx]->warm_reset =
-			of_property_read_bool(match->of_node, "ti,warm-reset");
+		rinit->warm_reset = of_property_read_bool(match->of_node,
+							  "ti,warm-reset");
 
 		ret = of_property_read_u32(match->of_node, "ti,roof-floor",
 					   &prop);
@@ -1533,18 +1533,17 @@ static void palmas_dt_to_pdata(struct device *dev,
 					break;
 				}
 			}
-			pdata->reg_init[idx]->roof_floor = econtrol;
+			rinit->roof_floor = econtrol;
 		}
 
 		ret = of_property_read_u32(match->of_node, "ti,mode-sleep",
 					   &prop);
 		if (!ret)
-			pdata->reg_init[idx]->mode_sleep = prop;
+			rinit->mode_sleep = prop;
 
 		ret = of_property_read_bool(match->of_node, "ti,smps-range");
 		if (ret)
-			pdata->reg_init[idx]->vsel =
-				PALMAS_SMPS12_VOLTAGE_RANGE;
+			rinit->vsel = PALMAS_SMPS12_VOLTAGE_RANGE;
 
 		if (idx == PALMAS_REG_LDO8)
 			pdata->enable_ldo8_tracking = of_property_read_bool(
-- 
2.8.0

[PATCH 3/4] regulator: tps65917/palmas: Handle possible memory allocation failure

[Nishanth Menon]

Stop the palmas regulator driver from imagining that the allocations
will always succeed. Since regulator dt nodes are optional in nature and
can be described in downstream drivers via platform data, continue to
maintain code flow as prior when of node is not found.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/regulator/palmas-regulator.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 3b9206224cd1..faa389be7ca3 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1467,10 +1467,10 @@ static struct palmas_pmic_driver_data tps65917_ddata = {
 	.ldo_register = tps65917_ldo_registration,
 };
 
-static void palmas_dt_to_pdata(struct device *dev,
-			       struct device_node *node,
-			       struct palmas_pmic_platform_data *pdata,
-			       struct palmas_pmic_driver_data *ddata)
+static int palmas_dt_to_pdata(struct device *dev,
+			      struct device_node *node,
+			      struct palmas_pmic_platform_data *pdata,
+			      struct palmas_pmic_driver_data *ddata)
 {
 	struct device_node *regulators;
 	u32 prop;
@@ -1479,7 +1479,7 @@ static void palmas_dt_to_pdata(struct device *dev,
 	regulators = of_get_child_by_name(node, "regulators");
 	if (!regulators) {
 		dev_info(dev, "regulator node not found\n");
-		return;
+		return 0;
 	}
 
 	ret = of_regulator_match(dev, regulators, ddata->palmas_matches,
@@ -1487,7 +1487,7 @@ static void palmas_dt_to_pdata(struct device *dev,
 	of_node_put(regulators);
 	if (ret < 0) {
 		dev_err(dev, "Error parsing regulator init data: %d\n", ret);
-		return;
+		return 0;
 	}
 
 	for (idx = 0; idx < ddata->max_reg; idx++) {
@@ -1500,6 +1500,9 @@ static void palmas_dt_to_pdata(struct device *dev,
 			continue;
 
 		rinit = devm_kzalloc(dev, sizeof(*rinit), GFP_KERNEL);
+		if (!rinit)
+			return -ENOMEM;
+
 		pdata->reg_data[idx] = match->init_data;
 		pdata->reg_init[idx] = rinit;
 
@@ -1552,6 +1555,8 @@ static void palmas_dt_to_pdata(struct device *dev,
 	}
 
 	pdata->ldo6_vibrator = of_property_read_bool(node, "ti,ldo6-vibrator");
+
+	return 0;
 }
 
 static const struct of_device_id of_palmas_match_tbl[] = {
@@ -1633,7 +1638,9 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, pmic);
 	pmic->palmas->pmic_ddata = driver_data;
 
-	palmas_dt_to_pdata(&pdev->dev, node, pdata, driver_data);
+	ret = palmas_dt_to_pdata(&pdev->dev, node, pdata, driver_data);
+	if (ret)
+		return ret;
 
 	ret = palmas_smps_read(palmas, PALMAS_SMPS_CTRL, &reg);
 	if (ret)
-- 
2.8.0

[PATCH 4/4] regulator: tps65917/palmas: Simplify multiple dereference of match->of_node

[Nishanth Menon]

Just dereference match->of_node once instead of using match->of_node.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/regulator/palmas-regulator.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index faa389be7ca3..f11d41dad9c1 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1493,10 +1493,12 @@ static int palmas_dt_to_pdata(struct device *dev,
 	for (idx = 0; idx < ddata->max_reg; idx++) {
 		static struct of_regulator_match *match;
 		struct palmas_reg_init *rinit;
+		struct device_node *np;
 
 		match = &ddata->palmas_matches[idx];
+		np = match->of_node;
 
-		if (!match->init_data || !match->of_node)
+		if (!match->init_data || !np)
 			continue;
 
 		rinit = devm_kzalloc(dev, sizeof(*rinit), GFP_KERNEL);
@@ -1506,11 +1508,8 @@ static int palmas_dt_to_pdata(struct device *dev,
 		pdata->reg_data[idx] = match->init_data;
 		pdata->reg_init[idx] = rinit;
 
-		rinit->warm_reset = of_property_read_bool(match->of_node,
-							  "ti,warm-reset");
-
-		ret = of_property_read_u32(match->of_node, "ti,roof-floor",
-					   &prop);
+		rinit->warm_reset = of_property_read_bool(np, "ti,warm-reset");
+		ret = of_property_read_u32(np, "ti,roof-floor", &prop);
 		/* EINVAL: Property not found */
 		if (ret != -EINVAL) {
 			int econtrol;
@@ -1539,19 +1538,17 @@ static int palmas_dt_to_pdata(struct device *dev,
 			rinit->roof_floor = econtrol;
 		}
 
-		ret = of_property_read_u32(match->of_node, "ti,mode-sleep",
-					   &prop);
+		ret = of_property_read_u32(np, "ti,mode-sleep", &prop);
 		if (!ret)
 			rinit->mode_sleep = prop;
 
-		ret = of_property_read_bool(match->of_node, "ti,smps-range");
+		ret = of_property_read_bool(np, "ti,smps-range");
 		if (ret)
 			rinit->vsel = PALMAS_SMPS12_VOLTAGE_RANGE;
 
 		if (idx == PALMAS_REG_LDO8)
 			pdata->enable_ldo8_tracking = of_property_read_bool(
-						match->of_node,
-						"ti,enable-ldo8-tracking");
+						np, "ti,enable-ldo8-tracking");
 	}
 
 	pdata->ldo6_vibrator = of_property_read_bool(node, "ti,ldo6-vibrator");
-- 
2.8.0

Re: [PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Laxman Dewangan]

Hi Nishanth,

On Friday 06 May 2016 05:59 AM, Nishanth Menon wrote:
> While attempting to try and get each of the regulator nodes of palmas
> to be capable of using vin-supply, I noticed a few cleanups and
> associated bug fixes.
>
> The series is based on next-20160504 and is split up to aid
> readability of the patch.
>
> Tested on OMAP5-uevm and DRA7 platform.
>

When you are here, can you implement the dt parsing with the new method 
from regulator framework.
Regulator FW calls callback to parse customized DT property, just need 
to pass the node and pointer when registering.

This will helps lots in cleanups and readability.

Re: [PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Mark Brown]

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

On Fri, May 06, 2016 at 12:44:23PM +0530, Laxman Dewangan wrote:

> When you are here, can you implement the dt parsing with the new method from
> regulator framework.
> Regulator FW calls callback to parse customized DT property, just need to
> pass the node and pointer when registering.

> This will helps lots in cleanups and readability.

Yes, please.

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

Applied "regulator: tps65917/palmas: Simplify multiple dereference of match->of_node" to the regulator tree

[Mark Brown]

The patch

   regulator: tps65917/palmas: Simplify multiple dereference of match->of_node

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 6c7d614fa22d32d038b5a6e88d71acb2711e7035 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Thu, 5 May 2016 19:29:52 -0500
Subject: [PATCH] regulator: tps65917/palmas: Simplify multiple dereference of
 match->of_node

Just dereference match->of_node once instead of using match->of_node.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/palmas-regulator.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index faa389be7ca3..f11d41dad9c1 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1493,10 +1493,12 @@ static int palmas_dt_to_pdata(struct device *dev,
 	for (idx = 0; idx < ddata->max_reg; idx++) {
 		static struct of_regulator_match *match;
 		struct palmas_reg_init *rinit;
+		struct device_node *np;
 
 		match = &ddata->palmas_matches[idx];
+		np = match->of_node;
 
-		if (!match->init_data || !match->of_node)
+		if (!match->init_data || !np)
 			continue;
 
 		rinit = devm_kzalloc(dev, sizeof(*rinit), GFP_KERNEL);
@@ -1506,11 +1508,8 @@ static int palmas_dt_to_pdata(struct device *dev,
 		pdata->reg_data[idx] = match->init_data;
 		pdata->reg_init[idx] = rinit;
 
-		rinit->warm_reset = of_property_read_bool(match->of_node,
-							  "ti,warm-reset");
-
-		ret = of_property_read_u32(match->of_node, "ti,roof-floor",
-					   &prop);
+		rinit->warm_reset = of_property_read_bool(np, "ti,warm-reset");
+		ret = of_property_read_u32(np, "ti,roof-floor", &prop);
 		/* EINVAL: Property not found */
 		if (ret != -EINVAL) {
 			int econtrol;
@@ -1539,19 +1538,17 @@ static int palmas_dt_to_pdata(struct device *dev,
 			rinit->roof_floor = econtrol;
 		}
 
-		ret = of_property_read_u32(match->of_node, "ti,mode-sleep",
-					   &prop);
+		ret = of_property_read_u32(np, "ti,mode-sleep", &prop);
 		if (!ret)
 			rinit->mode_sleep = prop;
 
-		ret = of_property_read_bool(match->of_node, "ti,smps-range");
+		ret = of_property_read_bool(np, "ti,smps-range");
 		if (ret)
 			rinit->vsel = PALMAS_SMPS12_VOLTAGE_RANGE;
 
 		if (idx == PALMAS_REG_LDO8)
 			pdata->enable_ldo8_tracking = of_property_read_bool(
-						match->of_node,
-						"ti,enable-ldo8-tracking");
+						np, "ti,enable-ldo8-tracking");
 	}
 
 	pdata->ldo6_vibrator = of_property_read_bool(node, "ti,ldo6-vibrator");
-- 
2.8.1

Applied "regulator: tps65917/palmas: Handle possible memory allocation failure" to the regulator tree

[Mark Brown]

The patch

   regulator: tps65917/palmas: Handle possible memory allocation failure

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 7f091e53c9efc52b2b3a03a8a1d479a47956fecd Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Thu, 5 May 2016 19:29:51 -0500
Subject: [PATCH] regulator: tps65917/palmas: Handle possible memory allocation
 failure

Stop the palmas regulator driver from imagining that the allocations
will always succeed. Since regulator dt nodes are optional in nature and
can be described in downstream drivers via platform data, continue to
maintain code flow as prior when of node is not found.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/palmas-regulator.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 3b9206224cd1..faa389be7ca3 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1467,10 +1467,10 @@ static struct palmas_pmic_driver_data tps65917_ddata = {
 	.ldo_register = tps65917_ldo_registration,
 };
 
-static void palmas_dt_to_pdata(struct device *dev,
-			       struct device_node *node,
-			       struct palmas_pmic_platform_data *pdata,
-			       struct palmas_pmic_driver_data *ddata)
+static int palmas_dt_to_pdata(struct device *dev,
+			      struct device_node *node,
+			      struct palmas_pmic_platform_data *pdata,
+			      struct palmas_pmic_driver_data *ddata)
 {
 	struct device_node *regulators;
 	u32 prop;
@@ -1479,7 +1479,7 @@ static void palmas_dt_to_pdata(struct device *dev,
 	regulators = of_get_child_by_name(node, "regulators");
 	if (!regulators) {
 		dev_info(dev, "regulator node not found\n");
-		return;
+		return 0;
 	}
 
 	ret = of_regulator_match(dev, regulators, ddata->palmas_matches,
@@ -1487,7 +1487,7 @@ static void palmas_dt_to_pdata(struct device *dev,
 	of_node_put(regulators);
 	if (ret < 0) {
 		dev_err(dev, "Error parsing regulator init data: %d\n", ret);
-		return;
+		return 0;
 	}
 
 	for (idx = 0; idx < ddata->max_reg; idx++) {
@@ -1500,6 +1500,9 @@ static void palmas_dt_to_pdata(struct device *dev,
 			continue;
 
 		rinit = devm_kzalloc(dev, sizeof(*rinit), GFP_KERNEL);
+		if (!rinit)
+			return -ENOMEM;
+
 		pdata->reg_data[idx] = match->init_data;
 		pdata->reg_init[idx] = rinit;
 
@@ -1552,6 +1555,8 @@ static void palmas_dt_to_pdata(struct device *dev,
 	}
 
 	pdata->ldo6_vibrator = of_property_read_bool(node, "ti,ldo6-vibrator");
+
+	return 0;
 }
 
 static const struct of_device_id of_palmas_match_tbl[] = {
@@ -1633,7 +1638,9 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, pmic);
 	pmic->palmas->pmic_ddata = driver_data;
 
-	palmas_dt_to_pdata(&pdev->dev, node, pdata, driver_data);
+	ret = palmas_dt_to_pdata(&pdev->dev, node, pdata, driver_data);
+	if (ret)
+		return ret;
 
 	ret = palmas_smps_read(palmas, PALMAS_SMPS_CTRL, &reg);
 	if (ret)
-- 
2.8.1

Applied "regulator: tps65917/palmas: Simplify multiple dereference of pdata->reg_init[idx]" to the regulator tree

[Mark Brown]

The patch

   regulator: tps65917/palmas: Simplify multiple dereference of pdata->reg_init[idx]

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 1b42443db670dde5e3cb4261f77b29010b163fc6 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Thu, 5 May 2016 19:29:50 -0500
Subject: [PATCH] regulator: tps65917/palmas: Simplify multiple dereference of
 pdata->reg_init[idx]

Converting dt to platform data logic involves picking up information
that is unique per regulator, however we can improve readability of
the code by allocating and referencing pdata->reg_init[idx] once in
the loop.

While at it, use sizeof(*pointer) when allocating pointer. This allows
for structure name changes with minimal code change.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/palmas-regulator.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 41b4e94a8d7d..3b9206224cd1 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1492,19 +1492,19 @@ static void palmas_dt_to_pdata(struct device *dev,
 
 	for (idx = 0; idx < ddata->max_reg; idx++) {
 		static struct of_regulator_match *match;
+		struct palmas_reg_init *rinit;
 
 		match = &ddata->palmas_matches[idx];
 
 		if (!match->init_data || !match->of_node)
 			continue;
 
+		rinit = devm_kzalloc(dev, sizeof(*rinit), GFP_KERNEL);
 		pdata->reg_data[idx] = match->init_data;
+		pdata->reg_init[idx] = rinit;
 
-		pdata->reg_init[idx] = devm_kzalloc(dev,
-				sizeof(struct palmas_reg_init), GFP_KERNEL);
-
-		pdata->reg_init[idx]->warm_reset =
-			of_property_read_bool(match->of_node, "ti,warm-reset");
+		rinit->warm_reset = of_property_read_bool(match->of_node,
+							  "ti,warm-reset");
 
 		ret = of_property_read_u32(match->of_node, "ti,roof-floor",
 					   &prop);
@@ -1533,18 +1533,17 @@ static void palmas_dt_to_pdata(struct device *dev,
 					break;
 				}
 			}
-			pdata->reg_init[idx]->roof_floor = econtrol;
+			rinit->roof_floor = econtrol;
 		}
 
 		ret = of_property_read_u32(match->of_node, "ti,mode-sleep",
 					   &prop);
 		if (!ret)
-			pdata->reg_init[idx]->mode_sleep = prop;
+			rinit->mode_sleep = prop;
 
 		ret = of_property_read_bool(match->of_node, "ti,smps-range");
 		if (ret)
-			pdata->reg_init[idx]->vsel =
-				PALMAS_SMPS12_VOLTAGE_RANGE;
+			rinit->vsel = PALMAS_SMPS12_VOLTAGE_RANGE;
 
 		if (idx == PALMAS_REG_LDO8)
 			pdata->enable_ldo8_tracking = of_property_read_bool(
-- 
2.8.1

Applied "regulator: tps65917/palmas: Simplify multiple dereference of ddata->palmas_matches[idx]" to the regulator tree

[Mark Brown]

The patch

   regulator: tps65917/palmas: Simplify multiple dereference of ddata->palmas_matches[idx]

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 036d193d3337365e0d69cff9bb2593bfc1210e7b Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Thu, 5 May 2016 19:29:49 -0500
Subject: [PATCH] regulator: tps65917/palmas: Simplify multiple dereference of
 ddata->palmas_matches[idx]

Converting dt to platform data logic involves picking up information
that is unique per regulator, however we can improve readability of
the code by dereferencing ddata->palmas_matches[idx] once in the loop.

While at it fix reuse of generic palmas_matches common variable
while reporting error for a specific regulator (which may be from
65917/palmas list).

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/palmas-regulator.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index c83e06b2cedb..41b4e94a8d7d 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -1491,21 +1491,23 @@ static void palmas_dt_to_pdata(struct device *dev,
 	}
 
 	for (idx = 0; idx < ddata->max_reg; idx++) {
-		if (!ddata->palmas_matches[idx].init_data ||
-		    !ddata->palmas_matches[idx].of_node)
+		static struct of_regulator_match *match;
+
+		match = &ddata->palmas_matches[idx];
+
+		if (!match->init_data || !match->of_node)
 			continue;
 
-		pdata->reg_data[idx] = ddata->palmas_matches[idx].init_data;
+		pdata->reg_data[idx] = match->init_data;
 
 		pdata->reg_init[idx] = devm_kzalloc(dev,
 				sizeof(struct palmas_reg_init), GFP_KERNEL);
 
 		pdata->reg_init[idx]->warm_reset =
-			of_property_read_bool(ddata->palmas_matches[idx].of_node,
-					      "ti,warm-reset");
+			of_property_read_bool(match->of_node, "ti,warm-reset");
 
-		ret = of_property_read_u32(ddata->palmas_matches[idx].of_node,
-					   "ti,roof-floor", &prop);
+		ret = of_property_read_u32(match->of_node, "ti,roof-floor",
+					   &prop);
 		/* EINVAL: Property not found */
 		if (ret != -EINVAL) {
 			int econtrol;
@@ -1527,27 +1529,26 @@ static void palmas_dt_to_pdata(struct device *dev,
 					WARN_ON(1);
 					dev_warn(dev,
 						 "%s: Invalid roof-floor option: %u\n",
-					     palmas_matches[idx].name, prop);
+						 match->name, prop);
 					break;
 				}
 			}
 			pdata->reg_init[idx]->roof_floor = econtrol;
 		}
 
-		ret = of_property_read_u32(ddata->palmas_matches[idx].of_node,
-					   "ti,mode-sleep", &prop);
+		ret = of_property_read_u32(match->of_node, "ti,mode-sleep",
+					   &prop);
 		if (!ret)
 			pdata->reg_init[idx]->mode_sleep = prop;
 
-		ret = of_property_read_bool(ddata->palmas_matches[idx].of_node,
-					    "ti,smps-range");
+		ret = of_property_read_bool(match->of_node, "ti,smps-range");
 		if (ret)
 			pdata->reg_init[idx]->vsel =
 				PALMAS_SMPS12_VOLTAGE_RANGE;
 
 		if (idx == PALMAS_REG_LDO8)
 			pdata->enable_ldo8_tracking = of_property_read_bool(
-						ddata->palmas_matches[idx].of_node,
+						match->of_node,
 						"ti,enable-ldo8-tracking");
 	}
 
-- 
2.8.1

Re: [PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Nishanth Menon]

On 05/06/2016 12:14 PM, Mark Brown wrote:
> On Fri, May 06, 2016 at 12:44:23PM +0530, Laxman Dewangan wrote:
> 
>> When you are here, can you implement the dt parsing with the new method from
>> regulator framework.
>> Regulator FW calls callback to parse customized DT property, just need to
>> pass the node and pointer when registering.
> 
>> This will helps lots in cleanups and readability.
> 
> Yes, please.
> 
yeah, the driver has started showing it's age, it will be good to do a
refactor.

-- 
Regards,
Nishanth Menon

Re: [PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Keerthy]

+ Lee Jones

On Saturday 07 May 2016 12:31 AM, Nishanth Menon wrote:
> On 05/06/2016 12:14 PM, Mark Brown wrote:
>> On Fri, May 06, 2016 at 12:44:23PM +0530, Laxman Dewangan wrote:
>>
>>> When you are here, can you implement the dt parsing with the new method from
>>> regulator framework.
>>> Regulator FW calls callback to parse customized DT property, just need to
>>> pass the node and pointer when registering.
>>
>>> This will helps lots in cleanups and readability.
>>
>> Yes, please.
>>
> yeah, the driver has started showing it's age, it will be good to do a
> refactor.

Laxman,

I got the dt parsing with new method from regulator framework part, But
by new method do you also want to remove the dt compatible of regulators 
and let only the mfd compatible stay?

replace of_platform_populate with mfd_add_devices so that linux handles 
the drivers split up and not the device tree?

Regards,
Keerthy
>

Re: [PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Laxman Dewangan]

On Friday 20 May 2016 10:01 AM, Keerthy wrote:
> + Lee Jones
>
> On Saturday 07 May 2016 12:31 AM, Nishanth Menon wrote:
>> On 05/06/2016 12:14 PM, Mark Brown wrote:
>>> On Fri, May 06, 2016 at 12:44:23PM +0530, Laxman Dewangan wrote:
>>>
>>>> When you are here, can you implement the dt parsing with the new 
>>>> method from
>>>> regulator framework.
>>>> Regulator FW calls callback to parse customized DT property, just 
>>>> need to
>>>> pass the node and pointer when registering.
>>>
>>>> This will helps lots in cleanups and readability.
>>>
>>> Yes, please.
>>>
>> yeah, the driver has started showing it's age, it will be good to do a
>> refactor.
>
> Laxman,
>
> I got the dt parsing with new method from regulator framework part, But
> by new method do you also want to remove the dt compatible of 
> regulators and let only the mfd compatible stay?
>
> replace of_platform_populate with mfd_add_devices so that linux 
> handles the drivers split up and not the device tree?
>

The DT binding of child devices of the palmas are like that each sub 
node has compatible.
So I dont think we can change this to avoid regression.

However, if we make the child devices independent of the parent devices 
then it will be very useful to use across different PMIC if they have 
same IP.
Currently, child devices are very much tightly coupled with parent 
devices for the register access and global structure member accces.

This is exactly what we did for the max77686 RTC driver which is used by 
max77686, max77802 and max77620.

There is two mfd core driver, max77686 and max77620 and uses same RTC 
driver rtc-max77686.c

Re: [PATCH 0/4] regulator: tps65917/palmas: Cleanups and bugfixes

[Keerthy]

On Friday 20 May 2016 11:46 AM, Laxman Dewangan wrote:
>
> On Friday 20 May 2016 10:01 AM, Keerthy wrote:
>> + Lee Jones
>>
>> On Saturday 07 May 2016 12:31 AM, Nishanth Menon wrote:
>>> On 05/06/2016 12:14 PM, Mark Brown wrote:
>>>> On Fri, May 06, 2016 at 12:44:23PM +0530, Laxman Dewangan wrote:
>>>>
>>>>> When you are here, can you implement the dt parsing with the new
>>>>> method from
>>>>> regulator framework.
>>>>> Regulator FW calls callback to parse customized DT property, just
>>>>> need to
>>>>> pass the node and pointer when registering.
>>>>
>>>>> This will helps lots in cleanups and readability.
>>>>
>>>> Yes, please.
>>>>
>>> yeah, the driver has started showing it's age, it will be good to do a
>>> refactor.
>>
>> Laxman,
>>
>> I got the dt parsing with new method from regulator framework part, But
>> by new method do you also want to remove the dt compatible of
>> regulators and let only the mfd compatible stay?
>>
>> replace of_platform_populate with mfd_add_devices so that linux
>> handles the drivers split up and not the device tree?
>>
>
> The DT binding of child devices of the palmas are like that each sub
> node has compatible.
> So I dont think we can change this to avoid regression.
>
> However, if we make the child devices independent of the parent devices
> then it will be very useful to use across different PMIC if they have
> same IP.
> Currently, child devices are very much tightly coupled with parent
> devices for the register access and global structure member accces.
>
> This is exactly what we did for the max77686 RTC driver which is used by
> max77686, max77802 and max77620.
>
> There is two mfd core driver, max77686 and max77620 and uses same RTC
> driver rtc-max77686.c

Laxman,

Sorry for responding late on this thread. The new way of the dt parsing 
with the new method expects the driver to populate vsel_reg, vsel_mask,
enable_reg, enable_mask.

The inherent difference in palmas regulator driver w.r.t handling 
regulators is that this driver treats smps and ldo differently. It has 
separate read/write functions for both and goes by separate base 
addresses for spmp and ldo. Now to get all this unified under one 
regulator_desc array a lot of code churn would be needed in both header 
and C files. Not sure if that is okay.

Regards,
Keerthy
>
>