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

[Waqar Hameed]

On Tue, Jul 28, 2026 at 10:45 +0800 Linmao Li <lilinmao@xxxxxxxxxx> 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@xxxxxxxxxx>

Much better! However, `git` complains:

  $ git am /path/to/patch
  error: cannot convert from y to UTF-8
  fatal: could not parse patch

LKML also warns about this [1]. I was able to force UTF-8 and workaround
this though... You might want check your setup.

> ---
>  drivers/power/supply/bq25630_charger.c | 47 ++++++++++++++------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/power/supply/bq25630_charger.c b/drivers/power/supply/bq25630_charger.c
> index 165f8c67b489..9b5f524505d3 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,77 @@ static int bq25630_setup(struct bq25630_data *data)
>  		return ret;
>  	}
>  
> +	ret = power_supply_get_battery_info(data->psy, &batinfo);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "Could not get battery info\n");
> +

`dev_err_probe()` shouldn't be used here. Yes, it is _currently_ only
used from `probe()`, but that shouldn't be an assumption/"policy".
Better to let caller decide on that. I understand that it was just a
pure copy-paste from `probe()`, but we should change it do `dev_error()`
to match the rest of this function.

[...]

[1] https://lore.kernel.org/lkml/20260728024558.3611522-2-lilinmao@xxxxxxxxxx/