diff mbox series

[v2] power: supply: max1720x: add read support for nvmem

Message ID 20240831182145.11589-1-dima.fedrau@gmail.com
State Superseded
Headers show
Series [v2] power: supply: max1720x: add read support for nvmem | expand

Commit Message

Dimitri Fedrau Aug. 31, 2024, 6:21 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

Changes in V2:
  - remove function max1720x_remove and use devm_add_action_or_reset() to
    unregister info->ancillary to avoid race condition during module remove

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

Comments

kernel test robot Sept. 1, 2024, 12:35 a.m. UTC | #1
Hi Dimitri,

kernel test robot noticed the following build errors:

[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on linus/master v6.11-rc5 next-20240830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dimitri-Fedrau/power-supply-max1720x-add-read-support-for-nvmem/20240901-022223
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240831182145.11589-1-dima.fedrau%40gmail.com
patch subject: [PATCH v2] power: supply: max1720x: add read support for nvmem
config: i386-buildonly-randconfig-001-20240901 (https://download.01.org/0day-ci/archive/20240901/202409010834.TxoQkZvt-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240901/202409010834.TxoQkZvt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409010834.TxoQkZvt-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/power/supply/max1720x_battery.c:137:37: error: array type has incomplete element type 'struct nvmem_cell_info'
     137 | static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
         |                                     ^~~~~~~~~~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c: In function 'max1720x_probe_nvmem':
>> drivers/power/supply/max1720x_battery.c:404:16: error: variable 'nvmem_config' has initializer but incomplete type
     404 |         struct nvmem_config nvmem_config = {
         |                ^~~~~~~~~~~~
>> drivers/power/supply/max1720x_battery.c:405:18: error: 'struct nvmem_config' has no member named 'dev'
     405 |                 .dev = dev,
         |                  ^~~
>> drivers/power/supply/max1720x_battery.c:405:24: warning: excess elements in struct initializer
     405 |                 .dev = dev,
         |                        ^~~
   drivers/power/supply/max1720x_battery.c:405:24: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:406:18: error: 'struct nvmem_config' has no member named 'name'
     406 |                 .name = "max1720x_nvmem",
         |                  ^~~~
   drivers/power/supply/max1720x_battery.c:406:25: warning: excess elements in struct initializer
     406 |                 .name = "max1720x_nvmem",
         |                         ^~~~~~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c:406:25: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:407:18: error: 'struct nvmem_config' has no member named 'cells'
     407 |                 .cells = max1720x_nvmem_cells,
         |                  ^~~~~
   drivers/power/supply/max1720x_battery.c:407:26: warning: excess elements in struct initializer
     407 |                 .cells = max1720x_nvmem_cells,
         |                          ^~~~~~~~~~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c:407:26: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:408:18: error: 'struct nvmem_config' has no member named 'ncells'
     408 |                 .ncells = ARRAY_SIZE(max1720x_nvmem_cells),
         |                  ^~~~~~
   In file included from include/linux/bitfield.h:10,
                    from drivers/power/supply/max1720x_battery.c:10:
   include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
      16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
         |                                                   ^
   include/linux/compiler.h:243:33: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
     243 | #define __must_be_array(a)      BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
         |                                 ^~~~~~~~~~~~~~~~~
   include/linux/array_size.h:11:59: note: in expansion of macro '__must_be_array'
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                                           ^~~~~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c:408:27: note: in expansion of macro 'ARRAY_SIZE'
     408 |                 .ncells = ARRAY_SIZE(max1720x_nvmem_cells),
         |                           ^~~~~~~~~~
   In file included from include/linux/string.h:6,
                    from arch/x86/include/asm/page_32.h:18,
                    from arch/x86/include/asm/page.h:14,
                    from arch/x86/include/asm/thread_info.h:12,
                    from include/linux/thread_info.h:60,
                    from include/linux/spinlock.h:60,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/slab.h:16,
                    from include/linux/resource_ext.h:11,
                    from include/linux/acpi.h:13,
                    from include/linux/i2c.h:13,
                    from drivers/power/supply/max1720x_battery.c:11:
   include/linux/array_size.h:11:25: warning: excess elements in struct initializer
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                         ^
   drivers/power/supply/max1720x_battery.c:408:27: note: in expansion of macro 'ARRAY_SIZE'
     408 |                 .ncells = ARRAY_SIZE(max1720x_nvmem_cells),
         |                           ^~~~~~~~~~
   include/linux/array_size.h:11:25: note: (near initialization for 'nvmem_config')
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                         ^
   drivers/power/supply/max1720x_battery.c:408:27: note: in expansion of macro 'ARRAY_SIZE'
     408 |                 .ncells = ARRAY_SIZE(max1720x_nvmem_cells),
         |                           ^~~~~~~~~~
>> drivers/power/supply/max1720x_battery.c:409:18: error: 'struct nvmem_config' has no member named 'read_only'
     409 |                 .read_only = true,
         |                  ^~~~~~~~~
   drivers/power/supply/max1720x_battery.c:409:30: warning: excess elements in struct initializer
     409 |                 .read_only = true,
         |                              ^~~~
   drivers/power/supply/max1720x_battery.c:409:30: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:410:18: error: 'struct nvmem_config' has no member named 'root_only'
     410 |                 .root_only = true,
         |                  ^~~~~~~~~
   drivers/power/supply/max1720x_battery.c:410:30: warning: excess elements in struct initializer
     410 |                 .root_only = true,
         |                              ^~~~
   drivers/power/supply/max1720x_battery.c:410:30: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:411:18: error: 'struct nvmem_config' has no member named 'reg_read'
     411 |                 .reg_read = max1720x_nvmem_reg_read,
         |                  ^~~~~~~~
   drivers/power/supply/max1720x_battery.c:411:29: warning: excess elements in struct initializer
     411 |                 .reg_read = max1720x_nvmem_reg_read,
         |                             ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c:411:29: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:412:18: error: 'struct nvmem_config' has no member named 'size'
     412 |                 .size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
         |                  ^~~~
   include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
      16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
         |                                                   ^
   include/linux/compiler.h:243:33: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
     243 | #define __must_be_array(a)      BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
         |                                 ^~~~~~~~~~~~~~~~~
   include/linux/array_size.h:11:59: note: in expansion of macro '__must_be_array'
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                                           ^~~~~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c:412:25: note: in expansion of macro 'ARRAY_SIZE'
     412 |                 .size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
         |                         ^~~~~~~~~~
   include/linux/array_size.h:11:25: warning: excess elements in struct initializer
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                         ^
   drivers/power/supply/max1720x_battery.c:412:25: note: in expansion of macro 'ARRAY_SIZE'
     412 |                 .size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
         |                         ^~~~~~~~~~
   include/linux/array_size.h:11:25: note: (near initialization for 'nvmem_config')
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                         ^
   drivers/power/supply/max1720x_battery.c:412:25: note: in expansion of macro 'ARRAY_SIZE'
     412 |                 .size = ARRAY_SIZE(max1720x_nvmem_cells) * 2,
         |                         ^~~~~~~~~~
>> drivers/power/supply/max1720x_battery.c:413:18: error: 'struct nvmem_config' has no member named 'word_size'
     413 |                 .word_size = 2,
         |                  ^~~~~~~~~
   drivers/power/supply/max1720x_battery.c:413:30: warning: excess elements in struct initializer
     413 |                 .word_size = 2,
         |                              ^
   drivers/power/supply/max1720x_battery.c:413:30: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:414:18: error: 'struct nvmem_config' has no member named 'stride'
     414 |                 .stride = 2,
         |                  ^~~~~~
   drivers/power/supply/max1720x_battery.c:414:27: warning: excess elements in struct initializer
     414 |                 .stride = 2,
         |                           ^
   drivers/power/supply/max1720x_battery.c:414:27: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:415:18: error: 'struct nvmem_config' has no member named 'priv'
     415 |                 .priv = info,
         |                  ^~~~
   drivers/power/supply/max1720x_battery.c:415:25: warning: excess elements in struct initializer
     415 |                 .priv = info,
         |                         ^~~~
   drivers/power/supply/max1720x_battery.c:415:25: note: (near initialization for 'nvmem_config')
>> drivers/power/supply/max1720x_battery.c:404:29: error: storage size of 'nvmem_config' isn't known
     404 |         struct nvmem_config nvmem_config = {
         |                             ^~~~~~~~~~~~
>> drivers/power/supply/max1720x_battery.c:453:17: error: implicit declaration of function 'devm_nvmem_register'; did you mean 'device_register'? [-Werror=implicit-function-declaration]
     453 |         nvmem = devm_nvmem_register(dev, &nvmem_config);
         |                 ^~~~~~~~~~~~~~~~~~~
         |                 device_register
>> drivers/power/supply/max1720x_battery.c:404:29: warning: unused variable 'nvmem_config' [-Wunused-variable]
     404 |         struct nvmem_config nvmem_config = {
         |                             ^~~~~~~~~~~~
   drivers/power/supply/max1720x_battery.c: At top level:
>> drivers/power/supply/max1720x_battery.c:137:37: warning: 'max1720x_nvmem_cells' defined but not used [-Wunused-variable]
     137 | static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
         |                                     ^~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +137 drivers/power/supply/max1720x_battery.c

   136	
 > 137	static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
   138		{ .name = "nXTable0",  .offset = 0,  .bytes = 2, },
   139		{ .name = "nXTable1",  .offset = 2,  .bytes = 2, },
   140		{ .name = "nXTable2",  .offset = 4,  .bytes = 2, },
   141		{ .name = "nXTable3",  .offset = 6,  .bytes = 2, },
   142		{ .name = "nXTable4",  .offset = 8,  .bytes = 2, },
   143		{ .name = "nXTable5",  .offset = 10, .bytes = 2, },
   144		{ .name = "nXTable6",  .offset = 12, .bytes = 2, },
   145		{ .name = "nXTable7",  .offset = 14, .bytes = 2, },
   146		{ .name = "nXTable8",  .offset = 16, .bytes = 2, },
   147		{ .name = "nXTable9",  .offset = 18, .bytes = 2, },
   148		{ .name = "nXTable10", .offset = 20, .bytes = 2, },
   149		{ .name = "nXTable11", .offset = 22, .bytes = 2, },
   150		{ .name = "nUser18C",  .offset = 24, .bytes = 2, },
   151		{ .name = "nUser18D",  .offset = 26, .bytes = 2, },
   152		{ .name = "nODSCTh",   .offset = 28, .bytes = 2, },
   153		{ .name = "nODSCCfg",  .offset = 30, .bytes = 2, },
   154	
   155		{ .name = "nOCVTable0",  .offset = 32, .bytes = 2, },
   156		{ .name = "nOCVTable1",  .offset = 34, .bytes = 2, },
   157		{ .name = "nOCVTable2",  .offset = 36, .bytes = 2, },
   158		{ .name = "nOCVTable3",  .offset = 38, .bytes = 2, },
   159		{ .name = "nOCVTable4",  .offset = 40, .bytes = 2, },
   160		{ .name = "nOCVTable5",  .offset = 42, .bytes = 2, },
   161		{ .name = "nOCVTable6",  .offset = 44, .bytes = 2, },
   162		{ .name = "nOCVTable7",  .offset = 46, .bytes = 2, },
   163		{ .name = "nOCVTable8",  .offset = 48, .bytes = 2, },
   164		{ .name = "nOCVTable9",  .offset = 50, .bytes = 2, },
   165		{ .name = "nOCVTable10", .offset = 52, .bytes = 2, },
   166		{ .name = "nOCVTable11", .offset = 54, .bytes = 2, },
   167		{ .name = "nIChgTerm",   .offset = 56, .bytes = 2, },
   168		{ .name = "nFilterCfg",  .offset = 58, .bytes = 2, },
   169		{ .name = "nVEmpty",     .offset = 60, .bytes = 2, },
   170		{ .name = "nLearnCfg",   .offset = 62, .bytes = 2, },
   171	
   172		{ .name = "nQRTable00",  .offset = 64, .bytes = 2, },
   173		{ .name = "nQRTable10",  .offset = 66, .bytes = 2, },
   174		{ .name = "nQRTable20",  .offset = 68, .bytes = 2, },
   175		{ .name = "nQRTable30",  .offset = 70, .bytes = 2, },
   176		{ .name = "nCycles",     .offset = 72, .bytes = 2, },
   177		{ .name = "nFullCapNom", .offset = 74, .bytes = 2, },
   178		{ .name = "nRComp0",     .offset = 76, .bytes = 2, },
   179		{ .name = "nTempCo",     .offset = 78, .bytes = 2, },
   180		{ .name = "nIAvgEmpty",  .offset = 80, .bytes = 2, },
   181		{ .name = "nFullCapRep", .offset = 82, .bytes = 2, },
   182		{ .name = "nVoltTemp",   .offset = 84, .bytes = 2, },
   183		{ .name = "nMaxMinCurr", .offset = 86, .bytes = 2, },
   184		{ .name = "nMaxMinVolt", .offset = 88, .bytes = 2, },
   185		{ .name = "nMaxMinTemp", .offset = 90, .bytes = 2, },
   186		{ .name = "nSOC",        .offset = 92, .bytes = 2, },
   187		{ .name = "nTimerH",     .offset = 94, .bytes = 2, },
   188	
   189		{ .name = "nConfig",    .offset = 96,  .bytes = 2, },
   190		{ .name = "nRippleCfg", .offset = 98,  .bytes = 2, },
   191		{ .name = "nMiscCfg",   .offset = 100, .bytes = 2, },
   192		{ .name = "nDesignCap", .offset = 102, .bytes = 2, },
   193		{ .name = "nHibCfg",    .offset = 104, .bytes = 2, },
   194		{ .name = "nPackCfg",   .offset = 106, .bytes = 2, },
   195		{ .name = "nRelaxCfg",  .offset = 108, .bytes = 2, },
   196		{ .name = "nConvgCfg",  .offset = 110, .bytes = 2, },
   197		{ .name = "nNVCfg0",    .offset = 112, .bytes = 2, },
   198		{ .name = "nNVCfg1",    .offset = 114, .bytes = 2, },
   199		{ .name = "nNVCfg2",    .offset = 116, .bytes = 2, },
   200		{ .name = "nSBSCfg",    .offset = 118, .bytes = 2, },
   201		{ .name = "nROMID0",    .offset = 120, .bytes = 2, },
   202		{ .name = "nROMID1",    .offset = 122, .bytes = 2, },
   203		{ .name = "nROMID2",    .offset = 124, .bytes = 2, },
   204		{ .name = "nROMID3",    .offset = 126, .bytes = 2, },
   205	
   206		{ .name = "nVAlrtTh",      .offset = 128, .bytes = 2, },
   207		{ .name = "nTAlrtTh",      .offset = 130, .bytes = 2, },
   208		{ .name = "nSAlrtTh",      .offset = 132, .bytes = 2, },
   209		{ .name = "nIAlrtTh",      .offset = 134, .bytes = 2, },
   210		{ .name = "nUser1C4",      .offset = 136, .bytes = 2, },
   211		{ .name = "nUser1C5",      .offset = 138, .bytes = 2, },
   212		{ .name = "nFullSOCThr",   .offset = 140, .bytes = 2, },
   213		{ .name = "nTTFCfg",       .offset = 142, .bytes = 2, },
   214		{ .name = "nCGain",        .offset = 144, .bytes = 2, },
   215		{ .name = "nTCurve",       .offset = 146, .bytes = 2, },
   216		{ .name = "nTGain",        .offset = 148, .bytes = 2, },
   217		{ .name = "nTOff",         .offset = 150, .bytes = 2, },
   218		{ .name = "nManfctrName0", .offset = 152, .bytes = 2, },
   219		{ .name = "nManfctrName1", .offset = 154, .bytes = 2, },
   220		{ .name = "nManfctrName2", .offset = 156, .bytes = 2, },
   221		{ .name = "nRSense",       .offset = 158, .bytes = 2, },
   222	
   223		{ .name = "nUser1D0",       .offset = 160, .bytes = 2, },
   224		{ .name = "nUser1D1",       .offset = 162, .bytes = 2, },
   225		{ .name = "nAgeFcCfg",      .offset = 164, .bytes = 2, },
   226		{ .name = "nDesignVoltage", .offset = 166, .bytes = 2, },
   227		{ .name = "nUser1D4",       .offset = 168, .bytes = 2, },
   228		{ .name = "nRFastVShdn",    .offset = 170, .bytes = 2, },
   229		{ .name = "nManfctrDate",   .offset = 172, .bytes = 2, },
   230		{ .name = "nFirstUsed",     .offset = 174, .bytes = 2, },
   231		{ .name = "nSerialNumber0", .offset = 176, .bytes = 2, },
   232		{ .name = "nSerialNumber1", .offset = 178, .bytes = 2, },
   233		{ .name = "nSerialNumber2", .offset = 180, .bytes = 2, },
   234		{ .name = "nDeviceName0",   .offset = 182, .bytes = 2, },
   235		{ .name = "nDeviceName1",   .offset = 184, .bytes = 2, },
   236		{ .name = "nDeviceName2",   .offset = 186, .bytes = 2, },
   237		{ .name = "nDeviceName3",   .offset = 188, .bytes = 2, },
   238		{ .name = "nDeviceName4",   .offset = 190, .bytes = 2, },
   239	};
   240
kernel test robot Sept. 1, 2024, 12:56 a.m. UTC | #2
Hi Dimitri,

kernel test robot noticed the following build errors:

[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on linus/master v6.11-rc5 next-20240830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dimitri-Fedrau/power-supply-max1720x-add-read-support-for-nvmem/20240901-022223
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240831182145.11589-1-dima.fedrau%40gmail.com
patch subject: [PATCH v2] power: supply: max1720x: add read support for nvmem
config: i386-buildonly-randconfig-004-20240901 (https://download.01.org/0day-ci/archive/20240901/202409010801.Ca4oeaWg-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240901/202409010801.Ca4oeaWg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409010801.Ca4oeaWg-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/power/supply/max1720x_battery.c:137:57: error: array has incomplete element type 'const struct nvmem_cell_info'
     137 | static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
         |                                                         ^
   drivers/power/supply/max1720x_battery.c:137:21: note: forward declaration of 'struct nvmem_cell_info'
     137 | static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
         |                     ^
>> drivers/power/supply/max1720x_battery.c:404:22: error: variable has incomplete type 'struct nvmem_config'
     404 |         struct nvmem_config nvmem_config = {
         |                             ^
   drivers/power/supply/max1720x_battery.c:404:9: note: forward declaration of 'struct nvmem_config'
     404 |         struct nvmem_config nvmem_config = {
         |                ^
>> drivers/power/supply/max1720x_battery.c:453:10: error: call to undeclared function 'devm_nvmem_register'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     453 |         nvmem = devm_nvmem_register(dev, &nvmem_config);
         |                 ^
   drivers/power/supply/max1720x_battery.c:453:10: note: did you mean 'device_register'?
   include/linux/device.h:1064:18: note: 'device_register' declared here
    1064 | int __must_check device_register(struct device *dev);
         |                  ^
   3 errors generated.


vim +137 drivers/power/supply/max1720x_battery.c

   136	
 > 137	static const struct nvmem_cell_info max1720x_nvmem_cells[] = {
   138		{ .name = "nXTable0",  .offset = 0,  .bytes = 2, },
   139		{ .name = "nXTable1",  .offset = 2,  .bytes = 2, },
   140		{ .name = "nXTable2",  .offset = 4,  .bytes = 2, },
   141		{ .name = "nXTable3",  .offset = 6,  .bytes = 2, },
   142		{ .name = "nXTable4",  .offset = 8,  .bytes = 2, },
   143		{ .name = "nXTable5",  .offset = 10, .bytes = 2, },
   144		{ .name = "nXTable6",  .offset = 12, .bytes = 2, },
   145		{ .name = "nXTable7",  .offset = 14, .bytes = 2, },
   146		{ .name = "nXTable8",  .offset = 16, .bytes = 2, },
   147		{ .name = "nXTable9",  .offset = 18, .bytes = 2, },
   148		{ .name = "nXTable10", .offset = 20, .bytes = 2, },
   149		{ .name = "nXTable11", .offset = 22, .bytes = 2, },
   150		{ .name = "nUser18C",  .offset = 24, .bytes = 2, },
   151		{ .name = "nUser18D",  .offset = 26, .bytes = 2, },
   152		{ .name = "nODSCTh",   .offset = 28, .bytes = 2, },
   153		{ .name = "nODSCCfg",  .offset = 30, .bytes = 2, },
   154	
   155		{ .name = "nOCVTable0",  .offset = 32, .bytes = 2, },
   156		{ .name = "nOCVTable1",  .offset = 34, .bytes = 2, },
   157		{ .name = "nOCVTable2",  .offset = 36, .bytes = 2, },
   158		{ .name = "nOCVTable3",  .offset = 38, .bytes = 2, },
   159		{ .name = "nOCVTable4",  .offset = 40, .bytes = 2, },
   160		{ .name = "nOCVTable5",  .offset = 42, .bytes = 2, },
   161		{ .name = "nOCVTable6",  .offset = 44, .bytes = 2, },
   162		{ .name = "nOCVTable7",  .offset = 46, .bytes = 2, },
   163		{ .name = "nOCVTable8",  .offset = 48, .bytes = 2, },
   164		{ .name = "nOCVTable9",  .offset = 50, .bytes = 2, },
   165		{ .name = "nOCVTable10", .offset = 52, .bytes = 2, },
   166		{ .name = "nOCVTable11", .offset = 54, .bytes = 2, },
   167		{ .name = "nIChgTerm",   .offset = 56, .bytes = 2, },
   168		{ .name = "nFilterCfg",  .offset = 58, .bytes = 2, },
   169		{ .name = "nVEmpty",     .offset = 60, .bytes = 2, },
   170		{ .name = "nLearnCfg",   .offset = 62, .bytes = 2, },
   171	
   172		{ .name = "nQRTable00",  .offset = 64, .bytes = 2, },
   173		{ .name = "nQRTable10",  .offset = 66, .bytes = 2, },
   174		{ .name = "nQRTable20",  .offset = 68, .bytes = 2, },
   175		{ .name = "nQRTable30",  .offset = 70, .bytes = 2, },
   176		{ .name = "nCycles",     .offset = 72, .bytes = 2, },
   177		{ .name = "nFullCapNom", .offset = 74, .bytes = 2, },
   178		{ .name = "nRComp0",     .offset = 76, .bytes = 2, },
   179		{ .name = "nTempCo",     .offset = 78, .bytes = 2, },
   180		{ .name = "nIAvgEmpty",  .offset = 80, .bytes = 2, },
   181		{ .name = "nFullCapRep", .offset = 82, .bytes = 2, },
   182		{ .name = "nVoltTemp",   .offset = 84, .bytes = 2, },
   183		{ .name = "nMaxMinCurr", .offset = 86, .bytes = 2, },
   184		{ .name = "nMaxMinVolt", .offset = 88, .bytes = 2, },
   185		{ .name = "nMaxMinTemp", .offset = 90, .bytes = 2, },
   186		{ .name = "nSOC",        .offset = 92, .bytes = 2, },
   187		{ .name = "nTimerH",     .offset = 94, .bytes = 2, },
   188	
   189		{ .name = "nConfig",    .offset = 96,  .bytes = 2, },
   190		{ .name = "nRippleCfg", .offset = 98,  .bytes = 2, },
   191		{ .name = "nMiscCfg",   .offset = 100, .bytes = 2, },
   192		{ .name = "nDesignCap", .offset = 102, .bytes = 2, },
   193		{ .name = "nHibCfg",    .offset = 104, .bytes = 2, },
   194		{ .name = "nPackCfg",   .offset = 106, .bytes = 2, },
   195		{ .name = "nRelaxCfg",  .offset = 108, .bytes = 2, },
   196		{ .name = "nConvgCfg",  .offset = 110, .bytes = 2, },
   197		{ .name = "nNVCfg0",    .offset = 112, .bytes = 2, },
   198		{ .name = "nNVCfg1",    .offset = 114, .bytes = 2, },
   199		{ .name = "nNVCfg2",    .offset = 116, .bytes = 2, },
   200		{ .name = "nSBSCfg",    .offset = 118, .bytes = 2, },
   201		{ .name = "nROMID0",    .offset = 120, .bytes = 2, },
   202		{ .name = "nROMID1",    .offset = 122, .bytes = 2, },
   203		{ .name = "nROMID2",    .offset = 124, .bytes = 2, },
   204		{ .name = "nROMID3",    .offset = 126, .bytes = 2, },
   205	
   206		{ .name = "nVAlrtTh",      .offset = 128, .bytes = 2, },
   207		{ .name = "nTAlrtTh",      .offset = 130, .bytes = 2, },
   208		{ .name = "nSAlrtTh",      .offset = 132, .bytes = 2, },
   209		{ .name = "nIAlrtTh",      .offset = 134, .bytes = 2, },
   210		{ .name = "nUser1C4",      .offset = 136, .bytes = 2, },
   211		{ .name = "nUser1C5",      .offset = 138, .bytes = 2, },
   212		{ .name = "nFullSOCThr",   .offset = 140, .bytes = 2, },
   213		{ .name = "nTTFCfg",       .offset = 142, .bytes = 2, },
   214		{ .name = "nCGain",        .offset = 144, .bytes = 2, },
   215		{ .name = "nTCurve",       .offset = 146, .bytes = 2, },
   216		{ .name = "nTGain",        .offset = 148, .bytes = 2, },
   217		{ .name = "nTOff",         .offset = 150, .bytes = 2, },
   218		{ .name = "nManfctrName0", .offset = 152, .bytes = 2, },
   219		{ .name = "nManfctrName1", .offset = 154, .bytes = 2, },
   220		{ .name = "nManfctrName2", .offset = 156, .bytes = 2, },
   221		{ .name = "nRSense",       .offset = 158, .bytes = 2, },
   222	
   223		{ .name = "nUser1D0",       .offset = 160, .bytes = 2, },
   224		{ .name = "nUser1D1",       .offset = 162, .bytes = 2, },
   225		{ .name = "nAgeFcCfg",      .offset = 164, .bytes = 2, },
   226		{ .name = "nDesignVoltage", .offset = 166, .bytes = 2, },
   227		{ .name = "nUser1D4",       .offset = 168, .bytes = 2, },
   228		{ .name = "nRFastVShdn",    .offset = 170, .bytes = 2, },
   229		{ .name = "nManfctrDate",   .offset = 172, .bytes = 2, },
   230		{ .name = "nFirstUsed",     .offset = 174, .bytes = 2, },
   231		{ .name = "nSerialNumber0", .offset = 176, .bytes = 2, },
   232		{ .name = "nSerialNumber1", .offset = 178, .bytes = 2, },
   233		{ .name = "nSerialNumber2", .offset = 180, .bytes = 2, },
   234		{ .name = "nDeviceName0",   .offset = 182, .bytes = 2, },
   235		{ .name = "nDeviceName1",   .offset = 184, .bytes = 2, },
   236		{ .name = "nDeviceName2",   .offset = 186, .bytes = 2, },
   237		{ .name = "nDeviceName3",   .offset = 188, .bytes = 2, },
   238		{ .name = "nDeviceName4",   .offset = 190, .bytes = 2, },
   239	};
   240
Sebastian Reichel Sept. 2, 2024, 9:55 p.m. UTC | #3
Hi,

On Sat, Aug 31, 2024 at 08:21:45PM 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
> 
> Changes in V2:
>   - remove function max1720x_remove and use devm_add_action_or_reset() to
>     unregister info->ancillary to avoid race condition during module remove

Thanks, but the transformation is quite incomplete. You probably
should have a look what device managed resource actually means :)

> ---
>  drivers/power/supply/max1720x_battery.c | 220 ++++++++++++++++++++++--
>  1 file changed, 205 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
> index edc262f0a62f..d27c94bdb835 100644
> --- a/drivers/power/supply/max1720x_battery.c
> +++ b/drivers/power/supply/max1720x_battery.c

[...]

> +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 = {

As noticed by the build bot: you need to add this include for the
struct:

#include <linux/nvmem-provider.h>

> +		.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;
> +	ret = devm_add_action_or_reset(dev, max1720x_unregister_ancillary, info);
> +	if (ret) {
> +		dev_err(dev, "Failed to add unregister callback\n");
> +		goto err;
> +	}
>  
> -	info->rsense = 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 = 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);

devm_add_action_or_reset() already unregisters on failure, so this
results in a double unregister. Please also simplify 'goto err'
with 'return ret;'.

> +
> +	return ret;
>  }
>  
>  static const struct power_supply_desc max1720x_bat_desc = {
> @@ -299,20 +487,22 @@ 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);

This is also already handled by devm and must be removed.

>  		return dev_err_probe(dev, PTR_ERR(bat),
>  				     "Failed to register power supply\n");
> +	}
>  
>  	return 0;
>  }

-- Sebastian
Dimitri Fedrau Sept. 3, 2024, 6:33 a.m. UTC | #4
Am Mon, Sep 02, 2024 at 09:55:42PM +0000 schrieb Sebastian Reichel:
> Hi,
> 
> On Sat, Aug 31, 2024 at 08:21:45PM 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
> > 
> > Changes in V2:
> >   - remove function max1720x_remove and use devm_add_action_or_reset() to
> >     unregister info->ancillary to avoid race condition during module remove
> 
> Thanks, but the transformation is quite incomplete. You probably
> should have a look what device managed resource actually means :)
>

Yes, I noticed shortly after sending V2, that I missed to remove all
i2c_unregister_device calls in the error path. Already prepared V3
handling this and also the missing header "linux/nvmem-provider.h".
Thanks for reviewing so quickly, I just hoped to get away with this by
sending V3 before you notice. :)

> > ---
> >  drivers/power/supply/max1720x_battery.c | 220 ++++++++++++++++++++++--
> >  1 file changed, 205 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
> > index edc262f0a62f..d27c94bdb835 100644
> > --- a/drivers/power/supply/max1720x_battery.c
> > +++ b/drivers/power/supply/max1720x_battery.c
> 
> [...]
> 
> > +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 = {
> 
> As noticed by the build bot: you need to add this include for the
> struct:
> 
> #include <linux/nvmem-provider.h>
>
Will fix it.

> > +		.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;
> > +	ret = devm_add_action_or_reset(dev, max1720x_unregister_ancillary, info);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to add unregister callback\n");
> > +		goto err;
> > +	}
> >  
> > -	info->rsense = 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 = 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);
> 
> devm_add_action_or_reset() already unregisters on failure, so this
> results in a double unregister. Please also simplify 'goto err'
> with 'return ret;'.
>
Will fix it.

> > +
> > +	return ret;
> >  }
> >  
> >  static const struct power_supply_desc max1720x_bat_desc = {
> > @@ -299,20 +487,22 @@ 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);
> 
> This is also already handled by devm and must be removed.
>
Will fix it.
> >  		return dev_err_probe(dev, PTR_ERR(bat),
> >  				     "Failed to register power supply\n");
> > +	}
> >  
> >  	return 0;
> >  }
> 
> -- Sebastian

Best regards
Dimitri Fedrau
diff mbox series

Patch

diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
index edc262f0a62f..d27c94bdb835 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,87 @@  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 void max1720x_unregister_ancillary(void *data)
+{
+	struct max1720x_device_info *info = data;
+
+	i2c_unregister_device(info->ancillary);
+}
+
+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;
+	ret = devm_add_action_or_reset(dev, max1720x_unregister_ancillary, info);
+	if (ret) {
+		dev_err(dev, "Failed to add unregister callback\n");
+		goto err;
+	}
 
-	info->rsense = 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 = 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,20 +487,22 @@  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;
 }