[PATCH v2 0/5] power: supply: core: convert to fwnode

[PATCH v2 0/5] power: supply: core: convert to fwnode

[Sebastian Reichel]

The goal of this series is to replace any OF specific code in the
power-supply core with more generic fwnode code.

The first 2 patches of this series mostly take care of removing .of_node
from power_supply_config in favor of using the existing .fwnode.

Patch 3 replaces the OF specific logic in battery-info. This will
hopefully also allow Hans de Goede reusing the code with his Intel
Dollar Cove TI CC battery driver series.

Patch 4 replaces the OF phandle code with fwnode to have everything
converted.

Finally patch 5 renames some functions to remove the OF terminology
and allows using them without CONFIG_OF being enabled.

Note, that I do not own a single device making use of the
"ocv-capacity-celsius" and "resistance-temp-table", which means patch 3
is basically untested. I would really appreciate if somebody gives this
series a test run on an affected device.

Changes in PATCHv2:
- Link to v1: https://lore.kernel.org/r/20250225-psy-core-convert-to-fwnode-v1-0-d5e4369936bb@collabora.com
- drop merged patches
- add new patch renaming power_supply_get_by_phandle to power_supply_get_by_reference
- rebase to latest power-supply for-next branch
- collected Reviewed-by
- rewrite cover letter accordingly

---
Sebastian Reichel (5):
      regulator: act8865-regulator: switch psy_cfg from of_node to fwnode
      power: supply: core: remove of_node from power_supply_config
      power: supply: core: battery-info: fully switch to fwnode
      power: supply: core: convert to fwnnode
      power: supply: core: rename power_supply_get_by_phandle to power_supply_get_by_reference

 drivers/phy/allwinner/phy-sun4i-usb.c    |   2 +-
 drivers/power/supply/bq2415x_charger.c   |   2 +-
 drivers/power/supply/power_supply_core.c | 193 +++++++++++++++++--------------
 drivers/regulator/act8865-regulator.c    |   2 +-
 include/linux/power_supply.h             |  16 +--
 5 files changed, 109 insertions(+), 106 deletions(-)
---
base-commit: fbc1d056d3f3d417bc9df521cb45a0f51758b64a
change-id: 20250221-psy-core-convert-to-fwnode-d5a5442fc3f9

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>

[PATCH v2 1/5] regulator: act8865-regulator: switch psy_cfg from of_node to fwnode

[Sebastian Reichel]

In order to remove .of_node from the power_supply_config struct,
use .fwnode instead.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/regulator/act8865-regulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c
index 0457af23c55acdd97b2cdc6fd6bfd07ae0f9d11f..b2a6ddc6f56d32e8758977e25858b972e294bc84 100644
--- a/drivers/regulator/act8865-regulator.c
+++ b/drivers/regulator/act8865-regulator.c
@@ -643,7 +643,7 @@ static int act8600_charger_probe(struct device *dev, struct regmap *regmap)
 	struct power_supply *charger;
 	struct power_supply_config cfg = {
 		.drv_data = regmap,
-		.of_node = dev->of_node,
+		.fwnode = dev_fwnode(dev),
 	};
 
 	charger = devm_power_supply_register(dev, &act8600_charger_desc, &cfg);

-- 
2.47.2

Re: [PATCH v2 1/5] regulator: act8865-regulator: switch psy_cfg from of_node to fwnode

[Hans de Goede]

Hi,

On 30-Apr-25 12:54 AM, Sebastian Reichel wrote:
> In order to remove .of_node from the power_supply_config struct,
> use .fwnode instead.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
>  drivers/regulator/act8865-regulator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c
> index 0457af23c55acdd97b2cdc6fd6bfd07ae0f9d11f..b2a6ddc6f56d32e8758977e25858b972e294bc84 100644
> --- a/drivers/regulator/act8865-regulator.c
> +++ b/drivers/regulator/act8865-regulator.c
> @@ -643,7 +643,7 @@ static int act8600_charger_probe(struct device *dev, struct regmap *regmap)
>  	struct power_supply *charger;
>  	struct power_supply_config cfg = {
>  		.drv_data = regmap,
> -		.of_node = dev->of_node,
> +		.fwnode = dev_fwnode(dev),
>  	};
>  
>  	charger = devm_power_supply_register(dev, &act8600_charger_desc, &cfg);
> 

[PATCH v2 2/5] power: supply: core: remove of_node from power_supply_config

[Sebastian Reichel]

