linux-kernel - Re: [PATCH 1/7] power: supply: core: get rid of of

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ocbwzuqk56yx34kc5vp6aaxnhxqd4zp2wixlv7p3mex66ibntu@ahigikrf5cg4>
Date: Tue, 25 Feb 2025 14:11:39 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Mark Brown <broonie@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Linus Walleij <linus.walleij@...aro.org>, 
	Hans de Goede <hdegoede@...hat.com>, Liam Girdwood <lgirdwood@...il.com>, 
	Chen-Yu Tsai <wens@...e.org>, Pali Rohár <pali@...nel.org>, 
	Paul Cercueil <paul@...pouillou.net>, Samuel Holland <samuel@...lland.org>, 
	David Lechner <david@...hnology.com>, Krzysztof Kozlowski <krzk@...nel.org>, 
	Marek Szyprowski <m.szyprowski@...sung.com>, Sebastian Krzyszkowiak <sebastian.krzyszkowiak@...i.sm>, 
	Purism Kernel Team <kernel@...i.sm>, Bartosz Golaszewski <brgl@...ev.pl>, 
	Konrad Dybcio <konradybcio@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Orson Zhai <orsonzhai@...il.com>, 
	Baolin Wang <baolin.wang@...ux.alibaba.com>, Chunyan Zhang <zhang.lyra@...il.com>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 1/7] power: supply: core: get rid of of_node

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@...labora.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.
But it's definitely not a thing for this series.

Greetings,

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)