[PATCH 00/43] power: Remove redundant error messages on IRQ request failure

[PATCH 00/43] power: Remove redundant error messages on IRQ request failure

[Pan Chuang]

Commit 55b48e23f5c4b6f5ca9b7ab09599b17dcf501c10 ("genirq/devres: Add
error handling in devm_request_*_irq()") added automatic error logging
to devm_request_threaded_irq() and devm_request_any_context_irq() via
the new devm_request_result() helper. The helper prints device name,
IRQ number, handler functions, and error code on failure.

Since devm_request_irq() is a static inline wrapper around
devm_request_threaded_irq(), it also benefits from this automatic
logging.

This series removes the now-redundant dev_err() and dev_err_probe() calls
in power drivers that follow these devm_request_*_irq() functions, as the
core now provides more detailed diagnostic information on failure.

Pan Chuang (43):
  power: reset: pwr-mlxbf: Remove redundant dev_err()/dev_err_probe()
  power: supply: 88pm860x_battery: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: 88pm860x_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: ab8500_btemp: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: ab8500_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: ab8500_fg: Remove redundant dev_err()/dev_err_probe()
  power: supply: act8945a_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: axp20x_ac_power: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: axp20x_usb_power: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: axp288_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: axp288_fuel_gauge: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: bq24190_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: bq24257_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: bq24735-charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: bq256xx_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: bq257xx_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: cpcap-battery: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: cpcap-charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: da9150-fg: Remove redundant dev_err()/dev_err_probe()
  power: supply: generic-adc-battery: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: max14656_charger_detector: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: max77759_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: max8903_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: max8971_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: mp2629_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: mt6360_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: mt6370-charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: pf1550-charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: qcom_smbb: Remove redundant dev_err()/dev_err_probe()
  power: supply: qcom_smbx: Remove redundant dev_err()/dev_err_probe()
  power: supply: rk817_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: rn5t618_power: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: rt9455_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: rt9467-charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: rt9471: Remove redundant dev_err()/dev_err_probe()
  power: supply: sbs-charger: Remove redundant dev_err()/dev_err_probe()
  power: supply: sc27xx_fuel_gauge: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: surface-rt-ec: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: tps65090-charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: tps65217_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: twl4030_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: twl6030_charger: Remove redundant
    dev_err()/dev_err_probe()
  power: supply: ucs1002_power: Remove redundant
    dev_err()/dev_err_probe()

 drivers/power/reset/pwr-mlxbf.c                  |  2 --
 drivers/power/supply/88pm860x_battery.c          | 10 ++--------
 drivers/power/supply/88pm860x_charger.c          |  5 +----
 drivers/power/supply/ab8500_btemp.c              |  5 +----
 drivers/power/supply/ab8500_charger.c            |  5 +----
 drivers/power/supply/ab8500_fg.c                 |  2 --
 drivers/power/supply/act8945a_charger.c          |  4 +---
 drivers/power/supply/axp20x_ac_power.c           |  5 +----
 drivers/power/supply/axp20x_usb_power.c          |  5 +----
 drivers/power/supply/axp288_charger.c            |  3 +--
 drivers/power/supply/axp288_fuel_gauge.c         |  2 +-
 drivers/power/supply/bq24190_charger.c           |  4 +---
 drivers/power/supply/bq24257_charger.c           |  4 +---
 drivers/power/supply/bq24735-charger.c           |  6 +-----
 drivers/power/supply/bq256xx_charger.c           |  4 +---
 drivers/power/supply/bq257xx_charger.c           |  2 --
 drivers/power/supply/cpcap-battery.c             |  6 +-----
 drivers/power/supply/cpcap-charger.c             |  6 +-----
 drivers/power/supply/da9150-fg.c                 |  4 +---
 drivers/power/supply/generic-adc-battery.c       |  2 +-
 drivers/power/supply/max14656_charger_detector.c |  4 +---
 drivers/power/supply/max77759_charger.c          |  4 +---
 drivers/power/supply/max8903_charger.c           | 15 +++------------
 drivers/power/supply/max8971_charger.c           |  2 +-
 drivers/power/supply/mp2629_charger.c            |  4 +---
 drivers/power/supply/mt6360_charger.c            |  3 +--
 drivers/power/supply/mt6370-charger.c            |  4 +---
 drivers/power/supply/pf1550-charger.c            |  3 +--
 drivers/power/supply/qcom_smbb.c                 |  5 +----
 drivers/power/supply/qcom_smbx.c                 |  3 +--
 drivers/power/supply/rk817_charger.c             | 12 ++++--------
 drivers/power/supply/rn5t618_power.c             |  5 +----
 drivers/power/supply/rt9455_charger.c            |  4 +---
 drivers/power/supply/rt9467-charger.c            |  3 +--
 drivers/power/supply/rt9471.c                    |  3 +--
 drivers/power/supply/sbs-charger.c               |  2 +-
 drivers/power/supply/sc27xx_fuel_gauge.c         |  8 ++------
 drivers/power/supply/surface-rt-ec.c             |  2 +-
 drivers/power/supply/tps65090-charger.c          |  6 +-----
 drivers/power/supply/tps65217_charger.c          |  6 +-----
 drivers/power/supply/twl4030_charger.c           | 10 ++--------
 drivers/power/supply/twl6030_charger.c           |  4 +---
 drivers/power/supply/ucs1002_power.c             | 10 ++--------
 43 files changed, 49 insertions(+), 159 deletions(-)

-- 
2.34.1

[PATCH 28/43] power: supply: pf1550-charger: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_threaded_irq() and devm_request_irq() now automatically
log detailed error messages on failure. This eliminates the need for
driver-specific dev_err() and dev_err_probe() calls that previously
printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/power/supply/pf1550-charger.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
index 41036f4cb64a..94fe81bab5a2 100644
--- a/drivers/power/supply/pf1550-charger.c
+++ b/drivers/power/supply/pf1550-charger.c
@@ -612,8 +612,7 @@ static int pf1550_charger_probe(struct platform_device *pdev)
 						IRQF_NO_SUSPEND,
 						"pf1550-charger", chg);
 		if (ret)