All drivers have been migrated from .of_node to .fwnode,
so let's kill the former.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/power_supply_core.c | 3 +--
 include/linux/power_supply.h             | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 33a5bfce4604f012344733ba489eda1c5e8b92c0..89947f1fe610d8a75756e1e4e5339b06349f9ab8 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1529,10 +1529,9 @@ __power_supply_register(struct device *parent,
 	dev_set_drvdata(dev, psy);
 	psy->desc = desc;
 	if (cfg) {
+		device_set_node(dev, cfg->fwnode);
 		dev->groups = cfg->attr_grp;
 		psy->drv_data = cfg->drv_data;
-		dev->of_node =
-			cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node;
 		psy->supplied_to = cfg->supplied_to;
 		psy->num_supplicants = cfg->num_supplicants;
 	}
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index c4cb854971f53a244ba7742a15ce7a5515da6199..b6eb31a23c878aa9ed8ad7bcb02a13721354e1cb 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -229,7 +229,6 @@ struct power_supply;
 
 /* Run-time specific power supply configuration */
 struct power_supply_config {
-	struct device_node *of_node;
 	struct fwnode_handle *fwnode;
 
 	/* Driver private data */

-- 
2.47.2

Re: [PATCH v2 2/5] power: supply: core: remove of_node from power_supply_config

[Hans de Goede]

Hi,

On 30-Apr-25 12:54 AM, Sebastian Reichel wrote:
> All drivers have been migrated from .of_node to .fwnode,
> so let's kill the former.
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans


> ---
>  drivers/power/supply/power_supply_core.c | 3 +--
>  include/linux/power_supply.h             | 1 -
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 33a5bfce4604f012344733ba489eda1c5e8b92c0..89947f1fe610d8a75756e1e4e5339b06349f9ab8 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1529,10 +1529,9 @@ __power_supply_register(struct device *parent,
>  	dev_set_drvdata(dev, psy);
>  	psy->desc = desc;
>  	if (cfg) {
> +		device_set_node(dev, cfg->fwnode);
>  		dev->groups = cfg->attr_grp;
>  		psy->drv_data = cfg->drv_data;
> -		dev->of_node =
> -			cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node;
>  		psy->supplied_to = cfg->supplied_to;
>  		psy->num_supplicants = cfg->num_supplicants;
>  	}
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index c4cb854971f53a244ba7742a15ce7a5515da6199..b6eb31a23c878aa9ed8ad7bcb02a13721354e1cb 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -229,7 +229,6 @@ struct power_supply;
>  
>  /* Run-time specific power supply configuration */
>  struct power_supply_config {
> -	struct device_node *of_node;
>  	struct fwnode_handle *fwnode;
>  
>  	/* Driver private data */
> 

[PATCH v2 3/5] power: supply: core: battery-info: fully switch to fwnode

[Sebastian Reichel]

Also use fwnode based parsing for "ocv-capacity-celsius" and
"resistance-temp-table", so that any DT specific bits are
removed from the power-supply core.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/power_supply_core.c | 109 ++++++++++++++++++-------------
 1 file changed, 63 insertions(+), 46 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 89947f1fe610d8a75756e1e4e5339b06349f9ab8..a8d1fe66e2486a833ccaa3ed77b861c6e52c5760 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -585,32 +585,19 @@ int power_supply_get_battery_info(struct power_supply *psy,
 {
 	struct power_supply_resistance_temp_table *resist_table;
 	struct power_supply_battery_info *info;
-	struct device_node *battery_np = NULL;
-	struct fwnode_reference_args args;
-	struct fwnode_handle *fwnode = NULL;
+	struct fwnode_handle *srcnode, *fwnode;
 	const char *value;
-	int err, len, index;
-	const __be32 *list;
+	int err, len, index, proplen;
+	u32 *propdata;
 	u32 min_max[2];
 
-	if (psy->dev.of_node) {
-		battery_np = of_parse_phandle(psy->dev.of_node, "monitored-battery", 0);
-		if (!battery_np)
-			return -ENODEV;
+	srcnode = dev_fwnode(&psy->dev);
+	if (!srcnode && psy->dev.parent)
+		srcnode = dev_fwnode(psy->dev.parent);
 
-		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
-	} else if (psy->dev.parent) {
-		err = fwnode_property_get_reference_args(
-					dev_fwnode(psy->dev.parent),
-					"monitored-battery", NULL, 0, 0, &args);
-		if (err)
-			return err;
-
-		fwnode = args.fwnode;
-	}
-
-	if (!fwnode)
-		return -ENOENT;
+	fwnode = fwnode_find_reference(srcnode, "monitored-battery", 0);
+	if (IS_ERR(fwnode))
+		return PTR_ERR(fwnode);
 
 	err = fwnode_property_read_string(fwnode, "compatible", &value);
 	if (err)
@@ -740,15 +727,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		info->temp_max = min_max[1];
 	}
 
-	/*
-	 * The below code uses raw of-data parsing to parse
-	 * /schemas/types.yaml#/definitions/uint32-matrix
-	 * data, so for now this is only support with of.
-	 */
-	if (!battery_np)
-		goto out_ret_pointer;
-
-	len = of_property_count_u32_elems(battery_np, "ocv-capacity-celsius");
+	len = fwnode_property_count_u32(fwnode, "ocv-capacity-celsius");
 	if (len < 0 && len != -EINVAL) {
 		err = len;
 		goto out_put_node;
@@ -757,13 +736,13 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		err = -EINVAL;
 		goto out_put_node;
 	} else if (len > 0) {
-		of_property_read_u32_array(battery_np, "ocv-capacity-celsius",
+		fwnode_property_read_u32_array(fwnode, "ocv-capacity-celsius",
 					   info->ocv_temp, len);
 	}
 
 	for (index = 0; index < len; index++) {
 		struct power_supply_battery_ocv_table *table;
-		int i, tab_len, size;
+		int i, tab_len;
 
 		char *propname __free(kfree) = kasprintf(GFP_KERNEL, "ocv-capacity-table-%d",
 							 index);
@@ -772,60 +751,98 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			err = -ENOMEM;
 			goto out_put_node;
 		}
-		list = of_get_property(battery_np, propname, &size);
-		if (!list || !size) {
+		proplen = fwnode_property_count_u32(fwnode, propname);
+		if (proplen < 0 || proplen % 2 != 0) {
 			dev_err(&psy->dev, "failed to get %s\n", propname);
 			power_supply_put_battery_info(psy, info);
 			err = -EINVAL;
 			goto out_put_node;
 		}
+		propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
+		if (!propdata) {
+			kfree(propname);
+			power_supply_put_battery_info(psy, info);
+			err = -EINVAL;
+			goto out_put_node;
+		}
+		err = fwnode_property_read_u32_array(fwnode, propname, propdata, proplen);
+		if (err < 0) {
+			dev_err(&psy->dev, "failed to get %s\n", propname);
+			kfree(propname);
+			kfree(propdata);
+			power_supply_put_battery_info(psy, info);
+			goto out_put_node;
+		}
 
-		tab_len = size / (2 * sizeof(__be32));
+		tab_len = proplen / 2;
 		info->ocv_table_size[index] = tab_len;
 
 		info->ocv_table[index] = table =
 			devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
 		if (!info->ocv_table[index]) {
+			kfree(propdata);
 			power_supply_put_battery_info(psy, info);
 			err = -ENOMEM;
 			goto out_put_node;
 		}
 
 		for (i = 0; i < tab_len; i++) {
-			table[i].ocv = be32_to_cpu(*list);
-			list++;
-			table[i].capacity = be32_to_cpu(*list);
-			list++;
+			table[i].ocv = propdata[i*2];
+			table[i].capacity = propdata[i*2+1];
 		}
+
+		kfree(propdata);
 	}
 
-	list = of_get_property(battery_np, "resistance-temp-table", &len);
-	if (!list || !len)
+	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");
+	if (proplen < 0 || proplen % 2 != 0) {
+		power_supply_put_battery_info(psy, info);
+		err = -ENOMEM;
 		goto out_ret_pointer;
+	} else if (proplen == 0) {
+		goto out_ret_pointer;
+	}
 
-	info->resist_table_size = len / (2 * sizeof(__be32));
+	propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
+	if (!propdata) {
+		power_supply_put_battery_info(psy, info);
+		err = -ENOMEM;
+		goto out_ret_pointer;
+	}
+
+	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
+					     propdata, proplen);
+	if (err < 0) {
+		kfree(propdata);
+		power_supply_put_battery_info(psy, info);
+		goto out_put_node;
+	}
+
+	info->resist_table_size = proplen / 2;
 	info->resist_table = resist_table = devm_kcalloc(&psy->dev,
 							 info->resist_table_size,
 							 sizeof(*resist_table),
 							 GFP_KERNEL);
 	if (!info->resist_table) {
+		kfree(propdata);
 		power_supply_put_battery_info(psy, info);
 		err = -ENOMEM;
 		goto out_put_node;
 	}
 
 	for (index = 0; index < info->resist_table_size; index++) {
-		resist_table[index].temp = be32_to_cpu(*list++);
-		resist_table[index].resistance = be32_to_cpu(*list++);
+		resist_table[index].temp = propdata[index*2];
+		resist_table[index].resistance = propdata[index*2+1];
 	}
 
+	kfree(propdata);
+
 out_ret_pointer:
 	/* Finally return the whole thing */
 	*info_out = info;
 
 out_put_node:
 	fwnode_handle_put(fwnode);
-	of_node_put(battery_np);
 	return err;
 }
 EXPORT_SYMBOL_GPL(power_supply_get_battery_info);

-- 
2.47.2

Re: [PATCH v2 3/5] power: supply: core: battery-info: fully switch to fwnode

[Hans de Goede]

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

Hi Sebastian,
On 30-Apr-25 12:54 AM, Sebastian Reichel wrote:
> Also use fwnode based parsing for "ocv-capacity-celsius" and
> "resistance-temp-table", so that any DT specific bits are
> removed from the power-supply core.
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

I have been testing this converting the ug3105 driver to
use power_supply_batinfo_ocv2cap(), replacing the hardcoded
ocv -> capacity table in that driver.

While testing I hit a bug and while looking closer at this
patch it needs more work on top of fixing that bug.

See comments inline, also I've attached 3 fixup patches
which can be squashed into this patch to address the remarks.

> ---
>  drivers/power/supply/power_supply_core.c | 109 ++++++++++++++++++-------------
>  1 file changed, 63 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 89947f1fe610d8a75756e1e4e5339b06349f9ab8..a8d1fe66e2486a833ccaa3ed77b861c6e52c5760 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -585,32 +585,19 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  {
>  	struct power_supply_resistance_temp_table *resist_table;
>  	struct power_supply_battery_info *info;
> -	struct device_node *battery_np = NULL;
> -	struct fwnode_reference_args args;
> -	struct fwnode_handle *fwnode = NULL;
> +	struct fwnode_handle *srcnode, *fwnode;
>  	const char *value;
> -	int err, len, index;
> -	const __be32 *list;
> +	int err, len, index, proplen;
> +	u32 *propdata;

propname which is also a local-variable for a temporary
malloc-ed buffer uses __free(kfree) instead of explicit
kfree() calls IMHO it would be good to do that here too.

This requires declaring it inside the
"for (index = 0; index < len; index++)" loop like how
this is done for propname, so that it gets freed on
every loop iteration.


>  	u32 min_max[2];
>  
> -	if (psy->dev.of_node) {
> -		battery_np = of_parse_phandle(psy->dev.of_node, "monitored-battery", 0);
> -		if (!battery_np)
> -			return -ENODEV;
> +	srcnode = dev_fwnode(&psy->dev);
> +	if (!srcnode && psy->dev.parent)
> +		srcnode = dev_fwnode(psy->dev.parent);
>  
> -		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
> -	} else if (psy->dev.parent) {
> -		err = fwnode_property_get_reference_args(
> -					dev_fwnode(psy->dev.parent),
> -					"monitored-battery", NULL, 0, 0, &args);
> -		if (err)
> -			return err;
> -
> -		fwnode = args.fwnode;
> -	}
> -
> -	if (!fwnode)
> -		return -ENOENT;
> +	fwnode = fwnode_find_reference(srcnode, "monitored-battery", 0);
> +	if (IS_ERR(fwnode))
> +		return PTR_ERR(fwnode);
>  
>  	err = fwnode_property_read_string(fwnode, "compatible", &value);
>  	if (err)
> @@ -740,15 +727,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  		info->temp_max = min_max[1];
>  	}
>  
> -	/*
> -	 * The below code uses raw of-data parsing to parse
> -	 * /schemas/types.yaml#/definitions/uint32-matrix
> -	 * data, so for now this is only support with of.
> -	 */
> -	if (!battery_np)
> -		goto out_ret_pointer;
> -
> -	len = of_property_count_u32_elems(battery_np, "ocv-capacity-celsius");
> +	len = fwnode_property_count_u32(fwnode, "ocv-capacity-celsius");
>  	if (len < 0 && len != -EINVAL) {
>  		err = len;
>  		goto out_put_node;
> @@ -757,13 +736,13 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  		err = -EINVAL;
>  		goto out_put_node;
>  	} else if (len > 0) {
> -		of_property_read_u32_array(battery_np, "ocv-capacity-celsius",
> +		fwnode_property_read_u32_array(fwnode, "ocv-capacity-celsius",
>  					   info->ocv_temp, len);
>  	}
>  
>  	for (index = 0; index < len; index++) {
>  		struct power_supply_battery_ocv_table *table;
> -		int i, tab_len, size;
> +		int i, tab_len;
>  
>  		char *propname __free(kfree) = kasprintf(GFP_KERNEL, "ocv-capacity-table-%d",
>  							 index);
> @@ -772,60 +751,98 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  			err = -ENOMEM;
>  			goto out_put_node;
>  		}
> -		list = of_get_property(battery_np, propname, &size);
> -		if (!list || !size) {
> +		proplen = fwnode_property_count_u32(fwnode, propname);
> +		if (proplen < 0 || proplen % 2 != 0) {
>  			dev_err(&psy->dev, "failed to get %s\n", propname);
>  			power_supply_put_battery_info(psy, info);
>  			err = -EINVAL;
>  			goto out_put_node;
>  		}
> +		propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);

As mentioned above I suggest to use the following here instead:

		u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
		

> +		if (!propdata) {
> +			kfree(propname);

propname must NOT be free-ed here since it is marked __free(kfree),
freeing it here will cause a double-free bug.

> +			power_supply_put_battery_info(psy, info);
> +			err = -EINVAL;
> +			goto out_put_node;
> +		}
> +		err = fwnode_property_read_u32_array(fwnode, propname, propdata, proplen);
> +		if (err < 0) {
> +			dev_err(&psy->dev, "failed to get %s\n", propname);
> +			kfree(propname);

same as above.

> +			kfree(propdata);

with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.

> +			power_supply_put_battery_info(psy, info);
> +			goto out_put_node;
> +		}
>  
> -		tab_len = size / (2 * sizeof(__be32));
> +		tab_len = proplen / 2;
>  		info->ocv_table_size[index] = tab_len;
>  
>  		info->ocv_table[index] = table =
>  			devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
>  		if (!info->ocv_table[index]) {
> +			kfree(propdata);

with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.

>  			power_supply_put_battery_info(psy, info);
>  			err = -ENOMEM;
>  			goto out_put_node;
>  		}
>  
>  		for (i = 0; i < tab_len; i++) {
> -			table[i].ocv = be32_to_cpu(*list);
> -			list++;
> -			table[i].capacity = be32_to_cpu(*list);
> -			list++;
> +			table[i].ocv = propdata[i*2];
> +			table[i].capacity = propdata[i*2+1];
>  		}
> +
> +		kfree(propdata);

with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.

>  	}
>  
> -	list = of_get_property(battery_np, "resistance-temp-table", &len);
> -	if (!list || !len)
> +	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");

This will return -EINVAL when the property does not exist, making
power_supply_get_battery_info() always fail when there is no
"resistance-temp-table" in the battery fwnode. See the attached fixup
patch for a suggested fix.

> +	if (proplen < 0 || proplen % 2 != 0) {
> +		power_supply_put_battery_info(psy, info);
> +		err = -ENOMEM;

-ENOMEM is the wrong error code here.

>  		goto out_ret_pointer;

This should be "goto out_put_node" since this is an error path.

> +	} else if (proplen == 0) {
> +		goto out_ret_pointer;
> +	}
>  
> -	info->resist_table_size = len / (2 * sizeof(__be32));
> +	propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);

As mentioned above I suggest to use the following here instead:

		u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);

> +	if (!propdata) {
> +		power_supply_put_battery_info(psy, info);
> +		err = -ENOMEM;
> +		goto out_ret_pointer;

This should be "goto out_put_node" since this is an error path.

> +	}
> +
> +	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
> +					     propdata, proplen);
> +	if (err < 0) {
> +		kfree(propdata);

with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.

> +		power_supply_put_battery_info(psy, info);
> +		goto out_put_node;
> +	}
> +
> +	info->resist_table_size = proplen / 2;
>  	info->resist_table = resist_table = devm_kcalloc(&psy->dev,
>  							 info->resist_table_size,
>  							 sizeof(*resist_table),
>  							 GFP_KERNEL);
>  	if (!info->resist_table) {
> +		kfree(propdata);

with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.

>  		power_supply_put_battery_info(psy, info);
>  		err = -ENOMEM;
>  		goto out_put_node;
>  	}
>  
>  	for (index = 0; index < info->resist_table_size; index++) {
> -		resist_table[index].temp = be32_to_cpu(*list++);
> -		resist_table[index].resistance = be32_to_cpu(*list++);
> +		resist_table[index].temp = propdata[index*2];
> +		resist_table[index].resistance = propdata[index*2+1];
>  	}
>  
> +	kfree(propdata);

with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.

> +
>  out_ret_pointer:
>  	/* Finally return the whole thing */
>  	*info_out = info;
>  
>  out_put_node:
>  	fwnode_handle_put(fwnode);
> -	of_node_put(battery_np);
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(power_supply_get_battery_info);
> 

Regards,

Hans



[-- Attachment #2: 0004-fixup-power-supply-core-battery-info-fully-switch-to.patch --]
[-- Type: text/x-patch, Size: 2103 bytes --]

From 68d2414d201083904dad83af4ebb2c35fc850b79 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hansg@kernel.org>
Date: Sun, 8 Jun 2025 21:18:11 +0200
Subject: [PATCH 04/10] fixup! power: supply: core: battery-info: fully switch
 to fwnode

After ("power: supply: core: battery-info: fully switch to fwnode")
power_supply_get_battery_info() will always fail for battery fwnodes
which do not define a "resistance-temp-table".

Fix this by cleanly exiting on both 0 and EINVAL returns from
fwnode_property_count_u32(fwnode, "resistance-temp-table") which
indicates that the property is empty or not there.

While at it also fix:

1. The weird -ENOMEM return for other errors. For other errors
propagate the existing error or -EINVAL for an odd proplen.
2. Wrongly using "goto out_ret_pointer" on errors, out_ret_pointer
should only be used on success, error paths should use
out_put_node;

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 drivers/power/supply/power_supply_core.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index a8d1fe66e248..9bbc3be2e483 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -795,19 +795,20 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	}
 
 	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");
-	if (proplen < 0 || proplen % 2 != 0) {
+	if (proplen == 0 || proplen == -EINVAL) {
+		err = 0;
+		goto out_ret_pointer;
+	} else if (proplen < 0 || proplen % 2 != 0) {
 		power_supply_put_battery_info(psy, info);
-		err = -ENOMEM;
-		goto out_ret_pointer;
-	} else if (proplen == 0) {
-		goto out_ret_pointer;
+		err = (proplen < 0) ? proplen : -EINVAL;
+		goto out_put_node;
 	}
 
 	propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
 	if (!propdata) {
 		power_supply_put_battery_info(psy, info);
 		err = -ENOMEM;
-		goto out_ret_pointer;
+		goto out_put_node;
 	}
 
 	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
-- 
2.49.0


[-- Attachment #3: 0005-fixup-power-supply-core-battery-info-fully-switch-to.patch --]
[-- Type: text/x-patch, Size: 3573 bytes --]

From b92ac226fa6888d7a53fc76815110e76e13b85ae Mon Sep 17 00:00:00 2001
From: Hans de Goede <hansg@kernel.org>
Date: Sun, 8 Jun 2025 21:37:59 +0200
Subject: [PATCH 05/10] fixup! power: supply: core: battery-info: fully switch
 to fwnode

propname must NOT be manually free-ed here since it is marked
__free(kfree), freeing it will cause a double-free bug.

Also move propdata to be a __free(kfree) value for consistency
and also for cleaner code.

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 drivers/power/supply/power_supply_core.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 9bbc3be2e483..0e42b9bafcab 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -588,7 +588,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	struct fwnode_handle *srcnode, *fwnode;
 	const char *value;
 	int err, len, index, proplen;
-	u32 *propdata;
 	u32 min_max[2];
 
 	srcnode = dev_fwnode(&psy->dev);
@@ -758,9 +757,9 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			err = -EINVAL;
 			goto out_put_node;
 		}
-		propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
+
+		u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
 		if (!propdata) {
-			kfree(propname);
 			power_supply_put_battery_info(psy, info);
 			err = -EINVAL;
 			goto out_put_node;
@@ -768,8 +767,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		err = fwnode_property_read_u32_array(fwnode, propname, propdata, proplen);
 		if (err < 0) {
 			dev_err(&psy->dev, "failed to get %s\n", propname);
-			kfree(propname);
-			kfree(propdata);
 			power_supply_put_battery_info(psy, info);
 			goto out_put_node;
 		}
@@ -780,7 +777,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		info->ocv_table[index] = table =
 			devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
 		if (!info->ocv_table[index]) {
-			kfree(propdata);
 			power_supply_put_battery_info(psy, info);
 			err = -ENOMEM;
 			goto out_put_node;
@@ -790,8 +786,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			table[i].ocv = propdata[i*2];
 			table[i].capacity = propdata[i*2+1];
 		}
-
-		kfree(propdata);
 	}
 
 	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");
@@ -804,7 +798,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		goto out_put_node;
 	}
 
-	propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
+	u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
 	if (!propdata) {
 		power_supply_put_battery_info(psy, info);
 		err = -ENOMEM;
@@ -814,7 +808,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
 					     propdata, proplen);
 	if (err < 0) {
-		kfree(propdata);
 		power_supply_put_battery_info(psy, info);
 		goto out_put_node;
 	}
@@ -825,7 +818,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 							 sizeof(*resist_table),
 							 GFP_KERNEL);
 	if (!info->resist_table) {
-		kfree(propdata);
 		power_supply_put_battery_info(psy, info);
 		err = -ENOMEM;
 		goto out_put_node;
@@ -836,8 +828,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		resist_table[index].resistance = propdata[index*2+1];
 	}
 
