Skip to contentSkip to conversation

regulator: Use of_node_name_eq for node name comparisons

PATCH In review Rob Herring 4 participants 5 messages Dec 5, 2018 – Mar 13, 2019

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 regulator: Use of_node_name_eq for node name comparisons

Convert string compares of DT node names to use of_node_name_eq helper instead. This removes direct access to the node name pointer.

For instances using of_node_cmp, this has the side effect of now using case sensitive comparisons. This should not matter for any FDT based system which all of these are.

Cc Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Support Opensource <[email protected]>
Cc Sangbeom Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/regulator/88pm8607.c               | 2 +-
 drivers/regulator/da9052-regulator.c       | 2 +-
 drivers/regulator/max8997-regulator.c      | 2 +-
 drivers/regulator/mc13xxx-regulator-core.c | 2 +-
 drivers/regulator/qcom-rpmh-regulator.c    | 2 +-
 drivers/regulator/s5m8767.c                | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)
drivers/regulator/88pm8607.c +1 -1
index fd86446e499b..28f55248eb90 100644
@@ -328,7 +328,7 @@ static int pm8607_regulator_dt_init(struct platform_device *pdev,
 		return -ENODEV;
 	}
 	for_each_child_of_node(nproot, np) {
-		if (!of_node_cmp(np->name, info->desc.name)) {
+		if (of_node_name_eq(np, info->desc.name)) {
 			config->init_data =
 				of_get_regulator_init_data(&pdev->dev, np,
 							   &info->desc);
drivers/regulator/da9052-regulator.c +1 -1
index 37e4025203e3..207cb3859dcc 100644
@@ -435,7 +435,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 			return -ENODEV;
 
 		for_each_child_of_node(nproot, np) {
-			if (!of_node_cmp(np->name,
+			if (of_node_name_eq(np,
 					 regulator->info->reg_desc.name)) {
 				config.init_data = of_get_regulator_init_data(
 					&pdev->dev, np,
drivers/regulator/max8997-regulator.c +1 -1
index 3bf5ddfaaea8..4d2487279a0a 100644
@@ -925,7 +925,7 @@ static int max8997_pmic_dt_parse_pdata(struct platform_device *pdev,
 	pdata->regulators = rdata;
 	for_each_child_of_node(regulators_np, reg_np) {
 		for (i = 0; i < ARRAY_SIZE(regulators); i++)
-			if (!of_node_cmp(reg_np->name, regulators[i].name))
+			if (of_node_name_eq(reg_np, regulators[i].name))
 				break;
 
 		if (i == ARRAY_SIZE(regulators)) {
drivers/regulator/mc13xxx-regulator-core.c +1 -1
index 65eb1e0350cf..2243138d8a58 100644
@@ -186,7 +186,7 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
 		for (i = 0; i < num_regulators; i++) {
 			if (!regulators[i].desc.name)
 				continue;
-			if (!of_node_cmp(child->name,
+			if (of_node_name_eq(child,
 					 regulators[i].desc.name)) {
 				p->id = i;
 				p->init_data = of_get_regulator_init_data(
drivers/regulator/qcom-rpmh-regulator.c +1 -1
index 39ccf53fdeb3..b2c2d01d1637 100644
@@ -410,7 +410,7 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
 	vreg->dev = dev;
 
 	for (rpmh_data = pmic_rpmh_data; rpmh_data->name; rpmh_data++)
-		if (!strcmp(rpmh_data->name, node->name))
+		if (of_node_name_eq(node, rpmh_data->name))
 			break;
 
 	if (!rpmh_data->name) {
drivers/regulator/s5m8767.c +1 -1
index 219b9afda0cb..654f65abcb5a 100644
@@ -561,7 +561,7 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 	pdata->opmode = rmode;
 	for_each_child_of_node(regulators_np, reg_np) {
 		for (i = 0; i < ARRAY_SIZE(regulators); i++)
-			if (!of_node_cmp(reg_np->name, regulators[i].name))
+			if (of_node_name_eq(reg_np, regulators[i].name))
 				break;
 
 		if (i == ARRAY_SIZE(regulators)) {
Signature · 1 line
2.19.1
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines · Rob Herring

Convert string compares of DT node names to use of_node_name_eq helper instead. This removes direct access to the node name pointer.

For instances using of_node_cmp, this has the side effect of now using case sensitive comparisons. This should not matter for any FDT based system which all of these are.

Quoted 8 lines
Cc Liam Girdwood <[email protected]>
Cc Mark Brown <[email protected]>
Cc Support Opensource <[email protected]>
Cc Sangbeom Kim <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Bartlomiej Zolnierkiewicz <[email protected]>

For da9052 regulator driver:

Quoted 99 lines
---
 drivers/regulator/88pm8607.c               | 2 +-
 drivers/regulator/da9052-regulator.c       | 2 +-
 drivers/regulator/max8997-regulator.c      | 2 +-
 drivers/regulator/mc13xxx-regulator-core.c | 2 +-
 drivers/regulator/qcom-rpmh-regulator.c    | 2 +-
 drivers/regulator/s5m8767.c                | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)
drivers/regulator/88pm8607.c index renamed from drivers/regulator/88pm8607.c +0 -0

fd86446e499b..28f55248eb90 100644

drivers/regulator/88pm8607.c +1 -1
@@ -328,7 +328,7 @@ static int pm8607_regulator_dt_init(struct
 platform_device *pdev,
 		return -ENODEV;
 	}
 	for_each_child_of_node(nproot, np) {
-		if (!of_node_cmp(np->name, info->desc.name)) {
+		if (of_node_name_eq(np, info->desc.name)) {
 			config->init_data =
 				of_get_regulator_init_data(&pdev->dev, np,
drivers/regulator/88pm8607.c +0 -0
 							   &info->desc);
drivers/regulator/da9052- renamed from drivers/regulator/da9052-regulator.c +0 -0

regulator.c
index 37e4025203e3..207cb3859dcc 100644

drivers/regulator/da9052-regulator.c +1 -1
@@ -435,7 +435,7 @@ static int da9052_regulator_probe(struct platform_device
 *pdev)
 			return -ENODEV;
 
 		for_each_child_of_node(nproot, np) {
-			if (!of_node_cmp(np->name,
+			if (of_node_name_eq(np,
 					 regulator->info->reg_desc.name)) {
 				config.init_data = of_get_regulator_init_data(
drivers/regulator/da9052-regulator.c +0 -0
 					&pdev->dev, np,
drivers/regulator/max8997- renamed from drivers/regulator/max8997-regulator.c +0 -0

regulator.c
index 3bf5ddfaaea8..4d2487279a0a 100644

drivers/regulator/max8997-regulator.c +1 -1
@@ -925,7 +925,7 @@ static int max8997_pmic_dt_parse_pdata(struct
 platform_device *pdev,
 	pdata->regulators = rdata;
 	for_each_child_of_node(regulators_np, reg_np) {
 		for (i = 0; i < ARRAY_SIZE(regulators); i++)
-			if (!of_node_cmp(reg_np->name, regulators[i].name))
+			if (of_node_name_eq(reg_np, regulators[i].name))
 				break;
 
drivers/regulator/max8997-regulator.c +0 -0
 		if (i == ARRAY_SIZE(regulators)) {
drivers/regulator/mc13xxx-regulator-core.c +1 -1
index 65eb1e0350cf..2243138d8a58 100644
@@ -186,7 +186,7 @@ struct mc13xxx_regulator_init_data
 *mc13xxx_parse_regulators_dt(
 		for (i = 0; i < num_regulators; i++) {
 			if (!regulators[i].desc.name)
 				continue;
-			if (!of_node_cmp(child->name,
+			if (of_node_name_eq(child,
 					 regulators[i].desc.name)) {
 				p->id = i;

p->init_data = of_get_regulator_init_data( diff --
git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh- regulator.c
index 39ccf53fdeb3..b2c2d01d1637 100644

drivers/regulator/qcom-rpmh-regulator.c +1 -1
@@ -410,7 +410,7 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg
 *vreg, struct device *dev,
 	vreg->dev = dev;
 
 	for (rpmh_data = pmic_rpmh_data; rpmh_data->name; rpmh_data++)
-		if (!strcmp(rpmh_data->name, node->name))
+		if (of_node_name_eq(node, rpmh_data->name))
 			break;
 
drivers/regulator/qcom-rpmh-regulator.c +0 -0
 	if (!rpmh_data->name) {
drivers/regulator/s5m8767.c index renamed from drivers/regulator/s5m8767.c +0 -0

219b9afda0cb..654f65abcb5a 100644

drivers/regulator/s5m8767.c +1 -1
@@ -561,7 +561,7 @@ static int s5m8767_pmic_dt_parse_pdata(struct
 platform_device *pdev,
 	pdata->opmode = rmode;
 	for_each_child_of_node(regulators_np, reg_np) {
 		for (i = 0; i < ARRAY_SIZE(regulators); i++)
-			if (!of_node_cmp(reg_np->name, regulators[i].name))
+			if (of_node_name_eq(reg_np, regulators[i].name))
 				break;
 

if (i == ARRAY_SIZE(regulators)) {
--
2.19.1

#2 lore raw
In reply to Rob Herring

Hi Rob, hi Mark,

Am Mittwoch, den 05.12.2018, 13:50 -0600 schrieb Rob Herring:

Quoted 6 lines

Convert string compares of DT node names to use of_node_name_eq helper instead. This removes direct access to the node name pointer.

For instances using of_node_cmp, this has the side effect of now using case sensitive comparisons. This should not matter for any FDT based system which all of these are.

[...]

Quoted 11 lines
drivers/regulator/mc13xxx-regulator-core.c +0 -0
index 65eb1e0350cf..2243138d8a58 100644
@@ -186,7 +186,7 @@ struct mc13xxx_regulator_init_data *mc13xxx_parse_regulators_dt(
 >  		for (i = 0; i < num_regulators; i++) {
 >  			if (!regulators[i].desc.name)
 >  				continue;
 > -			if (!of_node_cmp(child->name,
 > +			if (of_node_name_eq(child,
 >  					 regulators[i].desc.name))
Quoted 1 line
Quoted 1 line

Unfortunately this change causes a regression on systems with MC13xxx regulators. The desc.name field is filled with an uppercase name of the regulator, while the existing DTs (as far as I know) all use lowercase node names, so the matching in the function above doesn't work anymore.

Any opinions about how to proceed here?

Regards,
Lucas

#3 lore raw

Hi Lucas,

Quoted 6 lines · Lucas Stach

Unfortunately this change causes a regression on systems with MC13xxx regulators. The desc.name field is filled with an uppercase name of the regulator, while the existing DTs (as far as I know) all use lowercase node names, so the matching in the function above doesn't work anymore.

Any opinions about how to proceed here?

Does this patch fix the problem?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=ec520911ecc7eaf01a

#4 lore raw

Hi Fabio,

Am Mittwoch, den 13.03.2019, 14:36 -0300 schrieb Fabio Estevam:

Quoted 9 lines

Hi Lucas,

Quoted 7 lines · Lucas Stach

Unfortunately this change causes a regression on systems with MC13xxx regulators. The desc.name field is filled with an uppercase name of the regulator, while the existing DTs (as far as I know) all use lowercase node names, so the matching in the function above doesn't work anymore.

Quoted 4 lines
Quoted 1 line

Any opinions about how to proceed here?

Does this patch fix the problem?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=ec520911ecc7eaf01a

I wasn't aware of this patch. I haven't tested it yet, but this looks like it should fix the problem. I just wasn't sure if that's the route we want to go, as I don't know if there are any out of tree DTs that use uppercase node names.

But as there seems to be agreement to just change the regulator names to lowercase, I consider this issue fixed.

Regards,
Lucas