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

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 25/02/2025 15:11, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Feb 25, 2025 at 01:14:03PM +0200, Matti Vaittinen wrote:
>> 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@collabora.com>
>>> ---
>>>    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.
> 
> I was thinking about adding an init function to power_supply_desc,
> which would be called directly before psy->initialized is set to
> true in the power-supply registration phase. I think that would be
> the right place to setup device registers based on battery-info data.

Hm. I suppose that would work.

> But it's definitely not a thing for this series.

Fair enough. I thought that might be the case but decided to ask 
anyways, because, AFAIR there were no real problems what comes to not 
requiring the struct power_supply for reading the battery_info. But yes, 
I can see why killing the of_nodes is a big enough series, even without 
added complexity :)

Yours,
	-- Matti