-			return dev_err_probe(&pdev->dev, ret,
-					     "failed irq request\n");
+			return ret;
 	}
 
 	pf1550_dt_parse_dev_info(chg);
-- 
2.34.1

Re: [PATCH 28/43] power: supply: pf1550-charger: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

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

Pre-existing issues:
- [High] Inverted logic when turning on the charger based on battery presence in `pf1550_reg_init()`.
- [High] Use-after-free vulnerability during driver unbind or probe failure due to incorrect `devm` registration order.
- [High] Guaranteed probe failure for device trees omitting the optional `constant-charge-voltage-max-microvolt` property due to unhandled `-EINVAL`.
- [Medium] Resource leak of `struct power_supply_battery_info` due to uncalled `power_supply_put_battery_info()`.
--

--- Patch [28]: [PATCH 28/43] power: supply: pf1550-charger: Remove redundant dev_err()/dev_err_probe() ---
commit 7f7745c9eb293f3e6b24f5df79b35be592a07130
Author: Pan Chuang <panchuang@vivo.com>

power: supply: pf1550-charger: Remove redundant dev_err()/dev_err_probe()
    
The devm_request_threaded_irq() and devm_request_irq() now automatically
log detailed error messages on failure. This eliminates the need for
driver-specific dev_err() and dev_err_probe() calls.

> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> index 41036f4cb64a0..94fe81bab5a21 100644
> --- a/drivers/power/supply/pf1550-charger.c
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -612,8 +612,7 @@ static int pf1550_charger_probe(struct platform_device *pdev)
>  						IRQF_NO_SUSPEND,
>  						"pf1550-charger", chg);
>  		if (ret)
> -			return dev_err_probe(&pdev->dev, ret,
> -					     "failed irq request\n");
> +			return ret;
>  	}
>  
>  	pf1550_dt_parse_dev_info(chg);

[Severity: High]
This is a pre-existing issue, but does this devm registration order leave 
a use-after-free window during device unbind?

In drivers/power/supply/pf1550-charger.c:pf1550_charger_probe(), the delayed
works are registered before the power supplies:

	ret = devm_delayed_work_autocancel(chg->dev, &chg->vbus_sense_work,
					   pf1550_chg_vbus_work);
	...
	chg->charger = devm_power_supply_register(&pdev->dev,
						  &pf1550_charger_desc,
						  &psy_cfg);

Since devres cleans up in reverse order, the power supplies will be freed
first. If pf1550_chg_vbus_work executes before the work is canceled, it
will call power_supply_changed(chg->battery) on a freed power supply
pointer.


[Severity: Medium]
This is a pre-existing issue, but does this code leak the battery info
structure?

Looking at pf1550_dt_parse_dev_info(), it calls power_supply_get_battery_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;

The same call also occurs in pf1550_reg_init(). However, there are no
matching calls to power_supply_put_battery_info() in the driver to free the
memory allocated by these lookups.


[Severity: High]
This is a pre-existing issue, but does this code handle missing devicetree
properties correctly?

In 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;

If the devicetree lacks the optional constant-charge-voltage-max-microvolt
property, info->constant_charge_voltage_max_uv is set to -EINVAL. This
negative error code is then cast to the u32 chg->constant_volt, which
becomes 4294967274 and guarantees a failure in later upper-bound checks.


[Severity: High]
This is a pre-existing issue, but is the logic for enabling the charger
inverted here?

Looking at pf1550_reg_init():

	if (power_supply_get_battery_info(chg->charger, &info)) {
		ret = regmap_write(chg->pf1550->regmap,
				   PF1550_CHARG_REG_CHG_OPER,
				   PF1550_CHG_BAT_ON);

Since power_supply_get_battery_info() returns 0 on success and an error code
like -ENODEV on failure, this condition evaluates to true only when the
battery info lookup fails. This appears to turn on the charger exactly when
the battery is missing or misconfigured.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709033428.362970-1-panchuang@vivo.com?part=28