diff mbox series

power: supply: max1720x: add read support for nvmem

Message ID 20240717183757.3948-1-dima.fedrau@gmail.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series power: supply: max1720x: add read support for nvmem | expand

Commit Message

Dimitri Fedrau July 17, 2024, 6:37 p.m. UTC
ModelGauge m5 and device configuration values are stored in nonvolatile
memory to prevent data loss if the IC loses power. Add read support for
the nonvolatile memory on MAX1720X devices.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---

Based on:
479b6d04964b "power: supply: add support for MAX1720x standalone fuel gauge"
in branch for-next

---
 drivers/power/supply/max1720x_battery.c | 215 ++++++++++++++++++++++--
 1 file changed, 200 insertions(+), 15 deletions(-)

Comments

Sebastian Reichel Aug. 27, 2024, 5:28 p.m. UTC | #1
Hi,

On Wed, Jul 17, 2024 at 08:37:57PM GMT, Dimitri Fedrau wrote:
> ModelGauge m5 and device configuration values are stored in nonvolatile
> memory to prevent data loss if the IC loses power. Add read support for
> the nonvolatile memory on MAX1720X devices.
> 
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> ---
> 
> Based on:
> 479b6d04964b "power: supply: add support for MAX1720x standalone fuel gauge"
> in branch for-next
> 
> ---
>  drivers/power/supply/max1720x_battery.c | 215 ++++++++++++++++++++++--
>  1 file changed, 200 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
> index edc262f0a62f..175f36e83b85 100644
> --- a/drivers/power/supply/max1720x_battery.c
> +++ b/drivers/power/supply/max1720x_battery.c
> @@ -16,7 +16,9 @@
>  #include <asm/unaligned.h>
>  
>  /* Nonvolatile registers */
> +#define MAX1720X_NXTABLE0		0x80
>  #define MAX1720X_NRSENSE		0xCF	/* RSense in 10^-5 Ohm */
> +#define MAX1720X_NDEVICE_NAME4		0xDF
>  
>  /* ModelGauge m5 */
>  #define MAX172XX_STATUS			0x00	/* Status */
> @@ -46,6 +48,8 @@ static const char *const max17205_model = "MAX17205";
>  
>  struct max1720x_device_info {
>  	struct regmap *regmap;
> +	struct regmap *regmap_nv;
> +	struct i2c_client *ancillary;
>  	int rsense;
>  };
>  
> @@ -106,6 +110,134 @@ static const struct regmap_config max1720x_regmap_cfg = {
>  	.cache_type = REGCACHE_RBTREE,
>  };
>  
> +static const struct regmap_range max1720x_nvmem_allow[] = {
> +	regmap_reg_range(MAX1720X_NXTABLE0, MAX1720X_NDEVICE_NAME4),
> +};
> +
> +static const struct regmap_range max1720x_nvmem_deny[] = {
> +	regmap_reg_range(0x00, 0x7F),
> +	regmap_reg_range(0xE0, 0xFF),
> +};
> +
> +static const struct regmap_access_table max1720x_nvmem_regs = {
> +	.yes_ranges	= max1720x_nvmem_allow,
> +	.n_yes_ranges	= ARRAY_SIZE(max1720x_nvmem_allow),
> +	.no_ranges	= max1720x_nvmem_deny,
> +	.n_no_ranges	= ARRAY_SIZE(max1720x_nvmem_deny),
> +};
> +
> +static const struct regmap_config max1720x_nvmem_regmap_cfg = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = MAX1720X_NDEVICE_NAME4,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.rd_table = &max1720x_nvmem_regs,
> +};
> +
> +static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
> +	{ .name = "nXTable0",  .offset = 0,  .bytes = 2, },
> +	{ .name = "nXTable1",  .offset = 2,  .bytes = 2, },
> +	{ .name = "nXTable2",  .offset = 4,  .bytes = 2, },
> +	{ .name = "nXTable3",  .offset = 6,  .bytes = 2, },
> +	{ .name = "nXTable4",  .offset = 8,  .bytes = 2, },
> +	{ .name = "nXTable5",  .offset = 10, .bytes = 2, },
> +	{ .name = "nXTable6",  .offset = 12, .bytes = 2, },
> +	{ .name = "nXTable7",  .offset = 14, .bytes = 2, },
> +	{ .name = "nXTable8",  .offset = 16, .bytes = 2, },
> +	{ .name = "nXTable9",  .offset = 18, .bytes = 2, },
> +	{ .name = "nXTable10", .offset = 20, .bytes = 2, },
> +	{ .name = "nXTable11", .offset = 22, .bytes = 2, },
> +	{ .name = "nUser18C",  .offset = 24, .bytes = 2, },
> +	{ .name = "nUser18D",  .offset = 26, .bytes = 2, },
> +	{ .name = "nODSCTh",   .offset = 28, .bytes = 2, },
> +	{ .name = "nODSCCfg",  .offset = 30, .bytes = 2, },
> +
> +	{ .name = "nOCVTable0",  .offset = 32, .bytes = 2, },
> +	{ .name = "nOCVTable1",  .offset = 34, .bytes = 2, },
> +	{ .name = "nOCVTable2",  .offset = 36, .bytes = 2, },
> +	{ .name = "nOCVTable3",  .offset = 38, .bytes = 2, },
> +	{ .name = "nOCVTable4",  .offset = 40, .bytes = 2, },
> +	{ .name = "nOCVTable5",  .offset = 42, .bytes = 2, },
> +	{ .name = "nOCVTable6",  .offset = 44, .bytes = 2, },
> +	{ .name = "nOCVTable7",  .offset = 46, .bytes = 2, },
> +	{ .name = "nOCVTable8",  .offset = 48, .bytes = 2, },
> +	{ .name = "nOCVTable9",  .offset = 50, .bytes = 2, },
> +	{ .name = "nOCVTable10", .offset = 52, .bytes = 2, },
> +	{ .name = "nOCVTable11", .offset = 54, .bytes = 2, },
> +	{ .name = "nIChgTerm",   .offset = 56, .bytes = 2, },
> +	{ .name = "nFilterCfg",  .offset = 58, .bytes = 2, },
> +	{ .name = "nVEmpty",     .offset = 60, .bytes = 2, },
> +	{ .name = "nLearnCfg",   .offset = 62, .bytes = 2, },
> +
> +	{ .name = "nQRTable00",  .offset = 64, .bytes = 2, },
> +	{ .name = "nQRTable10",  .offset = 66, .bytes = 2, },
> +	{ .name = "nQRTable20",  .offset = 68, .bytes = 2, },
> +	{ .name = "nQRTable30",  .offset = 70, .bytes = 2, },
> +	{ .name = "nCycles",     .offset = 72, .bytes = 2, },
> +	{ .name = "nFullCapNom", .offset = 74, .bytes = 2, },
> +	{ .name = "nRComp0",     .offset = 76, .bytes = 2, },
> +	{ .name = "nTempCo",     .offset = 78, .bytes = 2, },
> +	{ .name = "nIAvgEmpty",  .offset = 80, .bytes = 2, },
> +	{ .name = "nFullCapRep", .offset = 82, .bytes = 2, },
> +	{ .name = "nVoltTemp",   .offset = 84, .bytes = 2, },
> +	{ .name = "nMaxMinCurr", .offset = 86, .bytes = 2, },
> +	{ .name = "nMaxMinVolt", .offset = 88, .bytes = 2, },
> +	{ .name = "nMaxMinTemp", .offset = 90, .bytes = 2, },
> +	{ .name = "nSOC",        .offset = 92, .bytes = 2, },
> +	{ .name = "nTimerH",     .offset = 94, .bytes = 2, },
> +
> +	{ .name = "nConfig",    .offset = 96,  .bytes = 2, },
> +	{ .name = "nRippleCfg", .offset = 98,  .bytes = 2, },
> +	{ .name = "nMiscCfg",   .offset = 100, .bytes = 2, },
> +	{ .name = "nDesignCap", .offset = 102, .bytes = 2, },
> +	{ .name = "nHibCfg",    .offset = 104, .bytes = 2, },
> +	{ .name = "nPackCfg",   .offset = 106, .bytes = 2, },
> +	{ .name = "nRelaxCfg",  .offset = 108, .bytes = 2, },
> +	{ .name = "nConvgCfg",  .offset = 110, .bytes = 2, },
> +	{ .name = "nNVCfg0",    .offset = 112, .bytes = 2, },
> +	{ .name = "nNVCfg1",    .offset = 114, .bytes = 2, },
> +	{ .name = "nNVCfg2",    .offset = 116, .bytes = 2, },
> +	{ .name = "nSBSCfg",    .offset = 118, .bytes = 2, },
> +	{ .name = "nROMID0",    .offset = 120, .bytes = 2, },
> +	{ .name = "nROMID1",    .offset = 122, .bytes = 2, },
> +	{ .name = "nROMID2",    .offset = 124, .bytes = 2, },
> +	{ .name = "nROMID3",    .offset = 126, .bytes = 2, },
> +
> +	{ .name = "nVAlrtTh",      .offset = 128, .bytes = 2, },
> +	{ .name = "nTAlrtTh",      .offset = 130, .bytes = 2, },
> +	{ .name = "nSAlrtTh",      .offset = 132, .bytes = 2, },
> +	{ .name = "nIAlrtTh",      .offset = 134, .bytes = 2, },
> +	{ .name = "nUser1C4",      .offset = 136, .bytes = 2, },
> +	{ .name = "nUser1C5",      .offset = 138, .bytes = 2, },
> +	{ .name = "nFullSOCThr",   .offset = 140, .bytes = 2, },
> +	{ .name = "nTTFCfg",       .offset = 142, .bytes = 2, },
> +	{ .name = "nCGain",        .offset = 144, .bytes = 2, },
> +	{ .name = "nTCurve",       .offset = 146, .bytes = 2, },
> +	{ .name = "nTGain",        .offset = 148, .bytes = 2, },
> +	{ .name = "nTOff",         .offset = 150, .bytes = 2, },
> +	{ .name = "nManfctrName0", .offset = 152, .bytes = 2, },
> +	{ .name = "nManfctrName1", .offset = 154, .bytes = 2, },
> +	{ .name = "nManfctrName2", .offset = 156, .bytes = 2, },
> +	{ .name = "nRSense",       .offset = 158, .bytes = 2, },
> +
> +	{ .name = "nUser1D0",       .offset = 160, .bytes = 2, },
> +	{ .name = "nUser1D1",       .offset = 162, .bytes = 2, },
> +	{ .name = "nAgeFcCfg",      .offset = 164, .bytes = 2, },
> +	{ .name = "nDesignVoltage", .offset = 166, .bytes = 2, },
> +	{ .name = "nUser1D4",       .offset = 168, .bytes = 2, },
> +	{ .name = "nRFastVShdn",    .offset = 170, .bytes = 2, },
> +	{ .name = "nManfctrDate",   .offset = 172, .bytes = 2, },
> +	{ .name = "nFirstUsed",     .offset = 174, .bytes = 2, },
> +	{ .name = "nSerialNumber0", .offset = 176, .bytes = 2, },
> +	{ .name = "nSerialNumber1", .offset = 178, .bytes = 2, },
> +	{ .name = "nSerialNumber2", .offset = 180, .bytes = 2, },
> +	{ .name = "nDeviceName0",   .offset = 182, .bytes = 2, },
> +	{ .name = "nDeviceName1",   .offset = 184, .bytes = 2, },
> +	{ .name = "nDeviceName2",   .offset = 186, .bytes = 2, },
> +	{ .name = "nDeviceName3",   .offset = 188, .bytes = 2, },
> +	{ .name = "nDeviceName4",   .offset = 190, .bytes = 2, },
> +};
> +
>  static const enum power_supply_property max1720x_battery_props[] = {
>  	POWER_SUPPLY_PROP_PRESENT,
>  	POWER_SUPPLY_PROP_CAPACITY,
> @@ -249,31 +381,74 @@ static int max1720x_battery_get_property(struct power_supply *psy,
>  	return ret;
>  }
>  
> -static int max1720x_probe_sense_resistor(struct i2c_client *client,
> -					 struct max1720x_device_info *info)
> +static
> +int max1720x_nvmem_reg_read(void *priv, unsigned int off, void *val, size_t len)
> +{
> +	struct max1720x_device_info *info = priv;
> +	unsigned int reg = MAX1720X_NXTABLE0 + (off / 2);
> +
> +	return regmap_bulk_read(info->regmap_nv, reg, val, len / 2);
> +}
> +
> +static int max1720x_probe_nvmem(struct i2c_client *client,
> +				struct max1720x_device_info *info)
>  {
>  	struct device *dev = &client->dev;
> -	struct i2c_client *ancillary;
> +	struct nvmem_config nvmem_config = {
> +		.dev = dev,
> +		.name = "max1720x_nvmem",
> +		.cells = max1720x_nvmem_cells,
> +		.ncells = ARRAY_SIZE(max1720x_nvmem_cells),
> +		.read_only = true,
> +		.root_only = true,
> +		.reg_read = max1720x_nvmem_reg_read,
> +		.size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
> +		.word_size = 2,
> +		.stride = 2,
> +		.priv = info,
> +	};
> +	struct nvmem_device *nvmem;
> +	unsigned int val;
>  	int ret;
>  
> -	ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
> -	if (IS_ERR(ancillary)) {
> +	info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
> +	if (IS_ERR(info->ancillary)) {
>  		dev_err(dev, "Failed to initialize ancillary i2c device\n");
> -		return PTR_ERR(ancillary);
> +		return PTR_ERR(info->ancillary);
>  	}

Use devm_add_action_or_reset() to unregister info->ancillary. It
results in cleaner code and also fixes a race condition during
module remove, which frees info->ancillary before the regmap with
your current patch.

Otherwise LGTM.

Greetings,

-- Sebastian

>  
> -	ret = i2c_smbus_read_word_data(ancillary, MAX1720X_NRSENSE);
> -	i2c_unregister_device(ancillary);
> -	if (ret < 0)
> -		return ret;
> +	info->regmap_nv = devm_regmap_init_i2c(info->ancillary,
> +					       &max1720x_nvmem_regmap_cfg);
> +	if (IS_ERR(info->regmap_nv)) {
> +		dev_err(dev, "regmap initialization of nvmem failed\n");
> +		ret = PTR_ERR(info->regmap_nv);
> +		goto err;
> +	}
> +
> +	ret = regmap_read(info->regmap_nv, MAX1720X_NRSENSE, &val);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to read sense resistor value\n");
> +		goto err;
> +	}
>  
> -	info->rsense = ret;
> +	info->rsense = val;
>  	if (!info->rsense) {
>  		dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n");
>  		info->rsense = 1000; /* in regs in 10^-5 */
>  	}
>  
> +	nvmem = devm_nvmem_register(dev, &nvmem_config);
> +	if (IS_ERR(nvmem)) {
> +		dev_err(dev, "Could not register nvmem!");
> +		ret = PTR_ERR(nvmem);
> +		goto err;
> +	}
> +
>  	return 0;
> +err:
> +	i2c_unregister_device(info->ancillary);
> +
> +	return ret;
>  }
>  
>  static const struct power_supply_desc max1720x_bat_desc = {
> @@ -299,24 +474,33 @@ static int max1720x_probe(struct i2c_client *client)
>  
>  	psy_cfg.drv_data = info;
>  	psy_cfg.fwnode = dev_fwnode(dev);
> +	i2c_set_clientdata(client, info);
>  	info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg);
>  	if (IS_ERR(info->regmap))
>  		return dev_err_probe(dev, PTR_ERR(info->regmap),
>  				     "regmap initialization failed\n");
>  
> -	ret = max1720x_probe_sense_resistor(client, info);
> +	ret = max1720x_probe_nvmem(client, info);
>  	if (ret)
> -		return dev_err_probe(dev, ret,
> -				     "Failed to read sense resistor value\n");
> +		return dev_err_probe(dev, ret, "Failed to probe nvmem\n");
>  
>  	bat = devm_power_supply_register(dev, &max1720x_bat_desc, &psy_cfg);
> -	if (IS_ERR(bat))
> +	if (IS_ERR(bat)) {
> +		i2c_unregister_device(info->ancillary);
>  		return dev_err_probe(dev, PTR_ERR(bat),
>  				     "Failed to register power supply\n");
> +	}
>  
>  	return 0;
>  }
>  
> +static void max1720x_remove(struct i2c_client *client)
> +{
> +	struct max1720x_device_info *info = i2c_get_clientdata(client);
> +
> +	i2c_unregister_device(info->ancillary);
> +}
> +
>  static const struct of_device_id max1720x_of_match[] = {
>  	{ .compatible = "maxim,max17201" },
>  	{}
> @@ -329,6 +513,7 @@ static struct i2c_driver max1720x_i2c_driver = {
>  		.of_match_table = max1720x_of_match,
>  	},
>  	.probe = max1720x_probe,
> +	.remove = max1720x_remove,
>  };
>  module_i2c_driver(max1720x_i2c_driver);
>  
> -- 
> 2.39.2
>
Dimitri Fedrau Aug. 31, 2024, 6:15 p.m. UTC | #2
Am Tue, Aug 27, 2024 at 07:28:43PM +0200 schrieb Sebastian Reichel:
> Hi,
> 
> On Wed, Jul 17, 2024 at 08:37:57PM GMT, Dimitri Fedrau wrote:
> > ModelGauge m5 and device configuration values are stored in nonvolatile
> > memory to prevent data loss if the IC loses power. Add read support for
> > the nonvolatile memory on MAX1720X devices.
> > 
> > Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> > ---
> > 
> > Based on:
> > 479b6d04964b "power: supply: add support for MAX1720x standalone fuel gauge"
> > in branch for-next
> > 
> > ---
> >  drivers/power/supply/max1720x_battery.c | 215 ++++++++++++++++++++++--
> >  1 file changed, 200 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
> > index edc262f0a62f..175f36e83b85 100644
> > --- a/drivers/power/supply/max1720x_battery.c
> > +++ b/drivers/power/supply/max1720x_battery.c
> > @@ -16,7 +16,9 @@
> >  #include <asm/unaligned.h>
> >  
> >  /* Nonvolatile registers */
> > +#define MAX1720X_NXTABLE0		0x80
> >  #define MAX1720X_NRSENSE		0xCF	/* RSense in 10^-5 Ohm */
> > +#define MAX1720X_NDEVICE_NAME4		0xDF
> >  
> >  /* ModelGauge m5 */
> >  #define MAX172XX_STATUS			0x00	/* Status */
> > @@ -46,6 +48,8 @@ static const char *const max17205_model = "MAX17205";
> >  
> >  struct max1720x_device_info {
> >  	struct regmap *regmap;
> > +	struct regmap *regmap_nv;
> > +	struct i2c_client *ancillary;
> >  	int rsense;
> >  };
> >  
> > @@ -106,6 +110,134 @@ static const struct regmap_config max1720x_regmap_cfg = {
> >  	.cache_type = REGCACHE_RBTREE,
> >  };
> >  
> > +static const struct regmap_range max1720x_nvmem_allow[] = {
> > +	regmap_reg_range(MAX1720X_NXTABLE0, MAX1720X_NDEVICE_NAME4),
> > +};
> > +
> > +static const struct regmap_range max1720x_nvmem_deny[] = {
> > +	regmap_reg_range(0x00, 0x7F),
> > +	regmap_reg_range(0xE0, 0xFF),
> > +};
> > +
> > +static const struct regmap_access_table max1720x_nvmem_regs = {
> > +	.yes_ranges	= max1720x_nvmem_allow,
> > +	.n_yes_ranges	= ARRAY_SIZE(max1720x_nvmem_allow),
> > +	.no_ranges	= max1720x_nvmem_deny,
> > +	.n_no_ranges	= ARRAY_SIZE(max1720x_nvmem_deny),
> > +};
> > +
> > +static const struct regmap_config max1720x_nvmem_regmap_cfg = {
> > +	.reg_bits = 8,
> > +	.val_bits = 16,
> > +	.max_register = MAX1720X_NDEVICE_NAME4,
> > +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> > +	.rd_table = &max1720x_nvmem_regs,
> > +};
> > +
> > +static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
> > +	{ .name = "nXTable0",  .offset = 0,  .bytes = 2, },
> > +	{ .name = "nXTable1",  .offset = 2,  .bytes = 2, },
> > +	{ .name = "nXTable2",  .offset = 4,  .bytes = 2, },
> > +	{ .name = "nXTable3",  .offset = 6,  .bytes = 2, },
> > +	{ .name = "nXTable4",  .offset = 8,  .bytes = 2, },
> > +	{ .name = "nXTable5",  .offset = 10, .bytes = 2, },
> > +	{ .name = "nXTable6",  .offset = 12, .bytes = 2, },
> > +	{ .name = "nXTable7",  .offset = 14, .bytes = 2, },
> > +	{ .name = "nXTable8",  .offset = 16, .bytes = 2, },
> > +	{ .name = "nXTable9",  .offset = 18, .bytes = 2, },
> > +	{ .name = "nXTable10", .offset = 20, .bytes = 2, },
> > +	{ .name = "nXTable11", .offset = 22, .bytes = 2, },
> > +	{ .name = "nUser18C",  .offset = 24, .bytes = 2, },
> > +	{ .name = "nUser18D",  .offset = 26, .bytes = 2, },
> > +	{ .name = "nODSCTh",   .offset = 28, .bytes = 2, },
> > +	{ .name = "nODSCCfg",  .offset = 30, .bytes = 2, },
> > +
> > +	{ .name = "nOCVTable0",  .offset = 32, .bytes = 2, },
> > +	{ .name = "nOCVTable1",  .offset = 34, .bytes = 2, },
> > +	{ .name = "nOCVTable2",  .offset = 36, .bytes = 2, },
> > +	{ .name = "nOCVTable3",  .offset = 38, .bytes = 2, },
> > +	{ .name = "nOCVTable4",  .offset = 40, .bytes = 2, },
> > +	{ .name = "nOCVTable5",  .offset = 42, .bytes = 2, },
> > +	{ .name = "nOCVTable6",  .offset = 44, .bytes = 2, },
> > +	{ .name = "nOCVTable7",  .offset = 46, .bytes = 2, },
> > +	{ .name = "nOCVTable8",  .offset = 48, .bytes = 2, },
> > +	{ .name = "nOCVTable9",  .offset = 50, .bytes = 2, },
> > +	{ .name = "nOCVTable10", .offset = 52, .bytes = 2, },
> > +	{ .name = "nOCVTable11", .offset = 54, .bytes = 2, },
> > +	{ .name = "nIChgTerm",   .offset = 56, .bytes = 2, },
> > +	{ .name = "nFilterCfg",  .offset = 58, .bytes = 2, },
> > +	{ .name = "nVEmpty",     .offset = 60, .bytes = 2, },
> > +	{ .name = "nLearnCfg",   .offset = 62, .bytes = 2, },
> > +
> > +	{ .name = "nQRTable00",  .offset = 64, .bytes = 2, },
> > +	{ .name = "nQRTable10",  .offset = 66, .bytes = 2, },
> > +	{ .name = "nQRTable20",  .offset = 68, .bytes = 2, },
> > +	{ .name = "nQRTable30",  .offset = 70, .bytes = 2, },
> > +	{ .name = "nCycles",     .offset = 72, .bytes = 2, },
> > +	{ .name = "nFullCapNom", .offset = 74, .bytes = 2, },
> > +	{ .name = "nRComp0",     .offset = 76, .bytes = 2, },
> > +	{ .name = "nTempCo",     .offset = 78, .bytes = 2, },
> > +	{ .name = "nIAvgEmpty",  .offset = 80, .bytes = 2, },
> > +	{ .name = "nFullCapRep", .offset = 82, .bytes = 2, },
> > +	{ .name = "nVoltTemp",   .offset = 84, .bytes = 2, },
> > +	{ .name = "nMaxMinCurr", .offset = 86, .bytes = 2, },
> > +	{ .name = "nMaxMinVolt", .offset = 88, .bytes = 2, },
> > +	{ .name = "nMaxMinTemp", .offset = 90, .bytes = 2, },
> > +	{ .name = "nSOC",        .offset = 92, .bytes = 2, },
> > +	{ .name = "nTimerH",     .offset = 94, .bytes = 2, },
> > +
> > +	{ .name = "nConfig",    .offset = 96,  .bytes = 2, },
> > +	{ .name = "nRippleCfg", .offset = 98,  .bytes = 2, },
> > +	{ .name = "nMiscCfg",   .offset = 100, .bytes = 2, },
> > +	{ .name = "nDesignCap", .offset = 102, .bytes = 2, },
> > +	{ .name = "nHibCfg",    .offset = 104, .bytes = 2, },
> > +	{ .name = "nPackCfg",   .offset = 106, .bytes = 2, },
> > +	{ .name = "nRelaxCfg",  .offset = 108, .bytes = 2, },
> > +	{ .name = "nConvgCfg",  .offset = 110, .bytes = 2, },
> > +	{ .name = "nNVCfg0",    .offset = 112, .bytes = 2, },
> > +	{ .name = "nNVCfg1",    .offset = 114, .bytes = 2, },
> > +	{ .name = "nNVCfg2",    .offset = 116, .bytes = 2, },
> > +	{ .name = "nSBSCfg",    .offset = 118, .bytes = 2, },
> > +	{ .name = "nROMID0",    .offset = 120, .bytes = 2, },
> > +	{ .name = "nROMID1",    .offset = 122, .bytes = 2, },
> > +	{ .name = "nROMID2",    .offset = 124, .bytes = 2, },
> > +	{ .name = "nROMID3",    .offset = 126, .bytes = 2, },
> > +
> > +	{ .name = "nVAlrtTh",      .offset = 128, .bytes = 2, },
> > +	{ .name = "nTAlrtTh",      .offset = 130, .bytes = 2, },
> > +	{ .name = "nSAlrtTh",      .offset = 132, .bytes = 2, },
> > +	{ .name = "nIAlrtTh",      .offset = 134, .bytes = 2, },
> > +	{ .name = "nUser1C4",      .offset = 136, .bytes = 2, },
> > +	{ .name = "nUser1C5",      .offset = 138, .bytes = 2, },
> > +	{ .name = "nFullSOCThr",   .offset = 140, .bytes = 2, },
> > +	{ .name = "nTTFCfg",       .offset = 142, .bytes = 2, },
> > +	{ .name = "nCGain",        .offset = 144, .bytes = 2, },
> > +	{ .name = "nTCurve",       .offset = 146, .bytes = 2, },
> > +	{ .name = "nTGain",        .offset = 148, .bytes = 2, },
> > +	{ .name = "nTOff",         .offset = 150, .bytes = 2, },
> > +	{ .name = "nManfctrName0", .offset = 152, .bytes = 2, },
> > +	{ .name = "nManfctrName1", .offset = 154, .bytes = 2, },
> > +	{ .name = "nManfctrName2", .offset = 156, .bytes = 2, },
> > +	{ .name = "nRSense",       .offset = 158, .bytes = 2, },
> > +
> > +	{ .name = "nUser1D0",       .offset = 160, .bytes = 2, },
> > +	{ .name = "nUser1D1",       .offset = 162, .bytes = 2, },
> > +	{ .name = "nAgeFcCfg",      .offset = 164, .bytes = 2, },
> > +	{ .name = "nDesignVoltage", .offset = 166, .bytes = 2, },
> > +	{ .name = "nUser1D4",       .offset = 168, .bytes = 2, },
> > +	{ .name = "nRFastVShdn",    .offset = 170, .bytes = 2, },
> > +	{ .name = "nManfctrDate",   .offset = 172, .bytes = 2, },
> > +	{ .name = "nFirstUsed",     .offset = 174, .bytes = 2, },
> > +	{ .name = "nSerialNumber0", .offset = 176, .bytes = 2, },
> > +	{ .name = "nSerialNumber1", .offset = 178, .bytes = 2, },
> > +	{ .name = "nSerialNumber2", .offset = 180, .bytes = 2, },
> > +	{ .name = "nDeviceName0",   .offset = 182, .bytes = 2, },
> > +	{ .name = "nDeviceName1",   .offset = 184, .bytes = 2, },
> > +	{ .name = "nDeviceName2",   .offset = 186, .bytes = 2, },
> > +	{ .name = "nDeviceName3",   .offset = 188, .bytes = 2, },
> > +	{ .name = "nDeviceName4",   .offset = 190, .bytes = 2, },
> > +};
> > +
> >  static const enum power_supply_property max1720x_battery_props[] = {
> >  	POWER_SUPPLY_PROP_PRESENT,
> >  	POWER_SUPPLY_PROP_CAPACITY,
> > @@ -249,31 +381,74 @@ static int max1720x_battery_get_property(struct power_supply *psy,
> >  	return ret;
> >  }
> >  
> > -static int max1720x_probe_sense_resistor(struct i2c_client *client,
> > -					 struct max1720x_device_info *info)
> > +static
> > +int max1720x_nvmem_reg_read(void *priv, unsigned int off, void *val, size_t len)
> > +{
> > +	struct max1720x_device_info *info = priv;
> > +	unsigned int reg = MAX1720X_NXTABLE0 + (off / 2);
> > +
> > +	return regmap_bulk_read(info->regmap_nv, reg, val, len / 2);
> > +}
> > +
> > +static int max1720x_probe_nvmem(struct i2c_client *client,
> > +				struct max1720x_device_info *info)
> >  {
> >  	struct device *dev = &client->dev;
> > -	struct i2c_client *ancillary;
> > +	struct nvmem_config nvmem_config = {
> > +		.dev = dev,
> > +		.name = "max1720x_nvmem",
> > +		.cells = max1720x_nvmem_cells,
> > +		.ncells = ARRAY_SIZE(max1720x_nvmem_cells),
> > +		.read_only = true,
> > +		.root_only = true,
> > +		.reg_read = max1720x_nvmem_reg_read,
> > +		.size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
> > +		.word_size = 2,
> > +		.stride = 2,
> > +		.priv = info,
> > +	};
> > +	struct nvmem_device *nvmem;
> > +	unsigned int val;
> >  	int ret;
> >  
> > -	ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
> > -	if (IS_ERR(ancillary)) {
> > +	info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
> > +	if (IS_ERR(info->ancillary)) {
> >  		dev_err(dev, "Failed to initialize ancillary i2c device\n");
> > -		return PTR_ERR(ancillary);
> > +		return PTR_ERR(info->ancillary);
> >  	}
> 
> Use devm_add_action_or_reset() to unregister info->ancillary. It
> results in cleaner code and also fixes a race condition during
> module remove, which frees info->ancillary before the regmap with
> your current patch.
>
> Otherwise LGTM.
>

Hi Sebastian,

thanks for pointing out, didn't know about the race condition. Will fix
it in V2.

Best regards,

Dimitri

> Greetings,
> 
> -- Sebastian
> 
> >  
> > -	ret = i2c_smbus_read_word_data(ancillary, MAX1720X_NRSENSE);
> > -	i2c_unregister_device(ancillary);
> > -	if (ret < 0)
> > -		return ret;
> > +	info->regmap_nv = devm_regmap_init_i2c(info->ancillary,
> > +					       &max1720x_nvmem_regmap_cfg);
> > +	if (IS_ERR(info->regmap_nv)) {
> > +		dev_err(dev, "regmap initialization of nvmem failed\n");
> > +		ret = PTR_ERR(info->regmap_nv);
> > +		goto err;
> > +	}
> > +
> > +	ret = regmap_read(info->regmap_nv, MAX1720X_NRSENSE, &val);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Failed to read sense resistor value\n");
> > +		goto err;
> > +	}
> >  
> > -	info->rsense = ret;
> > +	info->rsense = val;
> >  	if (!info->rsense) {
> >  		dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n");
> >  		info->rsense = 1000; /* in regs in 10^-5 */
> >  	}
> >  
> > +	nvmem = devm_nvmem_register(dev, &nvmem_config);
> > +	if (IS_ERR(nvmem)) {
> > +		dev_err(dev, "Could not register nvmem!");
> > +		ret = PTR_ERR(nvmem);
> > +		goto err;
> > +	}
> > +
> >  	return 0;
> > +err:
> > +	i2c_unregister_device(info->ancillary);
> > +
> > +	return ret;
> >  }
> >  
> >  static const struct power_supply_desc max1720x_bat_desc = {
> > @@ -299,24 +474,33 @@ static int max1720x_probe(struct i2c_client *client)
> >  
> >  	psy_cfg.drv_data = info;
> >  	psy_cfg.fwnode = dev_fwnode(dev);
> > +	i2c_set_clientdata(client, info);
> >  	info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg);
> >  	if (IS_ERR(info->regmap))
> >  		return dev_err_probe(dev, PTR_ERR(info->regmap),
> >  				     "regmap initialization failed\n");
> >  
> > -	ret = max1720x_probe_sense_resistor(client, info);
> > +	ret = max1720x_probe_nvmem(client, info);
> >  	if (ret)
> > -		return dev_err_probe(dev, ret,
> > -				     "Failed to read sense resistor value\n");
> > +		return dev_err_probe(dev, ret, "Failed to probe nvmem\n");
> >  
> >  	bat = devm_power_supply_register(dev, &max1720x_bat_desc, &psy_cfg);
> > -	if (IS_ERR(bat))
> > +	if (IS_ERR(bat)) {
> > +		i2c_unregister_device(info->ancillary);
> >  		return dev_err_probe(dev, PTR_ERR(bat),
> >  				     "Failed to register power supply\n");
> > +	}
> >  
> >  	return 0;
> >  }
> >  
> > +static void max1720x_remove(struct i2c_client *client)
> > +{
> > +	struct max1720x_device_info *info = i2c_get_clientdata(client);
> > +
> > +	i2c_unregister_device(info->ancillary);
> > +}
> > +
> >  static const struct of_device_id max1720x_of_match[] = {
> >  	{ .compatible = "maxim,max17201" },
> >  	{}
> > @@ -329,6 +513,7 @@ static struct i2c_driver max1720x_i2c_driver = {
> >  		.of_match_table = max1720x_of_match,
> >  	},
> >  	.probe = max1720x_probe,
> > +	.remove = max1720x_remove,
> >  };
> >  module_i2c_driver(max1720x_i2c_driver);
> >  
> > -- 
> > 2.39.2
> >
diff mbox series

