Re: [RFC PATCH 06/13] mfd: bd71828: Support ROHM BD72720
From: Matti Vaittinen
Date: Tue Oct 14 2025 - 01:41:59 EST
On 13/10/2025 16:19, Andreas Kemnade wrote:
On Mon, 13 Oct 2025 12:27:33 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
Hi Andreas!Well, you could also stack regmaps like ntxec.c is doing (but there
First of all, thanks for taking a look at this!
On 10/10/2025 16:03, Andreas Kemnade wrote:
On Fri, 10 Oct 2025 15:09:07 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
+static int bd72720_get_secondary_regmap(struct i2c_client *i2c,
Does this 'secondary' have a specific purpose or a better name?
I am not entirely sure. When I asked this from the designers they just
told me that they needed more than 255 registers so they added another
slave address... (I'm not sure what would have been wrong with using a
page register). So, I assume they just placed stuff that didn't fit in
first 255 register there. But yeah, it looks like most of the registers
there are related to the charger. So, perhaps it isn't completely
misleading to use "charger regmap"? The data-sheet seems to be just
using "Register map 1" and "Register map 2" in the tables listing these
registers. I kind of like using something which maps easily to the
data-sheet, but I really have no strong opinion on this.
just another idea: What about one regmap with custom functions covering
both these adresses? Maybe that could even be added to the regmap
functionality, maybe with a 0x100 offset for the second range.
That way the rest of the code only needs to real with one regmap
and properly defined registers.
Interesting idea.
I suppose you mean something like implementing custom remap_read() and
regmap_write() - which would practically select the I2C adapter to use
based on the register address - and then doing same thing as the
regmap_i2c_smbus_i2c_write() / regmap_i2c_smbus_i2c_read() do?
I suppose this would mean duplicating the functionality provided by the
regmap_i2c_smbus_i2c_write() and the regmap_i2c_smbus_i2c_read(), which
are static. It'd also mean we'll lose the 1 to 1 mapping between the
register addresses in driver and addresses in the data-sheet. I agree
this wouldn't be such a huge thing if we used offset like 0x100 though.
for some very weird reason). That would avoid duplicating code.
Ah. I suppose you suggest I'd try something like:
/* untested, not compiled, pseudo-code */
struct bd72720_regmaps {
struct regmap *map1_4b;
struct regmap *map2_4c;
};
static int regmap_write_wrapper(void *context,
unsigned int reg, unsigned int val)
{
struct bd72720_regmaps *maps = context;
if (reg >= 0x100)
return regmap_write(maps->map2_4c, reg, val);
return regmap_write(maps->map1_4b, reg, val);
}
static int regmap_read_wrapper(void *context, unsigned int reg,
unsigned int *val)
{
if (reg >= 0x100)
return regmap_read(maps->map2_4c, reg, val);
return regmap_read(maps->map1_4b, reg, val);
}
static const struct regmap_config wrapper_map_config {
.name = "Maybe_a_descriptive_name_here_Which_devices_can_use_to_get_this_instead_of_map1_4b",
...
.reg_write = regmap_write_wrapper,
.reg_read = regmap_read_wrapper,
}
probe()
{
struct bd72720_regmaps *maps;
struct regmap *the_map_we_use_from_all_the_devices;
...
maps->map1_4b = devm_regmap_init_i2c(i2c, &bd72720_regmap_4b);
maps->map2_4c = devm_regmap_init_i2c(i2c2, &bd72720_regmap_4c);
/* Init wrapper map */
the_map_we_use_from_all_the_devices = devm_regmap_init(dev,
NULL, maps, wrapper_map_config);
}
If this works, then I kind of like this. It avoids using the platform data and simplifies the regmap getting in the power_supply driver. Thanks for the good idea Andreas!
Lee, objections to this?
Yours,
-- Matti