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

["Duje Mihanović" <dujemihanovic32@gmail.com>]

On Sunday, 7 June 2026 12:19:21 Central European Summer Time Karel Balej 
wrote:
> Duje Mihanović, 2026-05-26T21:14:48+02:00:

[...]

> > @@ -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.

I opted not to move the declarations around in this case as IMO it would 
present noise in the diff, but on second thought it may have been the better 
choice.

Lee, do you by chance know how to proceed here?

[...]

> > +	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?

Good catch, I'll do that in v2.

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

I agree for the commit message, but adding one to the error messages would IMO 
make them inconsistent with the existing ones.

[...]

Regards,
--
Duje