-	kfree(propdata);
-
 out_ret_pointer:
 	/* Finally return the whole thing */
 	*info_out = info;
-- 
2.49.0

Re: [PATCH v2 3/5] power: supply: core: battery-info: fully switch to fwnode

[Hans de Goede]

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

Hi,

On 8-Jun-25 9:57 PM, Hans de Goede wrote:
> Hi Sebastian,
> On 30-Apr-25 12:54 AM, Sebastian Reichel wrote:
>> Also use fwnode based parsing for "ocv-capacity-celsius" and
>> "resistance-temp-table", so that any DT specific bits are
>> removed from the power-supply core.
>>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> I have been testing this converting the ug3105 driver to
> use power_supply_batinfo_ocv2cap(), replacing the hardcoded
> ocv -> capacity table in that driver.
> 
> While testing I hit a bug and while looking closer at this
> patch it needs more work on top of fixing that bug.
> 
> See comments inline, also I've attached 3 fixup patches
> which can be squashed into this patch to address the remarks.
> 
>> ---
>>  drivers/power/supply/power_supply_core.c | 109 ++++++++++++++++++-------------
>>  1 file changed, 63 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
>> index 89947f1fe610d8a75756e1e4e5339b06349f9ab8..a8d1fe66e2486a833ccaa3ed77b861c6e52c5760 100644
>> --- a/drivers/power/supply/power_supply_core.c
>> +++ b/drivers/power/supply/power_supply_core.c
>> @@ -585,32 +585,19 @@ int power_supply_get_battery_info(struct power_supply *psy,
>>  {
>>  	struct power_supply_resistance_temp_table *resist_table;
>>  	struct power_supply_battery_info *info;
>> -	struct device_node *battery_np = NULL;
>> -	struct fwnode_reference_args args;
>> -	struct fwnode_handle *fwnode = NULL;
>> +	struct fwnode_handle *srcnode, *fwnode;
>>  	const char *value;
>> -	int err, len, index;
>> -	const __be32 *list;
>> +	int err, len, index, proplen;
>> +	u32 *propdata;
> 
> propname which is also a local-variable for a temporary
> malloc-ed buffer uses __free(kfree) instead of explicit
> kfree() calls IMHO it would be good to do that here too.
> 
> This requires declaring it inside the
> "for (index = 0; index < len; index++)" loop like how
> this is done for propname, so that it gets freed on
> every loop iteration.
> 
> 
>>  	u32 min_max[2];
>>  
>> -	if (psy->dev.of_node) {
>> -		battery_np = of_parse_phandle(psy->dev.of_node, "monitored-battery", 0);
>> -		if (!battery_np)
>> -			return -ENODEV;
>> +	srcnode = dev_fwnode(&psy->dev);
>> +	if (!srcnode && psy->dev.parent)
>> +		srcnode = dev_fwnode(psy->dev.parent);
>>  
>> -		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
>> -	} else if (psy->dev.parent) {
>> -		err = fwnode_property_get_reference_args(
>> -					dev_fwnode(psy->dev.parent),
>> -					"monitored-battery", NULL, 0, 0, &args);
>> -		if (err)
>> -			return err;
>> -
>> -		fwnode = args.fwnode;
>> -	}
>> -
>> -	if (!fwnode)
>> -		return -ENOENT;
>> +	fwnode = fwnode_find_reference(srcnode, "monitored-battery", 0);
>> +	if (IS_ERR(fwnode))
>> +		return PTR_ERR(fwnode);
>>  
>>  	err = fwnode_property_read_string(fwnode, "compatible", &value);
>>  	if (err)
>> @@ -740,15 +727,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
>>  		info->temp_max = min_max[1];
>>  	}
>>  
>> -	/*
>> -	 * The below code uses raw of-data parsing to parse
>> -	 * /schemas/types.yaml#/definitions/uint32-matrix
>> -	 * data, so for now this is only support with of.
>> -	 */
>> -	if (!battery_np)
>> -		goto out_ret_pointer;
>> -
>> -	len = of_property_count_u32_elems(battery_np, "ocv-capacity-celsius");
>> +	len = fwnode_property_count_u32(fwnode, "ocv-capacity-celsius");
>>  	if (len < 0 && len != -EINVAL) {
>>  		err = len;
>>  		goto out_put_node;
>> @@ -757,13 +736,13 @@ int power_supply_get_battery_info(struct power_supply *psy,
>>  		err = -EINVAL;
>>  		goto out_put_node;
>>  	} else if (len > 0) {
>> -		of_property_read_u32_array(battery_np, "ocv-capacity-celsius",
>> +		fwnode_property_read_u32_array(fwnode, "ocv-capacity-celsius",
>>  					   info->ocv_temp, len);
>>  	}
>>  
>>  	for (index = 0; index < len; index++) {
>>  		struct power_supply_battery_ocv_table *table;
>> -		int i, tab_len, size;
>> +		int i, tab_len;
>>  
>>  		char *propname __free(kfree) = kasprintf(GFP_KERNEL, "ocv-capacity-table-%d",
>>  							 index);
>> @@ -772,60 +751,98 @@ int power_supply_get_battery_info(struct power_supply *psy,
>>  			err = -ENOMEM;
>>  			goto out_put_node;
>>  		}
>> -		list = of_get_property(battery_np, propname, &size);
>> -		if (!list || !size) {
>> +		proplen = fwnode_property_count_u32(fwnode, propname);
>> +		if (proplen < 0 || proplen % 2 != 0) {
>>  			dev_err(&psy->dev, "failed to get %s\n", propname);
>>  			power_supply_put_battery_info(psy, info);
>>  			err = -EINVAL;
>>  			goto out_put_node;
>>  		}
>> +		propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
> 
> As mentioned above I suggest to use the following here instead:
> 
> 		u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
> 		
> 
>> +		if (!propdata) {
>> +			kfree(propname);
> 
> propname must NOT be free-ed here since it is marked __free(kfree),
> freeing it here will cause a double-free bug.
> 
>> +			power_supply_put_battery_info(psy, info);
>> +			err = -EINVAL;
>> +			goto out_put_node;
>> +		}
>> +		err = fwnode_property_read_u32_array(fwnode, propname, propdata, proplen);
>> +		if (err < 0) {
>> +			dev_err(&psy->dev, "failed to get %s\n", propname);
>> +			kfree(propname);
> 
> same as above.
> 
>> +			kfree(propdata);
> 
> with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.
> 
>> +			power_supply_put_battery_info(psy, info);
>> +			goto out_put_node;
>> +		}
>>  
>> -		tab_len = size / (2 * sizeof(__be32));
>> +		tab_len = proplen / 2;
>>  		info->ocv_table_size[index] = tab_len;
>>  
>>  		info->ocv_table[index] = table =
>>  			devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
>>  		if (!info->ocv_table[index]) {
>> +			kfree(propdata);
> 
> with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.
> 
>>  			power_supply_put_battery_info(psy, info);
>>  			err = -ENOMEM;
>>  			goto out_put_node;
>>  		}
>>  
>>  		for (i = 0; i < tab_len; i++) {
>> -			table[i].ocv = be32_to_cpu(*list);
>> -			list++;
>> -			table[i].capacity = be32_to_cpu(*list);
>> -			list++;
>> +			table[i].ocv = propdata[i*2];
>> +			table[i].capacity = propdata[i*2+1];
>>  		}
>> +
>> +		kfree(propdata);
> 
> with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.
> 
>>  	}
>>  
>> -	list = of_get_property(battery_np, "resistance-temp-table", &len);
>> -	if (!list || !len)
>> +	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");
> 
> This will return -EINVAL when the property does not exist, making
> power_supply_get_battery_info() always fail when there is no
> "resistance-temp-table" in the battery fwnode. See the attached fixup
> patch for a suggested fix.
> 
>> +	if (proplen < 0 || proplen % 2 != 0) {
>> +		power_supply_put_battery_info(psy, info);
>> +		err = -ENOMEM;
> 
> -ENOMEM is the wrong error code here.
> 
>>  		goto out_ret_pointer;
> 
> This should be "goto out_put_node" since this is an error path.
> 
>> +	} else if (proplen == 0) {
>> +		goto out_ret_pointer;
>> +	}
>>  
>> -	info->resist_table_size = len / (2 * sizeof(__be32));
>> +	propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
> 
> As mentioned above I suggest to use the following here instead:
> 
> 		u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);

Ok, so this does not work, because we jump over this causing
propdata to not get initialized, but the __free(kfree) still
triggers.

Lesson learned: do not use modern C style mixing of declarations
and statements together with __free().

Attached are updated fixup patches which do not make things crash.

Regards,

Hans




> 
>> +	if (!propdata) {
>> +		power_supply_put_battery_info(psy, info);
>> +		err = -ENOMEM;
>> +		goto out_ret_pointer;
> 
> This should be "goto out_put_node" since this is an error path.
> 
>> +	}
>> +
>> +	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
>> +					     propdata, proplen);
>> +	if (err < 0) {
>> +		kfree(propdata);
> 
> with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.
> 
>> +		power_supply_put_battery_info(psy, info);
>> +		goto out_put_node;
>> +	}
>> +
>> +	info->resist_table_size = proplen / 2;
>>  	info->resist_table = resist_table = devm_kcalloc(&psy->dev,
>>  							 info->resist_table_size,
>>  							 sizeof(*resist_table),
>>  							 GFP_KERNEL);
>>  	if (!info->resist_table) {
>> +		kfree(propdata);
> 
> with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.
> 
>>  		power_supply_put_battery_info(psy, info);
>>  		err = -ENOMEM;
>>  		goto out_put_node;
>>  	}
>>  
>>  	for (index = 0; index < info->resist_table_size; index++) {
>> -		resist_table[index].temp = be32_to_cpu(*list++);
>> -		resist_table[index].resistance = be32_to_cpu(*list++);
>> +		resist_table[index].temp = propdata[index*2];
>> +		resist_table[index].resistance = propdata[index*2+1];
>>  	}
>>  
>> +	kfree(propdata);
> 
> with the suggested "u32 *propdata __free(kfree)" this can and must be dropped.
> 
>> +
>>  out_ret_pointer:
>>  	/* Finally return the whole thing */
>>  	*info_out = info;
>>  
>>  out_put_node:
>>  	fwnode_handle_put(fwnode);
>> -	of_node_put(battery_np);
>>  	return err;
>>  }
>>  EXPORT_SYMBOL_GPL(power_supply_get_battery_info);
>>
> 
> Regards,
> 
> Hans
> 
> 

[-- Attachment #2: 0004-fixup-power-supply-core-battery-info-fully-switch-to.patch --]
[-- Type: text/x-patch, Size: 2103 bytes --]

From 68d2414d201083904dad83af4ebb2c35fc850b79 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hansg@kernel.org>
Date: Sun, 8 Jun 2025 21:18:11 +0200
Subject: [PATCH 04/10] fixup! power: supply: core: battery-info: fully switch
 to fwnode

After ("power: supply: core: battery-info: fully switch to fwnode")
power_supply_get_battery_info() will always fail for battery fwnodes
which do not define a "resistance-temp-table".

Fix this by cleanly exiting on both 0 and EINVAL returns from
fwnode_property_count_u32(fwnode, "resistance-temp-table") which
indicates that the property is empty or not there.

While at it also fix:

1. The weird -ENOMEM return for other errors. For other errors
propagate the existing error or -EINVAL for an odd proplen.
2. Wrongly using "goto out_ret_pointer" on errors, out_ret_pointer
should only be used on success, error paths should use
out_put_node;

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 drivers/power/supply/power_supply_core.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index a8d1fe66e248..9bbc3be2e483 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -795,19 +795,20 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	}
 
 	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");
-	if (proplen < 0 || proplen % 2 != 0) {
+	if (proplen == 0 || proplen == -EINVAL) {
+		err = 0;
+		goto out_ret_pointer;
+	} else if (proplen < 0 || proplen % 2 != 0) {
 		power_supply_put_battery_info(psy, info);
-		err = -ENOMEM;
-		goto out_ret_pointer;
-	} else if (proplen == 0) {
-		goto out_ret_pointer;
+		err = (proplen < 0) ? proplen : -EINVAL;
+		goto out_put_node;
 	}
 
 	propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
 	if (!propdata) {
 		power_supply_put_battery_info(psy, info);
 		err = -ENOMEM;
-		goto out_ret_pointer;
+		goto out_put_node;
 	}
 
 	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
-- 
2.49.0


[-- Attachment #3: 0005-fixup-power-supply-core-battery-info-fully-switch-to.patch --]
[-- Type: text/x-patch, Size: 3277 bytes --]

From 1e2d54b1c946075590d011da17d96796ede58894 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hansg@kernel.org>
Date: Sun, 8 Jun 2025 21:37:59 +0200
Subject: [PATCH 05/10] fixup! power: supply: core: battery-info: fully switch
 to fwnode

propname must NOT be manually free-ed here since it is marked
__free(kfree), freeing it will cause a double-free bug.

Also move propdata to be a __free(kfree) value for consistency
and also for cleaner code.

Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 drivers/power/supply/power_supply_core.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 9bbc3be2e483..f2c79f15838d 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -588,7 +588,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	struct fwnode_handle *srcnode, *fwnode;
 	const char *value;
 	int err, len, index, proplen;
-	u32 *propdata;
+	u32 *propdata __free(kfree) = NULL;
 	u32 min_max[2];
 
 	srcnode = dev_fwnode(&psy->dev);
@@ -758,9 +758,9 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			err = -EINVAL;
 			goto out_put_node;
 		}
-		propdata = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
+
+		u32 *propdata __free(kfree) = kcalloc(proplen, sizeof(*propdata), GFP_KERNEL);
 		if (!propdata) {
-			kfree(propname);
 			power_supply_put_battery_info(psy, info);
 			err = -EINVAL;
 			goto out_put_node;
@@ -768,8 +768,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		err = fwnode_property_read_u32_array(fwnode, propname, propdata, proplen);
 		if (err < 0) {
 			dev_err(&psy->dev, "failed to get %s\n", propname);
-			kfree(propname);
-			kfree(propdata);
 			power_supply_put_battery_info(psy, info);
 			goto out_put_node;
 		}
@@ -780,7 +778,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		info->ocv_table[index] = table =
 			devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
 		if (!info->ocv_table[index]) {
-			kfree(propdata);
 			power_supply_put_battery_info(psy, info);
 			err = -ENOMEM;
 			goto out_put_node;
@@ -790,8 +787,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			table[i].ocv = propdata[i*2];
 			table[i].capacity = propdata[i*2+1];
 		}
-
-		kfree(propdata);
 	}
 
 	proplen = fwnode_property_count_u32(fwnode, "resistance-temp-table");
@@ -814,7 +809,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	err = fwnode_property_read_u32_array(fwnode, "resistance-temp-table",
 					     propdata, proplen);
 	if (err < 0) {
-		kfree(propdata);
 		power_supply_put_battery_info(psy, info);
 		goto out_put_node;
 	}
@@ -825,7 +819,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 							 sizeof(*resist_table),
 							 GFP_KERNEL);
 	if (!info->resist_table) {
-		kfree(propdata);
 		power_supply_put_battery_info(psy, info);
 		err = -ENOMEM;
 		goto out_put_node;
@@ -836,8 +829,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		resist_table[index].resistance = propdata[index*2+1];
 	}
 
-	kfree(propdata);
-
 out_ret_pointer:
 	/* Finally return the whole thing */
 	*info_out = info;
-- 
2.49.0

[PATCH v2 4/5] power: supply: core: convert to fwnnode

[Sebastian Reichel]

Replace any DT specific code with fwnode in the power-supply
core.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/bq2415x_charger.c   |  2 +-
 drivers/power/supply/power_supply_core.c | 65 ++++++++++++++++----------------
 include/linux/power_supply.h             |  2 +-
 3 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/power/supply/bq2415x_charger.c b/drivers/power/supply/bq2415x_charger.c
index 9e3b9181ee76a4f473228bba022917677acce256..1ecbca510bba99ee7abcda33a719035adfceeb5f 100644
--- a/drivers/power/supply/bq2415x_charger.c
+++ b/drivers/power/supply/bq2415x_charger.c
@@ -1674,7 +1674,7 @@ static int bq2415x_probe(struct i2c_client *client)
 	/* Query for initial reported_mode and set it */
 	if (bq->nb.notifier_call) {
 		if (np) {
-			notify_psy = power_supply_get_by_phandle(np,
+			notify_psy = power_supply_get_by_phandle(of_fwnode_handle(np),
 						"ti,usb-charger-detection");
 			if (IS_ERR(notify_psy))
 				notify_psy = NULL;
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index a8d1fe66e2486a833ccaa3ed77b861c6e52c5760..1d53ceaa8fd161e7e72b90befabb9380393c99f2 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -18,7 +18,6 @@
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/err.h>
-#include <linux/of.h>
 #include <linux/power_supply.h>
 #include <linux/property.h>
 #include <linux/thermal.h>
@@ -196,24 +195,24 @@ static int __power_supply_populate_supplied_from(struct power_supply *epsy,
 						 void *data)
 {
 	struct power_supply *psy = data;
-	struct device_node *np;
+	struct fwnode_handle *np;
 	int i = 0;
 
 	do {
-		np = of_parse_phandle(psy->dev.of_node, "power-supplies", i++);
-		if (!np)
+		np = fwnode_find_reference(psy->dev.fwnode, "power-supplies", i++);
+		if (IS_ERR(np))
 			break;
 
-		if (np == epsy->dev.of_node) {
+		if (np == epsy->dev.fwnode) {
 			dev_dbg(&psy->dev, "%s: Found supply : %s\n",
 				psy->desc->name, epsy->desc->name);
 			psy->supplied_from[i-1] = (char *)epsy->desc->name;
 			psy->num_supplies++;
-			of_node_put(np);
+			fwnode_handle_put(np);
 			break;
 		}
-		of_node_put(np);
-	} while (np);
+		fwnode_handle_put(np);
+	} while (!IS_ERR(np));
 
 	return 0;
 }
@@ -232,16 +231,16 @@ static int power_supply_populate_supplied_from(struct power_supply *psy)
 static int  __power_supply_find_supply_from_node(struct power_supply *epsy,
 						 void *data)
 {
-	struct device_node *np = data;
+	struct fwnode_handle *fwnode = data;
 
 	/* returning non-zero breaks out of power_supply_for_each_psy loop */
-	if (epsy->dev.of_node == np)
+	if (epsy->dev.fwnode == fwnode)
 		return 1;
 
 	return 0;
 }
 
-static int power_supply_find_supply_from_node(struct device_node *supply_node)
+static int power_supply_find_supply_from_fwnode(struct fwnode_handle *supply_node)
 {
 	int error;
 
@@ -249,7 +248,7 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
 	 * power_supply_for_each_psy() either returns its own errors or values
 	 * returned by __power_supply_find_supply_from_node().
 	 *
-	 * __power_supply_find_supply_from_node() will return 0 (no match)
+	 * __power_supply_find_supply_from_fwnode() will return 0 (no match)
 	 * or 1 (match).
 	 *
 	 * We return 0 if power_supply_for_each_psy() returned 1, -EPROBE_DEFER if
@@ -262,7 +261,7 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
 
 static int power_supply_check_supplies(struct power_supply *psy)
 {
-	struct device_node *np;
+	struct fwnode_handle *np;
 	int cnt = 0;
 
 	/* If there is already a list honor it */
@@ -270,24 +269,24 @@ static int power_supply_check_supplies(struct power_supply *psy)
 		return 0;
 
 	/* No device node found, nothing to do */
-	if (!psy->dev.of_node)
+	if (!psy->dev.fwnode)
 		return 0;
 
 	do {
 		int ret;
 
-		np = of_parse_phandle(psy->dev.of_node, "power-supplies", cnt++);
-		if (!np)
+		np = fwnode_find_reference(psy->dev.fwnode, "power-supplies", cnt++);
+		if (IS_ERR(np))
 			break;
 
-		ret = power_supply_find_supply_from_node(np);
-		of_node_put(np);
+		ret = power_supply_find_supply_from_fwnode(np);
+		fwnode_handle_put(np);
 
 		if (ret) {
 			dev_dbg(&psy->dev, "Failed to find supply!\n");
 			return ret;
 		}
-	} while (np);
+	} while (!IS_ERR(np));
 
 	/* Missing valid "power-supplies" entries */
 	if (cnt == 1)
@@ -498,14 +497,14 @@ void power_supply_put(struct power_supply *psy)
 EXPORT_SYMBOL_GPL(power_supply_put);
 
 #ifdef CONFIG_OF
-static int power_supply_match_device_node(struct device *dev, const void *data)
+static int power_supply_match_device_fwnode(struct device *dev, const void *data)
 {
-	return dev->parent && dev->parent->of_node == data;
+	return dev->parent && dev_fwnode(dev->parent) == data;
 }
 
 /**
  * power_supply_get_by_phandle() - Search for a power supply and returns its ref
- * @np: Pointer to device node holding phandle property
+ * @fwnode: Pointer to fwnode holding phandle property
  * @property: Name of property holding a power supply name
  *
  * If power supply was found, it increases reference count for the
@@ -515,21 +514,21 @@ static int power_supply_match_device_node(struct device *dev, const void *data)
  * Return: On success returns a reference to a power supply with
  * matching name equals to value under @property, NULL or ERR_PTR otherwise.
  */
-struct power_supply *power_supply_get_by_phandle(struct device_node *np,
-							const char *property)
+struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
+						 const char *property)
 {
-	struct device_node *power_supply_np;
+	struct fwnode_handle *power_supply_fwnode;
 	struct power_supply *psy = NULL;
 	struct device *dev;
 
-	power_supply_np = of_parse_phandle(np, property, 0);
-	if (!power_supply_np)
-		return ERR_PTR(-ENODEV);
+	power_supply_fwnode = fwnode_find_reference(fwnode, property, 0);
+	if (IS_ERR(power_supply_fwnode))
+		return ERR_CAST(power_supply_fwnode);
 
-	dev = class_find_device(&power_supply_class, NULL, power_supply_np,
-				power_supply_match_device_node);
+	dev = class_find_device(&power_supply_class, NULL, power_supply_fwnode,
+				power_supply_match_device_fwnode);
 
-	of_node_put(power_supply_np);
+	fwnode_handle_put(power_supply_fwnode);
 
 	if (dev) {
 		psy = dev_to_psy(dev);
@@ -561,14 +560,14 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
 {
 	struct power_supply **ptr, *psy;
 
-	if (!dev->of_node)
+	if (!dev_fwnode(dev))
 		return ERR_PTR(-ENODEV);
 
 	ptr = devres_alloc(devm_power_supply_put, sizeof(*ptr), GFP_KERNEL);
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	psy = power_supply_get_by_phandle(dev->of_node, property);
+	psy = power_supply_get_by_phandle(dev_fwnode(dev), property);
 	if (IS_ERR_OR_NULL(psy)) {
 		devres_free(ptr);
 	} else {
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index b6eb31a23c878aa9ed8ad7bcb02a13721354e1cb..c95f098374cbdeafe8cddb52da3903f4f0e0f0fc 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -805,7 +805,7 @@ static inline struct power_supply *power_supply_get_by_name(const char *name)
 { return NULL; }
 #endif
 #ifdef CONFIG_OF
-extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
+extern struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
 							const char *property);
 extern struct power_supply *devm_power_supply_get_by_phandle(
 				    struct device *dev, const char *property);

-- 
2.47.2

Re: [PATCH v2 4/5] power: supply: core: convert to fwnnode

[Hans de Goede]

Hi, 

On 30-Apr-25 12:54 AM, Sebastian Reichel wrote:
> Replace any DT specific code with fwnode in the power-supply
> core.
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
>  drivers/power/supply/bq2415x_charger.c   |  2 +-
>  drivers/power/supply/power_supply_core.c | 65 ++++++++++++++++----------------
>  include/linux/power_supply.h             |  2 +-
>  3 files changed, 34 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/power/supply/bq2415x_charger.c b/drivers/power/supply/bq2415x_charger.c
> index 9e3b9181ee76a4f473228bba022917677acce256..1ecbca510bba99ee7abcda33a719035adfceeb5f 100644
> --- a/drivers/power/supply/bq2415x_charger.c
> +++ b/drivers/power/supply/bq2415x_charger.c
> @@ -1674,7 +1674,7 @@ static int bq2415x_probe(struct i2c_client *client)
>  	/* Query for initial reported_mode and set it */
>  	if (bq->nb.notifier_call) {
>  		if (np) {
> -			notify_psy = power_supply_get_by_phandle(np,
> +			notify_psy = power_supply_get_by_phandle(of_fwnode_handle(np),
>  						"ti,usb-charger-detection");
>  			if (IS_ERR(notify_psy))
>  				notify_psy = NULL;
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index a8d1fe66e2486a833ccaa3ed77b861c6e52c5760..1d53ceaa8fd161e7e72b90befabb9380393c99f2 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -18,7 +18,6 @@
>  #include <linux/device.h>
>  #include <linux/notifier.h>
>  #include <linux/err.h>
> -#include <linux/of.h>
>  #include <linux/power_supply.h>
>  #include <linux/property.h>
>  #include <linux/thermal.h>
> @@ -196,24 +195,24 @@ static int __power_supply_populate_supplied_from(struct power_supply *epsy,
>  						 void *data)
>  {
>  	struct power_supply *psy = data;
> -	struct device_node *np;
> +	struct fwnode_handle *np;
>  	int i = 0;
>  
>  	do {
> -		np = of_parse_phandle(psy->dev.of_node, "power-supplies", i++);
> -		if (!np)
> +		np = fwnode_find_reference(psy->dev.fwnode, "power-supplies", i++);
> +		if (IS_ERR(np))
>  			break;
>  
> -		if (np == epsy->dev.of_node) {
> +		if (np == epsy->dev.fwnode) {
>  			dev_dbg(&psy->dev, "%s: Found supply : %s\n",
>  				psy->desc->name, epsy->desc->name);
>  			psy->supplied_from[i-1] = (char *)epsy->desc->name;
>  			psy->num_supplies++;
> -			of_node_put(np);
> +			fwnode_handle_put(np);
>  			break;
>  		}
> -		of_node_put(np);
> -	} while (np);
> +		fwnode_handle_put(np);
> +	} while (!IS_ERR(np));
>  
>  	return 0;
>  }
> @@ -232,16 +231,16 @@ static int power_supply_populate_supplied_from(struct power_supply *psy)
>  static int  __power_supply_find_supply_from_node(struct power_supply *epsy,
>  						 void *data)
>  {
> -	struct device_node *np = data;
> +	struct fwnode_handle *fwnode = data;
>  
>  	/* returning non-zero breaks out of power_supply_for_each_psy loop */
> -	if (epsy->dev.of_node == np)
> +	if (epsy->dev.fwnode == fwnode)
>  		return 1;
>  
>  	return 0;
>  }
>  
> -static int power_supply_find_supply_from_node(struct device_node *supply_node)
> +static int power_supply_find_supply_from_fwnode(struct fwnode_handle *supply_node)
>  {
>  	int error;
>  
> @@ -249,7 +248,7 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
>  	 * power_supply_for_each_psy() either returns its own errors or values
>  	 * returned by __power_supply_find_supply_from_node().
>  	 *
> -	 * __power_supply_find_supply_from_node() will return 0 (no match)
> +	 * __power_supply_find_supply_from_fwnode() will return 0 (no match)
>  	 * or 1 (match).
>  	 *
>  	 * We return 0 if power_supply_for_each_psy() returned 1, -EPROBE_DEFER if
> @@ -262,7 +261,7 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
>  
>  static int power_supply_check_supplies(struct power_supply *psy)
>  {
> -	struct device_node *np;
> +	struct fwnode_handle *np;
>  	int cnt = 0;
>  
>  	/* If there is already a list honor it */
> @@ -270,24 +269,24 @@ static int power_supply_check_supplies(struct power_supply *psy)
>  		return 0;
>  
>  	/* No device node found, nothing to do */
> -	if (!psy->dev.of_node)
> +	if (!psy->dev.fwnode)
>  		return 0;
>  
>  	do {
>  		int ret;
>  
> -		np = of_parse_phandle(psy->dev.of_node, "power-supplies", cnt++);
> -		if (!np)
> +		np = fwnode_find_reference(psy->dev.fwnode, "power-supplies", cnt++);
> +		if (IS_ERR(np))
>  			break;
>  
> -		ret = power_supply_find_supply_from_node(np);
> -		of_node_put(np);
> +		ret = power_supply_find_supply_from_fwnode(np);
> +		fwnode_handle_put(np);
>  
>  		if (ret) {
>  			dev_dbg(&psy->dev, "Failed to find supply!\n");
>  			return ret;
>  		}
> -	} while (np);
> +	} while (!IS_ERR(np));
>  
>  	/* Missing valid "power-supplies" entries */
>  	if (cnt == 1)
> @@ -498,14 +497,14 @@ void power_supply_put(struct power_supply *psy)
>  EXPORT_SYMBOL_GPL(power_supply_put);
>  
>  #ifdef CONFIG_OF
> -static int power_supply_match_device_node(struct device *dev, const void *data)
> +static int power_supply_match_device_fwnode(struct device *dev, const void *data)
>  {
> -	return dev->parent && dev->parent->of_node == data;
> +	return dev->parent && dev_fwnode(dev->parent) == data;
>  }
>  
>  /**
>   * power_supply_get_by_phandle() - Search for a power supply and returns its ref
> - * @np: Pointer to device node holding phandle property
> + * @fwnode: Pointer to fwnode holding phandle property
>   * @property: Name of property holding a power supply name
>   *
>   * If power supply was found, it increases reference count for the
> @@ -515,21 +514,21 @@ static int power_supply_match_device_node(struct device *dev, const void *data)
>   * Return: On success returns a reference to a power supply with
>   * matching name equals to value under @property, NULL or ERR_PTR otherwise.
>   */
> -struct power_supply *power_supply_get_by_phandle(struct device_node *np,
> -							const char *property)
> +struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
> +						 const char *property)
>  {
> -	struct device_node *power_supply_np;
> +	struct fwnode_handle *power_supply_fwnode;
>  	struct power_supply *psy = NULL;
>  	struct device *dev;
>  
> -	power_supply_np = of_parse_phandle(np, property, 0);
> -	if (!power_supply_np)
> -		return ERR_PTR(-ENODEV);
> +	power_supply_fwnode = fwnode_find_reference(fwnode, property, 0);
> +	if (IS_ERR(power_supply_fwnode))
> +		return ERR_CAST(power_supply_fwnode);
>  
> -	dev = class_find_device(&power_supply_class, NULL, power_supply_np,
> -				power_supply_match_device_node);
> +	dev = class_find_device(&power_supply_class, NULL, power_supply_fwnode,
> +				power_supply_match_device_fwnode);
>  
> -	of_node_put(power_supply_np);
> +	fwnode_handle_put(power_supply_fwnode);
>  
>  	if (dev) {
>  		psy = dev_to_psy(dev);
> @@ -561,14 +560,14 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
>  {
>  	struct power_supply **ptr, *psy;
>  
> -	if (!dev->of_node)
> +	if (!dev_fwnode(dev))
>  		return ERR_PTR(-ENODEV);
>  
>  	ptr = devres_alloc(devm_power_supply_put, sizeof(*ptr), GFP_KERNEL);
>  	if (!ptr)
>  		return ERR_PTR(-ENOMEM);
>  
> -	psy = power_supply_get_by_phandle(dev->of_node, property);
> +	psy = power_supply_get_by_phandle(dev_fwnode(dev), property);
>  	if (IS_ERR_OR_NULL(psy)) {
>  		devres_free(ptr);
>  	} else {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index b6eb31a23c878aa9ed8ad7bcb02a13721354e1cb..c95f098374cbdeafe8cddb52da3903f4f0e0f0fc 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -805,7 +805,7 @@ static inline struct power_supply *power_supply_get_by_name(const char *name)
>  { return NULL; }
>  #endif
>  #ifdef CONFIG_OF
> -extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
> +extern struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
>  							const char *property);
>  extern struct power_supply *devm_power_supply_get_by_phandle(
>  				    struct device *dev, const char *property);
> 

[PATCH v2 5/5] power: supply: core: rename power_supply_get_by_phandle to power_supply_get_by_reference

[Sebastian Reichel]

(devm_)power_supply_get_by_phandle now internally uses fwnode and are no
longer DT specific. Thus drop the ifdef check for CONFIG_OF and rename
to (devm_)power_supply_get_by_reference to avoid the DT terminology.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c    |  2 +-
 drivers/power/supply/bq2415x_charger.c   |  2 +-
 drivers/power/supply/power_supply_core.c | 22 ++++++++++------------
 include/linux/power_supply.h             | 15 +++------------
 4 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 29b8fd4b935113f3e4790ee7f78141226048492d..8873aed3a52aa3f26564b6b2e576110c4069d28c 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -754,7 +754,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 	}
 
 	if (of_property_present(np, "usb0_vbus_power-supply")) {
-		data->vbus_power_supply = devm_power_supply_get_by_phandle(dev,
+		data->vbus_power_supply = devm_power_supply_get_by_reference(dev,
 						     "usb0_vbus_power-supply");
 		if (IS_ERR(data->vbus_power_supply)) {
 			dev_err(dev, "Couldn't get the VBUS power supply\n");
diff --git a/drivers/power/supply/bq2415x_charger.c b/drivers/power/supply/bq2415x_charger.c
index 1ecbca510bba99ee7abcda33a719035adfceeb5f..917c26ee56bc9f9da2f95f75a7d7f1afb0cea8d8 100644
--- a/drivers/power/supply/bq2415x_charger.c
+++ b/drivers/power/supply/bq2415x_charger.c
@@ -1674,7 +1674,7 @@ static int bq2415x_probe(struct i2c_client *client)
 	/* Query for initial reported_mode and set it */
 	if (bq->nb.notifier_call) {
 		if (np) {
-			notify_psy = power_supply_get_by_phandle(of_fwnode_handle(np),
+			notify_psy = power_supply_get_by_reference(of_fwnode_handle(np),
 						"ti,usb-charger-detection");
 			if (IS_ERR(notify_psy))
 				notify_psy = NULL;
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 1d53ceaa8fd161e7e72b90befabb9380393c99f2..37b9fa48faab27754d14d8379ed40d9bdda098ef 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -496,14 +496,13 @@ void power_supply_put(struct power_supply *psy)
 }
 EXPORT_SYMBOL_GPL(power_supply_put);
 
-#ifdef CONFIG_OF
 static int power_supply_match_device_fwnode(struct device *dev, const void *data)
 {
 	return dev->parent && dev_fwnode(dev->parent) == data;
 }
 
 /**
- * power_supply_get_by_phandle() - Search for a power supply and returns its ref
+ * power_supply_get_by_reference() - Search for a power supply and returns its ref
  * @fwnode: Pointer to fwnode holding phandle property
  * @property: Name of property holding a power supply name
  *
@@ -514,8 +513,8 @@ static int power_supply_match_device_fwnode(struct device *dev, const void *data
  * Return: On success returns a reference to a power supply with
  * matching name equals to value under @property, NULL or ERR_PTR otherwise.
  */
-struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
-						 const char *property)
+struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
+						   const char *property)
 {
 	struct fwnode_handle *power_supply_fwnode;
 	struct power_supply *psy = NULL;
@@ -537,7 +536,7 @@ struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
 
 	return psy;
 }
-EXPORT_SYMBOL_GPL(power_supply_get_by_phandle);
+EXPORT_SYMBOL_GPL(power_supply_get_by_reference);
 
 static void devm_power_supply_put(struct device *dev, void *res)
 {
@@ -547,16 +546,16 @@ static void devm_power_supply_put(struct device *dev, void *res)
 }
 
 /**
- * devm_power_supply_get_by_phandle() - Resource managed version of
- *  power_supply_get_by_phandle()
+ * devm_power_supply_get_by_reference() - Resource managed version of
+ *  power_supply_get_by_reference()
  * @dev: Pointer to device holding phandle property
  * @property: Name of property holding a power supply phandle
  *
  * Return: On success returns a reference to a power supply with
  * matching name equals to value under @property, NULL or ERR_PTR otherwise.
  */
-struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
-						      const char *property)
+struct power_supply *devm_power_supply_get_by_reference(struct device *dev,
+							const char *property)
 {
 	struct power_supply **ptr, *psy;
 
@@ -567,7 +566,7 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	psy = power_supply_get_by_phandle(dev_fwnode(dev), property);
+	psy = power_supply_get_by_reference(dev_fwnode(dev), property);
 	if (IS_ERR_OR_NULL(psy)) {
 		devres_free(ptr);
 	} else {
@@ -576,8 +575,7 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
 	}
 	return psy;
 }
-EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
-#endif /* CONFIG_OF */
+EXPORT_SYMBOL_GPL(devm_power_supply_get_by_reference);
 
 int power_supply_get_battery_info(struct power_supply *psy,
 				  struct power_supply_battery_info **info_out)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index c95f098374cbdeafe8cddb52da3903f4f0e0f0fc..158227e86cfcb91b0fae7b1f9c944c5c395969ca 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -804,19 +804,10 @@ static inline void power_supply_put(struct power_supply *psy) {}
 static inline struct power_supply *power_supply_get_by_name(const char *name)
 { return NULL; }
 #endif
-#ifdef CONFIG_OF
-extern struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
-							const char *property);
-extern struct power_supply *devm_power_supply_get_by_phandle(
+extern struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
+							  const char *property);
+extern struct power_supply *devm_power_supply_get_by_reference(
 				    struct device *dev, const char *property);
-#else /* !CONFIG_OF */
-static inline struct power_supply *
-power_supply_get_by_phandle(struct device_node *np, const char *property)
-{ return NULL; }
-static inline struct power_supply *
-devm_power_supply_get_by_phandle(struct device *dev, const char *property)
-{ return NULL; }
-#endif /* CONFIG_OF */
 
 extern const enum power_supply_property power_supply_battery_info_properties[];
 extern const size_t power_supply_battery_info_properties_size;

-- 
2.47.2

Re: [PATCH v2 5/5] power: supply: core: rename power_supply_get_by_phandle to power_supply_get_by_reference

[Hans de Goede]

Hi,


On 30-Apr-25 12:54 AM, Sebastian Reichel wrote:
> (devm_)power_supply_get_by_phandle now internally uses fwnode and are no
> longer DT specific. Thus drop the ifdef check for CONFIG_OF and rename
> to (devm_)power_supply_get_by_reference to avoid the DT terminology.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans


> ---
>  drivers/phy/allwinner/phy-sun4i-usb.c    |  2 +-
>  drivers/power/supply/bq2415x_charger.c   |  2 +-
>  drivers/power/supply/power_supply_core.c | 22 ++++++++++------------
>  include/linux/power_supply.h             | 15 +++------------
>  4 files changed, 15 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 29b8fd4b935113f3e4790ee7f78141226048492d..8873aed3a52aa3f26564b6b2e576110c4069d28c 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -754,7 +754,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>  	}
>  
>  	if (of_property_present(np, "usb0_vbus_power-supply")) {
> -		data->vbus_power_supply = devm_power_supply_get_by_phandle(dev,
> +		data->vbus_power_supply = devm_power_supply_get_by_reference(dev,
>  						     "usb0_vbus_power-supply");
>  		if (IS_ERR(data->vbus_power_supply)) {
>  			dev_err(dev, "Couldn't get the VBUS power supply\n");
> diff --git a/drivers/power/supply/bq2415x_charger.c b/drivers/power/supply/bq2415x_charger.c
> index 1ecbca510bba99ee7abcda33a719035adfceeb5f..917c26ee56bc9f9da2f95f75a7d7f1afb0cea8d8 100644
> --- a/drivers/power/supply/bq2415x_charger.c
> +++ b/drivers/power/supply/bq2415x_charger.c
> @@ -1674,7 +1674,7 @@ static int bq2415x_probe(struct i2c_client *client)
>  	/* Query for initial reported_mode and set it */
>  	if (bq->nb.notifier_call) {
>  		if (np) {
> -			notify_psy = power_supply_get_by_phandle(of_fwnode_handle(np),
> +			notify_psy = power_supply_get_by_reference(of_fwnode_handle(np),
>  						"ti,usb-charger-detection");
>  			if (IS_ERR(notify_psy))
>  				notify_psy = NULL;
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 1d53ceaa8fd161e7e72b90befabb9380393c99f2..37b9fa48faab27754d14d8379ed40d9bdda098ef 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -496,14 +496,13 @@ void power_supply_put(struct power_supply *psy)
>  }
>  EXPORT_SYMBOL_GPL(power_supply_put);
>  
> -#ifdef CONFIG_OF
>  static int power_supply_match_device_fwnode(struct device *dev, const void *data)
>  {
>  	return dev->parent && dev_fwnode(dev->parent) == data;
>  }
>  
>  /**
> - * power_supply_get_by_phandle() - Search for a power supply and returns its ref
> + * power_supply_get_by_reference() - Search for a power supply and returns its ref
>   * @fwnode: Pointer to fwnode holding phandle property
>   * @property: Name of property holding a power supply name
>   *
> @@ -514,8 +513,8 @@ static int power_supply_match_device_fwnode(struct device *dev, const void *data
>   * Return: On success returns a reference to a power supply with
>   * matching name equals to value under @property, NULL or ERR_PTR otherwise.
>   */
> -struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
> -						 const char *property)
> +struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
> +						   const char *property)
>  {
>  	struct fwnode_handle *power_supply_fwnode;
>  	struct power_supply *psy = NULL;
> @@ -537,7 +536,7 @@ struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
>  
>  	return psy;
>  }
> -EXPORT_SYMBOL_GPL(power_supply_get_by_phandle);
> +EXPORT_SYMBOL_GPL(power_supply_get_by_reference);
>  
>  static void devm_power_supply_put(struct device *dev, void *res)
>  {
> @@ -547,16 +546,16 @@ static void devm_power_supply_put(struct device *dev, void *res)
>  }
>  
>  /**
> - * devm_power_supply_get_by_phandle() - Resource managed version of
> - *  power_supply_get_by_phandle()
> + * devm_power_supply_get_by_reference() - Resource managed version of
> + *  power_supply_get_by_reference()
>   * @dev: Pointer to device holding phandle property
>   * @property: Name of property holding a power supply phandle
>   *
>   * Return: On success returns a reference to a power supply with
>   * matching name equals to value under @property, NULL or ERR_PTR otherwise.
>   */
> -struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
> -						      const char *property)
> +struct power_supply *devm_power_supply_get_by_reference(struct device *dev,
> +							const char *property)
>  {
>  	struct power_supply **ptr, *psy;
>  
> @@ -567,7 +566,7 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
>  	if (!ptr)
>  		return ERR_PTR(-ENOMEM);
>  
> -	psy = power_supply_get_by_phandle(dev_fwnode(dev), property);
> +	psy = power_supply_get_by_reference(dev_fwnode(dev), property);
>  	if (IS_ERR_OR_NULL(psy)) {
>  		devres_free(ptr);
>  	} else {
> @@ -576,8 +575,7 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
>  	}
>  	return psy;
>  }
> -EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
> -#endif /* CONFIG_OF */
> +EXPORT_SYMBOL_GPL(devm_power_supply_get_by_reference);
>  
>  int power_supply_get_battery_info(struct power_supply *psy,
>  				  struct power_supply_battery_info **info_out)
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index c95f098374cbdeafe8cddb52da3903f4f0e0f0fc..158227e86cfcb91b0fae7b1f9c944c5c395969ca 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -804,19 +804,10 @@ static inline void power_supply_put(struct power_supply *psy) {}
>  static inline struct power_supply *power_supply_get_by_name(const char *name)
>  { return NULL; }
>  #endif
> -#ifdef CONFIG_OF
> -extern struct power_supply *power_supply_get_by_phandle(struct fwnode_handle *fwnode,
> -							const char *property);
> -extern struct power_supply *devm_power_supply_get_by_phandle(
> +extern struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
> +							  const char *property);
> +extern struct power_supply *devm_power_supply_get_by_reference(
>  				    struct device *dev, const char *property);
> -#else /* !CONFIG_OF */
> -static inline struct power_supply *
> -power_supply_get_by_phandle(struct device_node *np, const char *property)
> -{ return NULL; }
> -static inline struct power_supply *
> -devm_power_supply_get_by_phandle(struct device *dev, const char *property)
> -{ return NULL; }
> -#endif /* CONFIG_OF */
>  
>  extern const enum power_supply_property power_supply_battery_info_properties[];
>  extern const size_t power_supply_battery_info_properties_size;
>