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;
@@ -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;
--
2.47.2On 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.
>
> @@ -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;
>
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.
But it's definitely not a thing for this series.
Greetings,
-- Sebastian
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
Il 25/02/25 00:21, Sebastian Reichel ha scritto: > 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> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
© 2016 - 2026 Red Hat, Inc.