* Re: [PATCH] power: supply: core: Honor supplied-from with CONFIG_OF=y
[not found] <20260901145156.3177187-1-mauriziocasciano7@gmail.com>
@ 2026-09-01 16:07 ` Hans de Goede
2026-09-01 17:04 ` Maurizio Casciano
0 siblings, 1 reply; 2+ messages in thread
From: Hans de Goede @ 2026-09-01 16:07 UTC (permalink / raw)
To: Maurizio Casciano, Sebastian Reichel
Cc: linux-pm, linux-kernel, Maurizio Casciano, stable
Hi,
Thank you for your patch.
On 1-Sep-26 16:51, Maurizio Casciano wrote:
> From: Maurizio Casciano <maurizio.casciano@eng.it>
>
> The supplied-from device property is the name-based counterpart to
> firmware-node power-supplies references. It was added for non-DT platforms,
> but its parser is compiled only when CONFIG_OF is disabled. CONFIG_OF is a
> global kernel option, so x86 systems commonly enable it even when
> individual power supplies are described by software nodes.
>
> Consequently, these consumers never populate supplied_from and supplier
> notifications do not reach their external_power_changed() callbacks. On a
> Lenovo Yoga Book YB1-X91L, ftrace showed the Whiskey Cove supplier
> notification running without invoking the BQ25892 callback, leaving the
> input current limit at its boot-time value.
>
> Move the generic supplied-from parser into an unconditional helper and try
> it before firmware-reference power-supplies lookup. Keep an explicitly
> supplied list at the highest priority and retain power-supplies as the
> fallback. With the fix, ftrace shows the BQ25892 callback on hotplug and a
> boot-offline test changes its input current limit from 500 mA to 2 A.
>
> Fixes: 58a36bb06891 ("power: supply: core: Add support for supplied-from device-property")
> Cc: stable@vger.kernel.org
> Signed-off-by: Maurizio Casciano <maurizio.casciano@eng.it>
Interesting. This seems to be a new problem / development in 7.3-rc1
where it seems CONFIG_OF now seems to get enabled on x86 configs.
This change has also lead to other problems, e.g. :
https://bugzilla.redhat.com/show_bug.cgi?id=2523734#c7
Still I agree that this code should do the right thing when CONFIG_OF
is enabled on x86 which it currently clearly is not doing.
But I don't taking that making the new power_supply_check_supplies_by_name()
function higher priority then proper OF/devicetree node links is a good
idea.
IMHO this should be the fallback (in the CONFIG_OF enabled case)
when no suppliers are found through looking at DT node links first.
Sebastian, what do you think ?
Regards,
Hans
> ---
> drivers/power/supply/power_supply_core.c | 61 ++++++++++++++----------
> 1 file changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 00d8bc98d588..5101decebb7a 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -190,6 +190,35 @@ static void power_supply_deferred_register_work(struct work_struct *work)
> device_unlock(psy->dev.parent);
> }
>
> +static int power_supply_check_supplies_by_name(struct power_supply *psy)
> +{
> + struct device *parent = psy->dev.parent;
> + int nval, ret;
> +
> + if (!parent)
> + return 0;
> +
> + nval = device_property_string_array_count(parent, "supplied-from");
> + if (nval <= 0)
> + return 0;
> +
> + psy->supplied_from = devm_kmalloc_array(&psy->dev, nval,
> + sizeof(*psy->supplied_from),
> + GFP_KERNEL);
> + if (!psy->supplied_from)
> + return -ENOMEM;
> +
> + ret = device_property_read_string_array(parent, "supplied-from",
> + (const char **)psy->supplied_from,
> + nval);
> + if (ret < 0)
> + return ret;
> +
> + psy->num_supplies = nval;
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_OF
> static int __power_supply_populate_supplied_from(struct power_supply *epsy,
> void *data)
> @@ -262,19 +291,22 @@ static int power_supply_find_supply_from_fwnode(struct fwnode_handle *supply_nod
> static int power_supply_check_supplies(struct power_supply *psy)
> {
> struct fwnode_handle *np;
> - int cnt = 0;
> + int cnt = 0, ret;
>
> /* If there is already a list honor it */
> if (psy->supplied_from && psy->num_supplies > 0)
> return 0;
>
> + /* Check for the name-based "supplied-from" device property first. */
> + ret = power_supply_check_supplies_by_name(psy);
> + if (ret || psy->num_supplies)
> + return ret;
> +
> /* No device node found, nothing to do */
> if (!psy->dev.fwnode)
> return 0;
>
> do {
> - int ret;
> -
> np = fwnode_find_reference(psy->dev.fwnode, "power-supplies", cnt++);
> if (IS_ERR(np))
> break;
> @@ -304,28 +336,7 @@ static int power_supply_check_supplies(struct power_supply *psy)
> #else
> static int power_supply_check_supplies(struct power_supply *psy)
> {
> - int nval, ret;
> -
> - if (!psy->dev.parent)
> - return 0;
> -
> - nval = device_property_string_array_count(psy->dev.parent, "supplied-from");
> - if (nval <= 0)
> - return 0;
> -
> - psy->supplied_from = devm_kmalloc_array(&psy->dev, nval,
> - sizeof(char *), GFP_KERNEL);
> - if (!psy->supplied_from)
> - return -ENOMEM;
> -
> - ret = device_property_read_string_array(psy->dev.parent,
> - "supplied-from", (const char **)psy->supplied_from, nval);
> - if (ret < 0)
> - return ret;
> -
> - psy->num_supplies = nval;
> -
> - return 0;
> + return power_supply_check_supplies_by_name(psy);
> }
> #endif
>
^ permalink raw reply [flat|nested] 2+ messages in thread