Patch

diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
index edc262f0a62f..175f36e83b85 100644
--- a/drivers/power/supply/max1720x_battery.c
+++ b/drivers/power/supply/max1720x_battery.c
@@ -16,7 +16,9 @@ 
 #include <asm/unaligned.h>
 
 /* Nonvolatile registers */
+#define MAX1720X_NXTABLE0		0x80
 #define MAX1720X_NRSENSE		0xCF	/* RSense in 10^-5 Ohm */
+#define MAX1720X_NDEVICE_NAME4		0xDF
 
 /* ModelGauge m5 */
 #define MAX172XX_STATUS			0x00	/* Status */
@@ -46,6 +48,8 @@  static const char *const max17205_model = "MAX17205";
 
 struct max1720x_device_info {
 	struct regmap *regmap;
+	struct regmap *regmap_nv;
+	struct i2c_client *ancillary;
 	int rsense;
 };
 
@@ -106,6 +110,134 @@  static const struct regmap_config max1720x_regmap_cfg = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
+static const struct regmap_range max1720x_nvmem_allow[] = {
+	regmap_reg_range(MAX1720X_NXTABLE0, MAX1720X_NDEVICE_NAME4),
+};
+
+static const struct regmap_range max1720x_nvmem_deny[] = {
+	regmap_reg_range(0x00, 0x7F),
+	regmap_reg_range(0xE0, 0xFF),
+};
+
+static const struct regmap_access_table max1720x_nvmem_regs = {
+	.yes_ranges	= max1720x_nvmem_allow,
+	.n_yes_ranges	= ARRAY_SIZE(max1720x_nvmem_allow),
+	.no_ranges	= max1720x_nvmem_deny,
+	.n_no_ranges	= ARRAY_SIZE(max1720x_nvmem_deny),
+};
+
+static const struct regmap_config max1720x_nvmem_regmap_cfg = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = MAX1720X_NDEVICE_NAME4,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.rd_table = &max1720x_nvmem_regs,
+};
+
+static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
+	{ .name = "nXTable0",  .offset = 0,  .bytes = 2, },
+	{ .name = "nXTable1",  .offset = 2,  .bytes = 2, },
+	{ .name = "nXTable2",  .offset = 4,  .bytes = 2, },
+	{ .name = "nXTable3",  .offset = 6,  .bytes = 2, },
+	{ .name = "nXTable4",  .offset = 8,  .bytes = 2, },
+	{ .name = "nXTable5",  .offset = 10, .bytes = 2, },
+	{ .name = "nXTable6",  .offset = 12, .bytes = 2, },
+	{ .name = "nXTable7",  .offset = 14, .bytes = 2, },
+	{ .name = "nXTable8",  .offset = 16, .bytes = 2, },
+	{ .name = "nXTable9",  .offset = 18, .bytes = 2, },
+	{ .name = "nXTable10", .offset = 20, .bytes = 2, },
+	{ .name = "nXTable11", .offset = 22, .bytes = 2, },
+	{ .name = "nUser18C",  .offset = 24, .bytes = 2, },
+	{ .name = "nUser18D",  .offset = 26, .bytes = 2, },
+	{ .name = "nODSCTh",   .offset = 28, .bytes = 2, },
+	{ .name = "nODSCCfg",  .offset = 30, .bytes = 2, },
+
+	{ .name = "nOCVTable0",  .offset = 32, .bytes = 2, },
+	{ .name = "nOCVTable1",  .offset = 34, .bytes = 2, },
+	{ .name = "nOCVTable2",  .offset = 36, .bytes = 2, },
+	{ .name = "nOCVTable3",  .offset = 38, .bytes = 2, },
+	{ .name = "nOCVTable4",  .offset = 40, .bytes = 2, },
+	{ .name = "nOCVTable5",  .offset = 42, .bytes = 2, },
+	{ .name = "nOCVTable6",  .offset = 44, .bytes = 2, },
+	{ .name = "nOCVTable7",  .offset = 46, .bytes = 2, },
+	{ .name = "nOCVTable8",  .offset = 48, .bytes = 2, },
+	{ .name = "nOCVTable9",  .offset = 50, .bytes = 2, },
+	{ .name = "nOCVTable10", .offset = 52, .bytes = 2, },
+	{ .name = "nOCVTable11", .offset = 54, .bytes = 2, },
+	{ .name = "nIChgTerm",   .offset = 56, .bytes = 2, },
+	{ .name = "nFilterCfg",  .offset = 58, .bytes = 2, },
+	{ .name = "nVEmpty",     .offset = 60, .bytes = 2, },
+	{ .name = "nLearnCfg",   .offset = 62, .bytes = 2, },
+
+	{ .name = "nQRTable00",  .offset = 64, .bytes = 2, },
+	{ .name = "nQRTable10",  .offset = 66, .bytes = 2, },
+	{ .name = "nQRTable20",  .offset = 68, .bytes = 2, },
+	{ .name = "nQRTable30",  .offset = 70, .bytes = 2, },
+	{ .name = "nCycles",     .offset = 72, .bytes = 2, },
+	{ .name = "nFullCapNom", .offset = 74, .bytes = 2, },
+	{ .name = "nRComp0",     .offset = 76, .bytes = 2, },
+	{ .name = "nTempCo",     .offset = 78, .bytes = 2, },
+	{ .name = "nIAvgEmpty",  .offset = 80, .bytes = 2, },
+	{ .name = "nFullCapRep", .offset = 82, .bytes = 2, },
+	{ .name = "nVoltTemp",   .offset = 84, .bytes = 2, },
+	{ .name = "nMaxMinCurr", .offset = 86, .bytes = 2, },
+	{ .name = "nMaxMinVolt", .offset = 88, .bytes = 2, },
+	{ .name = "nMaxMinTemp", .offset = 90, .bytes = 2, },
+	{ .name = "nSOC",        .offset = 92, .bytes = 2, },
+	{ .name = "nTimerH",     .offset = 94, .bytes = 2, },
+
+	{ .name = "nConfig",    .offset = 96,  .bytes = 2, },
+	{ .name = "nRippleCfg", .offset = 98,  .bytes = 2, },
+	{ .name = "nMiscCfg",   .offset = 100, .bytes = 2, },
+	{ .name = "nDesignCap", .offset = 102, .bytes = 2, },
+	{ .name = "nHibCfg",    .offset = 104, .bytes = 2, },
+	{ .name = "nPackCfg",   .offset = 106, .bytes = 2, },
+	{ .name = "nRelaxCfg",  .offset = 108, .bytes = 2, },
+	{ .name = "nConvgCfg",  .offset = 110, .bytes = 2, },
+	{ .name = "nNVCfg0",    .offset = 112, .bytes = 2, },
+	{ .name = "nNVCfg1",    .offset = 114, .bytes = 2, },
+	{ .name = "nNVCfg2",    .offset = 116, .bytes = 2, },
+	{ .name = "nSBSCfg",    .offset = 118, .bytes = 2, },
+	{ .name = "nROMID0",    .offset = 120, .bytes = 2, },
+	{ .name = "nROMID1",    .offset = 122, .bytes = 2, },
+	{ .name = "nROMID2",    .offset = 124, .bytes = 2, },
+	{ .name = "nROMID3",    .offset = 126, .bytes = 2, },
+
+	{ .name = "nVAlrtTh",      .offset = 128, .bytes = 2, },
+	{ .name = "nTAlrtTh",      .offset = 130, .bytes = 2, },
+	{ .name = "nSAlrtTh",      .offset = 132, .bytes = 2, },
+	{ .name = "nIAlrtTh",      .offset = 134, .bytes = 2, },
+	{ .name = "nUser1C4",      .offset = 136, .bytes = 2, },
+	{ .name = "nUser1C5",      .offset = 138, .bytes = 2, },
+	{ .name = "nFullSOCThr",   .offset = 140, .bytes = 2, },
+	{ .name = "nTTFCfg",       .offset = 142, .bytes = 2, },
+	{ .name = "nCGain",        .offset = 144, .bytes = 2, },
+	{ .name = "nTCurve",       .offset = 146, .bytes = 2, },
+	{ .name = "nTGain",        .offset = 148, .bytes = 2, },
+	{ .name = "nTOff",         .offset = 150, .bytes = 2, },
+	{ .name = "nManfctrName0", .offset = 152, .bytes = 2, },
+	{ .name = "nManfctrName1", .offset = 154, .bytes = 2, },
+	{ .name = "nManfctrName2", .offset = 156, .bytes = 2, },
+	{ .name = "nRSense",       .offset = 158, .bytes = 2, },
+
+	{ .name = "nUser1D0",       .offset = 160, .bytes = 2, },
+	{ .name = "nUser1D1",       .offset = 162, .bytes = 2, },
+	{ .name = "nAgeFcCfg",      .offset = 164, .bytes = 2, },
+	{ .name = "nDesignVoltage", .offset = 166, .bytes = 2, },
+	{ .name = "nUser1D4",       .offset = 168, .bytes = 2, },
+	{ .name = "nRFastVShdn",    .offset = 170, .bytes = 2, },
+	{ .name = "nManfctrDate",   .offset = 172, .bytes = 2, },
+	{ .name = "nFirstUsed",     .offset = 174, .bytes = 2, },
+	{ .name = "nSerialNumber0", .offset = 176, .bytes = 2, },
+	{ .name = "nSerialNumber1", .offset = 178, .bytes = 2, },
+	{ .name = "nSerialNumber2", .offset = 180, .bytes = 2, },
+	{ .name = "nDeviceName0",   .offset = 182, .bytes = 2, },
+	{ .name = "nDeviceName1",   .offset = 184, .bytes = 2, },
+	{ .name = "nDeviceName2",   .offset = 186, .bytes = 2, },
+	{ .name = "nDeviceName3",   .offset = 188, .bytes = 2, },
+	{ .name = "nDeviceName4",   .offset = 190, .bytes = 2, },
+};
+
 static const enum power_supply_property max1720x_battery_props[] = {
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_CAPACITY,
@@ -249,31 +381,74 @@  static int max1720x_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
-static int max1720x_probe_sense_resistor(struct i2c_client *client,
-					 struct max1720x_device_info *info)
+static
+int max1720x_nvmem_reg_read(void *priv, unsigned int off, void *val, size_t len)
+{
+	struct max1720x_device_info *info = priv;
+	unsigned int reg = MAX1720X_NXTABLE0 + (off / 2);
+
+	return regmap_bulk_read(info->regmap_nv, reg, val, len / 2);
+}
+
+static int max1720x_probe_nvmem(struct i2c_client *client,
+				struct max1720x_device_info *info)
 {
 	struct device *dev = &client->dev;
-	struct i2c_client *ancillary;
+	struct nvmem_config nvmem_config = {
+		.dev = dev,
+		.name = "max1720x_nvmem",
+		.cells = max1720x_nvmem_cells,
+		.ncells = ARRAY_SIZE(max1720x_nvmem_cells),
+		.read_only = true,
+		.root_only = true,
+		.reg_read = max1720x_nvmem_reg_read,
+		.size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
+		.word_size = 2,
+		.stride = 2,
+		.priv = info,
+	};
+	struct nvmem_device *nvmem;
+	unsigned int val;
 	int ret;
 
-	ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
-	if (IS_ERR(ancillary)) {
+	info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
+	if (IS_ERR(info->ancillary)) {
 		dev_err(dev, "Failed to initialize ancillary i2c device\n");
-		return PTR_ERR(ancillary);
+		return PTR_ERR(info->ancillary);
 	}
 
-	ret = i2c_smbus_read_word_data(ancillary, MAX1720X_NRSENSE);
-	i2c_unregister_device(ancillary);
-	if (ret < 0)
-		return ret;
+	info->regmap_nv = devm_regmap_init_i2c(info->ancillary,
+					       &max1720x_nvmem_regmap_cfg);
+	if (IS_ERR(info->regmap_nv)) {
+		dev_err(dev, "regmap initialization of nvmem failed\n");
+		ret = PTR_ERR(info->regmap_nv);
+		goto err;
+	}
+
+	ret = regmap_read(info->regmap_nv, MAX1720X_NRSENSE, &val);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read sense resistor value\n");
+		goto err;
+	}
 
-	info->rsense = ret;
+	info->rsense = val;
 	if (!info->rsense) {
 		dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n");
 		info->rsense = 1000; /* in regs in 10^-5 */
 	}
 
+	nvmem = devm_nvmem_register(dev, &nvmem_config);
+	if (IS_ERR(nvmem)) {
+		dev_err(dev, "Could not register nvmem!");
+		ret = PTR_ERR(nvmem);
+		goto err;
+	}
+
 	return 0;
+err:
+	i2c_unregister_device(info->ancillary);
+
+	return ret;
 }
 
 static const struct power_supply_desc max1720x_bat_desc = {
@@ -299,24 +474,33 @@  static int max1720x_probe(struct i2c_client *client)
 
 	psy_cfg.drv_data = info;
 	psy_cfg.fwnode = dev_fwnode(dev);
+	i2c_set_clientdata(client, info);
 	info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg);
 	if (IS_ERR(info->regmap))
 		return dev_err_probe(dev, PTR_ERR(info->regmap),
 				     "regmap initialization failed\n");
 
-	ret = max1720x_probe_sense_resistor(client, info);
+	ret = max1720x_probe_nvmem(client, info);
 	if (ret)
-		return dev_err_probe(dev, ret,
-				     "Failed to read sense resistor value\n");
+		return dev_err_probe(dev, ret, "Failed to probe nvmem\n");
 
 	bat = devm_power_supply_register(dev, &max1720x_bat_desc, &psy_cfg);
-	if (IS_ERR(bat))
+	if (IS_ERR(bat)) {
+		i2c_unregister_device(info->ancillary);
 		return dev_err_probe(dev, PTR_ERR(bat),
 				     "Failed to register power supply\n");
+	}
 
 	return 0;
 }
 
+static void max1720x_remove(struct i2c_client *client)
+{
+	struct max1720x_device_info *info = i2c_get_clientdata(client);
+
+	i2c_unregister_device(info->ancillary);
+}
+
 static const struct of_device_id max1720x_of_match[] = {
 	{ .compatible = "maxim,max17201" },
 	{}
@@ -329,6 +513,7 @@  static struct i2c_driver max1720x_i2c_driver = {
 		.of_match_table = max1720x_of_match,
 	},
 	.probe = max1720x_probe,
+	.remove = max1720x_remove,
 };
 module_i2c_driver(max1720x_i2c_driver);