[PATCH v3 0/2] power: supply: bq25630: scope the battery information

[PATCH v3 0/2] power: supply: bq25630: scope the battery information

[Linmao Li]

data->batinfo is only read by bq25630_setup(), but it is fetched in
bq25630_probe() and kept in the driver data for the lifetime of the
device.

Patch 1 moves the get/put pair into bq25630_setup() and drops the member.
Patch 2 then uses the new power_supply_desc::init callback so the charger
registers are programmed before the device is exposed to the system.

Patch 2 depends on commit c1eb5905fdce ("power: supply: Add registration
init callback"), which is in the power-supply tree but not in linux-next
yet. The series is therefore based on power-supply/for-next.

Changes in v3:
- Use dev_err() instead of dev_err_probe() for the battery info failure in
  bq25630_setup(). The function is currently only reached from probe(), but
  that should not be baked in as a policy, and the rest of the function
  already uses dev_err() (Waqar Hameed).
- Rebased onto current power-supply/for-next.

Note that v2 went out with a broken charset in the Content-Type header,
which made git am fail. That was a send-email misconfiguration on my side,
not a change in the patches themselves; it is fixed for this posting.
Thanks to Waqar Hameed for reporting it.

Changes in v2:
- Move the battery information get/put pair into bq25630_setup().
- Remove the now-unused batinfo member from the driver data.
- Use power_supply_desc::init to complete the hardware setup before the
  power supply is exposed.
- Split the changes into two patches.

Link to v2:
https://lore.kernel.org/linux-pm/20260728024558.3611522-1-lilinmao@kylinos.cn/
Link to v1:
https://lore.kernel.org/linux-pm/20260727093738.2611185-1-lilinmao@kylinos.cn/

Compile-tested on power-supply/for-next with CONFIG_CHARGER_BQ25630=m; both
patches build cleanly with W=1. Not tested on hardware.

Linmao Li (2):
  power: supply: bq25630: Scope battery information to bq25630_setup()
  power: supply: bq25630: Initialize hardware before exposing the power
    supply

 drivers/power/supply/bq25630_charger.c | 56 +++++++++++++-------------
 1 file changed, 29 insertions(+), 27 deletions(-)


base-commit: 5584ad5706e594c7648655171785aa4fffdd3db5
-- 
2.25.1

[PATCH v3 1/2] power: supply: bq25630: Scope battery information to bq25630_setup()

[Linmao Li]

data->batinfo is only used by bq25630_setup() to program the initial
charge limits, but power_supply_get_battery_info() allocates it on
psy->dev, so it stays around for the lifetime of the device. Nothing
else in the driver uses it.

Get the battery information in bq25630_setup(), just before it is read,
and release it on every path out of that function. The driver data no
longer has to carry the pointer.

Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/power/supply/bq25630_charger.c | 48 ++++++++++++++------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/power/supply/bq25630_charger.c b/drivers/power/supply/bq25630_charger.c
index 165f8c67b489..54c62b7d6514 100644
--- a/drivers/power/supply/bq25630_charger.c
+++ b/drivers/power/supply/bq25630_charger.c
@@ -356,7 +356,6 @@ struct bq25630_data {
 	struct regmap_field *regfields[BQ25630_REGF_MAX];
 
 	struct power_supply *psy;
-	struct power_supply_battery_info *batinfo;
 
 	/* State status from IRQs. */
 	u8 statregs[BQ25630_NR_STAT_REGS];
@@ -668,6 +667,7 @@ static int bq25630_reset(struct bq25630_data *data)
 
 static int bq25630_setup(struct bq25630_data *data)
 {
+	struct power_supply_battery_info *batinfo;
 	int ret;
 
 	ret = bq25630_reset(data);
@@ -684,69 +684,78 @@ static int bq25630_setup(struct bq25630_data *data)
 		return ret;
 	}
 
+	ret = power_supply_get_battery_info(data->psy, &batinfo);
+	if (ret) {
+		dev_err(data->dev, "Could not get battery info (%d)\n", ret);
+		return ret;
+	}
+
 	/*
 	 * Set values according to battery info. Warn on missing "dangerous"
 	 * properties.
 	 */
-	if (data->batinfo->voltage_min_design_uv >= 0) {
+	if (batinfo->voltage_min_design_uv >= 0) {
 		ret = bq25630_write_limit(data, BQ25630_REGF_VSYSMIN,
 					  BQ25630_VSYSMIN_MIN,
 					  BQ25630_VSYSMIN_MAX,
 					  BQ25630_VSYSMIN_STEP,
 					  BQ25630_VSYSMIN_MIN_REGVAL,
-					  data->batinfo->voltage_min_design_uv);
+					  batinfo->voltage_min_design_uv);
 		if (ret)
-			return ret;
+			goto out_put_batinfo;
 	} else
 		dev_warn(data->dev,
 			 "Using default value for minimum voltage\n");
 
-	if (data->batinfo->constant_charge_voltage_max_uv >= 0) {
+	if (batinfo->constant_charge_voltage_max_uv >= 0) {
 		ret = bq25630_write_limit(
 			data, BQ25630_REGF_VREG, BQ25630_VREG_MIN,
 			BQ25630_VREG_MAX, BQ25630_VREG_STEP,
 			BQ25630_VREG_MIN_REGVAL,
-			data->batinfo->constant_charge_voltage_max_uv);
+			batinfo->constant_charge_voltage_max_uv);
 		if (ret)
-			return ret;
+			goto out_put_batinfo;
 	} else
 		dev_warn(data->dev,
 			 "Using default value for maximum constant charge voltage\n");
 
-	if (data->batinfo->constant_charge_current_max_ua >= 0) {
+	if (batinfo->constant_charge_current_max_ua >= 0) {
 		ret = bq25630_write_limit(
 			data, BQ25630_REGF_ICHG, BQ25630_ICHG_MIN,
 			BQ25630_ICHG_MAX, BQ25630_ICHG_STEP,
 			BQ25630_ICHG_MIN_REGVAL,
-			data->batinfo->constant_charge_current_max_ua);
+			batinfo->constant_charge_current_max_ua);
 		if (ret)
-			return ret;
+			goto out_put_batinfo;
 	} else
 		dev_warn(data->dev,
 			 "Using default value for maximum constant charge current\n");
 
-	if (data->batinfo->charge_term_current_ua >= 0) {
+	if (batinfo->charge_term_current_ua >= 0) {
 		ret = bq25630_write_limit(
 			data, BQ25630_REGF_ITERM, BQ25630_ITERM_MIN,
 			BQ25630_ITERM_MAX, BQ25630_ITERM_STEP,
 			BQ25630_ITERM_MIN_REGVAL,
-			data->batinfo->charge_term_current_ua);
+			batinfo->charge_term_current_ua);
 		if (ret)
-			return ret;
+			goto out_put_batinfo;
 	}
 
-	if (data->batinfo->precharge_current_ua >= 0) {
+	if (batinfo->precharge_current_ua >= 0) {
 		ret = bq25630_write_limit(data, BQ25630_REGF_IPRECHG,
 					  BQ25630_IPRECHG_MIN,
 					  BQ25630_IPRECHG_MAX,
 					  BQ25630_IPRECHG_STEP,
 					  BQ25630_IPRECHG_MIN_REGVAL,
-					  data->batinfo->precharge_current_ua);
+					  batinfo->precharge_current_ua);
 		if (ret)
-			return ret;
+			goto out_put_batinfo;
 	}
 
-	return 0;
+out_put_batinfo:
+	power_supply_put_battery_info(data->psy, batinfo);
+
+	return ret;
 }
 
 static int bq25630_charger_get_property(struct power_supply *psy,
@@ -1024,11 +1033,6 @@ static int bq25630_probe(struct i2c_client *client)
 		return dev_err_probe(data->dev, PTR_ERR(data->psy),
 			      "Could not register power supply\n");
 
-	ret = power_supply_get_battery_info(data->psy, &data->batinfo);
-	if (ret)
-		return dev_err_probe(data->dev, ret,
-				     "Could not get battery info\n");
-
 	/*
 	 * Device sends active low 256 µs pulse to report status and fault.
 	 *
-- 
2.25.1

[PATCH v3 2/2] power: supply: bq25630: Initialize hardware before exposing the power supply

[Linmao Li]

bq25630_setup() resets the device, disables the watchdog and programs
the charge limits from the battery information. It runs at the end of
bq25630_probe(), that is after the power supply has been registered, so
the device is already exposed to the system while the hardware still
holds its power-on defaults.

power_supply_desc::init runs during registration, after the driver data
and the fwnode are available and before the device is added. Use it for
bq25630_setup() and drop the explicit call from bq25630_probe().

The callback is passed the power supply, so take the driver data from it
and use it for the battery information as well: data->psy is only
assigned once devm_power_supply_register() returns, which is after the
callback has run.

Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/power/supply/bq25630_charger.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/power/supply/bq25630_charger.c b/drivers/power/supply/bq25630_charger.c
index 54c62b7d6514..200f74f8eab6 100644
--- a/drivers/power/supply/bq25630_charger.c
+++ b/drivers/power/supply/bq25630_charger.c
@@ -665,8 +665,9 @@ static int bq25630_reset(struct bq25630_data *data)
 	return 0;
 }
 
-static int bq25630_setup(struct bq25630_data *data)
+static int bq25630_setup(struct power_supply *psy)
 {
+	struct bq25630_data *data = power_supply_get_drvdata(psy);
 	struct power_supply_battery_info *batinfo;
 	int ret;
 
@@ -684,7 +685,7 @@ static int bq25630_setup(struct bq25630_data *data)
 		return ret;
 	}
 
-	ret = power_supply_get_battery_info(data->psy, &batinfo);
+	ret = power_supply_get_battery_info(psy, &batinfo);
 	if (ret) {
 		dev_err(data->dev, "Could not get battery info (%d)\n", ret);
 		return ret;
@@ -753,7 +754,7 @@ static int bq25630_setup(struct bq25630_data *data)
 	}
 
 out_put_batinfo:
-	power_supply_put_battery_info(data->psy, batinfo);
+	power_supply_put_battery_info(psy, batinfo);
 
 	return ret;
 }
@@ -976,6 +977,7 @@ static const struct power_supply_desc bq25630_charger_psy_desc = {
 	.get_property = bq25630_charger_get_property,
 	.set_property = bq25630_charger_set_property,
 	.property_is_writeable = bq25630_charger_property_is_writeable,
+	.init = bq25630_setup,
 };
 
 static int bq25630_probe(struct i2c_client *client)
@@ -1048,10 +1050,6 @@ static int bq25630_probe(struct i2c_client *client)
 	if (ret)
 		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
 
-	ret = bq25630_setup(data);
-	if (ret)
-		return ret;
-
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH v3 2/2] power: supply: bq25630: Initialize hardware before exposing the power supply

[Waqar Hameed]

On Thu, Jul 30, 2026 at 09:17 +0800 Linmao Li <lilinmao@kylinos.cn> wrote:

> bq25630_setup() resets the device, disables the watchdog and programs
> the charge limits from the battery information. It runs at the end of
> bq25630_probe(), that is after the power supply has been registered, so
> the device is already exposed to the system while the hardware still
> holds its power-on defaults.
>
> power_supply_desc::init runs during registration, after the driver data
> and the fwnode are available and before the device is added. Use it for
> bq25630_setup() and drop the explicit call from bq25630_probe().
>
> The callback is passed the power supply, so take the driver data from it
> and use it for the battery information as well: data->psy is only
> assigned once devm_power_supply_register() returns, which is after the
> callback has run.
>
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>

You didn't include my "Reviewed-by" tag? Anyway, here it is again (there is no diff against previous version):

Reviewed-by: Waqar Hameed <waqar.hameed@axis.com>

Re: [PATCH v3 1/2] power: supply: bq25630: Scope battery information to bq25630_setup()

[Waqar Hameed]

On Thu, Jul 30, 2026 at 09:17 +0800 Linmao Li <lilinmao@kylinos.cn> wrote:

> data->batinfo is only used by bq25630_setup() to program the initial
> charge limits, but power_supply_get_battery_info() allocates it on
> psy->dev, so it stays around for the lifetime of the device. Nothing
> else in the driver uses it.
>
> Get the battery information in bq25630_setup(), just before it is read,
> and release it on every path out of that function. The driver data no
> longer has to carry the pointer.
>
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>

Looks good, thanks!

Reviewed-by: Waqar Hameed <waqar.hameed@axis.com>

[...]

Re: [PATCH v3 0/2] power: supply: bq25630: scope the battery information

[Sebastian Reichel]

On Thu, 30 Jul 2026 09:17:11 +0800, Linmao Li wrote:
> data->batinfo is only read by bq25630_setup(), but it is fetched in
> bq25630_probe() and kept in the driver data for the lifetime of the
> device.
> 
> Patch 1 moves the get/put pair into bq25630_setup() and drops the member.
> Patch 2 then uses the new power_supply_desc::init callback so the charger
> registers are programmed before the device is exposed to the system.
> 
> [...]

Applied, thanks!

[1/2] power: supply: bq25630: Scope battery information to bq25630_setup()
      commit: 9092828e187f7cb3c7346a64e01a10d8f75ca246
[2/2] power: supply: bq25630: Initialize hardware before exposing the power supply
      commit: fdece8642eca8e20a218837f58c0ae6d83fe53a1

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>