Re: [PATCH 1/7] power: supply: core: get rid of of_node

[Matti Vaittinen]

On 25/02/2025 01:21, Sebastian Reichel wrote:
> This removes .of_node from 'struct power_supply', since there
> is already a copy in .dev.of_node and there is no need to have
> two copies.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> ---
>   drivers/power/supply/power_supply_core.c | 17 ++++++++---------
>   include/linux/power_supply.h             |  1 -
>   2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index d0bb52a7a0367a8e07787be211691cad14a41a54..11030035da6f121ca76bebf800c06cfd5db57578 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -200,11 +200,11 @@ static int __power_supply_populate_supplied_from(struct power_supply *epsy,
>   	int i = 0;
>   
>   	do {
> -		np = of_parse_phandle(psy->of_node, "power-supplies", i++);
> +		np = of_parse_phandle(psy->dev.of_node, "power-supplies", i++);
>   		if (!np)
>   			break;
>   
> -		if (np == epsy->of_node) {
> +		if (np == epsy->dev.of_node) {
>   			dev_dbg(&psy->dev, "%s: Found supply : %s\n",
>   				psy->desc->name, epsy->desc->name);
>   			psy->supplied_from[i-1] = (char *)epsy->desc->name;
> @@ -235,7 +235,7 @@ static int  __power_supply_find_supply_from_node(struct power_supply *epsy,
>   	struct device_node *np = data;
>   
>   	/* returning non-zero breaks out of power_supply_for_each_psy loop */
> -	if (epsy->of_node == np)
> +	if (epsy->dev.of_node == np)
>   		return 1;
>   
>   	return 0;
> @@ -270,13 +270,13 @@ static int power_supply_check_supplies(struct power_supply *psy)
>   		return 0;
>   
>   	/* No device node found, nothing to do */
> -	if (!psy->of_node)
> +	if (!psy->dev.of_node)
>   		return 0;
>   
>   	do {
>   		int ret;
>   
> -		np = of_parse_phandle(psy->of_node, "power-supplies", cnt++);
> +		np = of_parse_phandle(psy->dev.of_node, "power-supplies", cnt++);
>   		if (!np)
>   			break;
>   
> @@ -606,8 +606,8 @@ int power_supply_get_battery_info(struct power_supply *psy,
>   	const __be32 *list;
>   	u32 min_max[2];
>   
> -	if (psy->of_node) {
> -		battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
> +	if (psy->dev.of_node) {
> +		battery_np = of_parse_phandle(psy->dev.of_node, "monitored-battery", 0);
>   		if (!battery_np)
>   			return -ENODEV;

This reminded me of a change I once did to power_supply - but maybe 
never got it further than RFC stage. Anyways, do you think it would be 
possible to decouple the battery info and struct power_suppply (while at 
it)?

I believe that the chargers and especially fuel-gauges which are 
designed to operate with different batteries (and which get battery 
details using static battery nodes), would like to get the battery info 
_before_ registering the power_supply (to avoid sending bogus values 
while operating on defaults, before the battery info is read and before 
things are set accordingly).

I know this may be a bit much to ask, but I believe it'd be an improvement.

Other than that, looks good to me.

>   
> @@ -1544,9 +1544,8 @@ __power_supply_register(struct device *parent,
>   	if (cfg) {
>   		dev->groups = cfg->attr_grp;
>   		psy->drv_data = cfg->drv_data;
> -		psy->of_node =
> +		dev->of_node =
>   			cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node;
> -		dev->of_node = psy->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 6ed53b292162469d7b357734d5589bff18a201d0..975ccab56597ef579ef0c9dc913dcb0a26b5855a 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -316,7 +316,6 @@ struct power_supply {
>   
>   	char **supplied_from;
>   	size_t num_supplies;
> -	struct device_node *of_node;
>   
>   	/* Driver private data */
>   	void *drv_data;