Re: [PATCH 2/3] mfd: 88pm886: Initialize battery page

Duje Mihanović, 2026-05-26T21:14:48+02:00:
> From: Duje Mihanović <[email protected]>
>
> Initialize the PMIC's battery page. The battery page registers are
> shared between Vbus regulator, charger, fuelgauge and camera flash
> blocks, hence the commonization of the page.
>
> Signed-off-by: Duje Mihanović <[email protected]>
> ---
>  drivers/mfd/88pm886.c       | 21 ++++++++++++++++++++-
>  include/linux/mfd/88pm886.h |  5 +++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/88pm886.c b/drivers/mfd/88pm886.c
> index e411d8dee554..73102e638eba 100644
> --- a/drivers/mfd/88pm886.c
> +++ b/drivers/mfd/88pm886.c
> @@ -16,6 +16,12 @@ static const struct regmap_config pm886_regmap_config = {
>  	.max_register = PM886_REG_RTC_SPARE6,
>  };
>  
> +static const struct regmap_config pm886_regmap_battery_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = PM886_REG_CLS_CONFIG1,
> +};
> +
>  static const struct regmap_irq pm886_regmap_irqs[] = {
>  	REGMAP_IRQ_REG(PM886_IRQ_ONKEY, 0, PM886_INT_ENA1_ONKEY),
>  };
> @@ -88,7 +94,8 @@ static int pm886_probe(struct i2c_client *client)
>  	struct regmap_irq_chip_data *irq_data;
>  	struct device *dev = &client->dev;
>  	struct pm886_chip *chip;
> -	struct regmap *regmap;
> +	struct regmap *regmap, *regmap_battery;
> +	struct i2c_client *battery_page;

Not sure if this should be reordered to preserve the "reverse Christmas
tree", but maybe it's fine if checkpatch didn't complain.

>  	unsigned int chip_id;
>  	int err;
>  
> @@ -105,6 +112,18 @@ static int pm886_probe(struct i2c_client *client)
>  		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialize regmap\n");
>  	chip->regmap = regmap;
>  
> +	battery_page = devm_i2c_new_dummy_device(dev, client->adapter,
> +						 client->addr + PM886_PAGE_OFFSET_BATTERY);
> +	if (IS_ERR(battery_page))
> +		return dev_err_probe(dev, PTR_ERR(battery_page),
> +				     "Failed to initialize battery page\n");
> +
> +	regmap_battery = devm_regmap_init_i2c(battery_page, &pm886_regmap_battery_config);
> +	if (IS_ERR(regmap_battery))
> +		return dev_err_probe(dev, PTR_ERR(regmap_battery),
> +				     "Failed to initialize battery regmap\n");
> +	chip->regmap_battery = regmap_battery;
> +

Nit: maybe this could go below the chip ID check so that it's not
executed if the check subsequently fails?

Also, I think the error messages are missing either a definite article
or an underscore. Same in the commit message title.

>  	err = regmap_read(regmap, PM886_REG_ID, &chip_id);
>  	if (err)
>  		return dev_err_probe(dev, err, "Failed to read chip ID\n");
> diff --git a/include/linux/mfd/88pm886.h b/include/linux/mfd/88pm886.h
> index 38892ba7b8a4..2c24dd3032ab 100644
> --- a/include/linux/mfd/88pm886.h
> +++ b/include/linux/mfd/88pm886.h
> @@ -11,6 +11,7 @@
>  
>  #define PM886_PAGE_OFFSET_REGULATORS	1
>  #define PM886_PAGE_OFFSET_GPADC		2
> +#define PM886_PAGE_OFFSET_BATTERY	3
>  
>  #define PM886_REG_ID			0x00
>  
> @@ -128,9 +129,13 @@
>  #define PM886_GPADC_BIAS_LEVELS		16
>  #define PM886_GPADC_INDEX_TO_BIAS_uA(i)	(1 + (i) * 5)
>  
> +/* Battery block register definitions */
> +#define PM886_REG_CLS_CONFIG1		0x71
> +
>  struct pm886_chip {
>  	struct i2c_client *client;
>  	unsigned int chip_id;
>  	struct regmap *regmap;
> +	struct regmap *regmap_battery;
>  };
>  #endif /* __MFD_88PM886_H */
>
> -- 
> 2.54.0

Reviewed-by: Karel Balej <[email protected]>