[PATCH v2] power: supply: pf1550: enable charging when battery profile exists

[PATCH v2] power: supply: pf1550: enable charging when battery profile exists

[raoxu]

From: Xu Rao <raoxu@uniontech.com>

PF1550 starts in charger mode 1, where charging is disabled. The driver
comment says that mode 2 should be selected for applications using a
battery, but the condition is inverted: PF1550_CHG_BAT_ON is written only
when power_supply_get_battery_info() fails.

Consequently, a board with a valid monitored-battery profile is left in
the default charger-off mode, while a board without battery information
enables charging with fallback settings.

Select mode 2 when battery information is available.

Fixes: 4b6b6433a97d ("power: supply: pf1550: add battery charger support")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
Changes in v2:
- Drop power_supply_put_battery_info() calls because static battery
  profiles are not devm allocated.
- Leave the pre-existing handling of an unspecified constant charge
  voltage for a separate fix.

 drivers/power/supply/pf1550-charger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
index 41036f4cb64a..38cac156d2e6 100644
--- a/drivers/power/supply/pf1550-charger.c
+++ b/drivers/power/supply/pf1550-charger.c
@@ -514,7 +514,7 @@ static int pf1550_reg_init(struct pf1550_charger *chg)
 	 * a battery. The other supported mode is mode 2, the charger is turned
 	 * on to charge a battery when present.
 	 */
-	if (power_supply_get_battery_info(chg->charger, &info)) {
+	if (!power_supply_get_battery_info(chg->charger, &info)) {
 		ret = regmap_write(chg->pf1550->regmap,
 				   PF1550_CHARG_REG_CHG_OPER,
 				   PF1550_CHG_BAT_ON);
--
2.50.1

Re: [PATCH v2] power: supply: pf1550: enable charging when battery profile exists

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unspecified `constant-charge-voltage-max-microvolt` causes driver probe to fail due to unsigned conversion of `-EINVAL`.
- [High] Delayed works are initialized before power supplies are registered, leading to a potential Use-After-Free during teardown.
--

--- Patch [1]: [PATCH v2] power: supply: pf1550: enable charging when battery profile exists ---
commit d674322929d0145285ab1259aa1b4032720d8bd7
Author: Xu Rao <raoxu@uniontech.com>

power: supply: pf1550: enable charging when battery profile exists

PF1550 starts in charger mode 1, where charging is disabled. The driver
comment says that mode 2 should be selected for applications using a
battery, but the condition is inverted: PF1550_CHG_BAT_ON is written only
when power_supply_get_battery_info() fails.

Cc: stable@vger.kernel.org

> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> index a457862ef4610..a826a938f52d7 100644
> --- a/drivers/power/supply/pf1550-charger.c
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -514,7 +514,7 @@ static int pf1550_reg_init(struct pf1550_charger *chg)
>  	 * a battery. The other supported mode is mode 2, the charger is turned
>  	 * on to charge a battery when present.
>  	 */
> -	if (power_supply_get_battery_info(chg->charger, &info)) {
> +	if (!power_supply_get_battery_info(chg->charger, &info)) {
>  		ret = regmap_write(chg->pf1550->regmap,
>  				   PF1550_CHARG_REG_CHG_OPER,
>  				   PF1550_CHG_BAT_ON);

[Severity: High]
This isn't a bug introduced by this patch, but if the device tree specifies
a simple-battery but omits the optional constant-charge-voltage-max-microvolt
property, power_supply_get_battery_info() leaves
info->constant_charge_voltage_max_uv initialized to -EINVAL.

This negative error code is assigned to chg->constant_volt (a u32) in
pf1550_dt_parse_dev_info():

drivers/power/supply/pf1550-charger.c:pf1550_dt_parse_dev_info() {
    ...
    if (power_supply_get_battery_info(chg->charger, &info))
        chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
    else
        chg->constant_volt = info->constant_charge_voltage_max_uv;
    ...
}

Later, pf1550_set_constant_volt() checks this huge unsigned value and
fails its bounds check, returning -EINVAL:

drivers/power/supply/pf1550-charger.c:pf1550_set_constant_volt() {
    ...
    if (val < 3500000 || val > 4440000)
        return dev_err_probe(chg->dev, -EINVAL,
                             "Wrong value for constant voltage\n");
    ...
}

Could this cause the driver probe to unconditionally fail when the property
is missing?


[Severity: High]
This is a pre-existing issue, but I noticed the delayed works are
initialized before the power supplies in pf1550_charger_probe():

drivers/power/supply/pf1550-charger.c:pf1550_charger_probe() {
    ...
    ret = devm_delayed_work_autocancel(chg->dev, &chg->vbus_sense_work,
                                       pf1550_chg_vbus_work);
    ...
    chg->charger = devm_power_supply_register(&pdev->dev, ...);
    ...
}

Since devres framework cleans up resources in reverse order (LIFO), during
unbind or late probe failure, the power supplies will be unregistered and
freed first, and the delayed works cancelled afterward.

If pf1550_chg_vbus_work() executes in this window, it calls
power_supply_changed(chg->battery). Will this dereference a freed power
supply pointer and cause a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/097F0559A936ACCB+20260724095437.368905-1-raoxu@uniontech.com?part=1

Re: [PATCH v2] power: supply: pf1550: enable charging when battery profile exists

[Frank Li]

On Fri, Jul 24, 2026 at 05:54:37PM +0800, raoxu wrote:
> [You don't often get email from raoxu@uniontech.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Xu Rao <raoxu@uniontech.com>
>
> PF1550 starts in charger mode 1, where charging is disabled. The driver
> comment says that mode 2 should be selected for applications using a
> battery, but the condition is inverted: PF1550_CHG_BAT_ON is written only
> when power_supply_get_battery_info() fails.
>
> Consequently, a board with a valid monitored-battery profile is left in
> the default charger-off mode, while a board without battery information
> enables charging with fallback settings.
>
> Select mode 2 when battery information is available.
>
> Fixes: 4b6b6433a97d ("power: supply: pf1550: add battery charger support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v2:
> - Drop power_supply_put_battery_info() calls because static battery
>   profiles are not devm allocated.
> - Leave the pre-existing handling of an unspecified constant charge
>   voltage for a separate fix.
>
>  drivers/power/supply/pf1550-charger.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> index 41036f4cb64a..38cac156d2e6 100644
> --- a/drivers/power/supply/pf1550-charger.c
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -514,7 +514,7 @@ static int pf1550_reg_init(struct pf1550_charger *chg)
>          * a battery. The other supported mode is mode 2, the charger is turned
>          * on to charge a battery when present.
>          */
> -       if (power_supply_get_battery_info(chg->charger, &info)) {
> +       if (!power_supply_get_battery_info(chg->charger, &info)) {
>                 ret = regmap_write(chg->pf1550->regmap,
>                                    PF1550_CHARG_REG_CHG_OPER,
>                                    PF1550_CHG_BAT_ON);
> --
> 2.50.1
>