Re: [PATCH v3 09/11] mfd: bq257xx: Add BQ25792 support

[Alexey Charkov]

On Tue, Mar 10, 2026 at 5:03 PM Lee Jones <lee@xxxxxxxxxx> wrote:
>
> On Tue, 10 Mar 2026, Alexey Charkov wrote:
>
> > Add register definitions and a new 'type' enum to be passed in MFD
> > platform data to support the BQ25792, which is a newer variant of the
> > BQ257xx family.
> >
> > BQ25792 shares similar logic of operation with the already supported
> > BQ25703A but has a completely different register map and different
> > electrical constraints.
> >
> > Tested-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
> > ---
> >  drivers/mfd/bq257xx.c       |  64 ++++++-
> >  include/linux/mfd/bq257xx.h | 417 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 477 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mfd/bq257xx.c b/drivers/mfd/bq257xx.c
> > index e9d49dac0a16..7516e2ec00e6 100644
> > --- a/drivers/mfd/bq257xx.c
> > +++ b/drivers/mfd/bq257xx.c
> > @@ -10,6 +10,11 @@
> >  #include <linux/mfd/core.h>
> >  #include <linux/regmap.h>
> >
> > +struct bq257xx_match_data {
> > +     const struct bq257xx_plat plat;
> > +     const struct regmap_config *regmap_config;
> > +};
> > +
> >  static const struct regmap_range bq25703_readonly_reg_ranges[] = {
> >       regmap_reg_range(BQ25703_CHARGER_STATUS, BQ25703_MANUFACT_DEV_ID),
> >  };
> > @@ -39,15 +44,64 @@ static const struct regmap_config bq25703_regmap_config = {
> >       .val_format_endian = REGMAP_ENDIAN_LITTLE,
> >  };
> >
> > -static const struct mfd_cell cells[] = {
> > +static const struct bq257xx_match_data bq25703a_match_data = {
> > +     .plat = { .type = BQ25703A },
> > +     .regmap_config = &bq25703_regmap_config,
> > +};
> > +
> > +static const struct regmap_range bq25792_writeable_reg_ranges[] = {
> > +     regmap_reg_range(BQ25792_REG00_MIN_SYS_VOLTAGE,
> > +                      BQ25792_REG18_NTC_CONTROL_1),
> > +     regmap_reg_range(BQ25792_REG28_CHARGER_MASK_0,
> > +                      BQ25792_REG30_ADC_FUNCTION_DISABLE_1),
> > +};
> > +
> > +static const struct regmap_access_table bq25792_writeable_regs = {
> > +     .yes_ranges = bq25792_writeable_reg_ranges,
> > +     .n_yes_ranges = ARRAY_SIZE(bq25792_writeable_reg_ranges),
> > +};
> > +
> > +static const struct regmap_range bq25792_volatile_reg_ranges[] = {
> > +     regmap_reg_range(BQ25792_REG19_ICO_CURRENT_LIMIT,
> > +                      BQ25792_REG27_FAULT_FLAG_1),
> > +     regmap_reg_range(BQ25792_REG31_IBUS_ADC,
> > +                      BQ25792_REG47_DPDM_DRIVER),
> > +};
> > +
> > +static const struct regmap_access_table bq25792_volatile_regs = {
> > +     .yes_ranges = bq25792_volatile_reg_ranges,
> > +     .n_yes_ranges = ARRAY_SIZE(bq25792_volatile_reg_ranges),
> > +};
> > +
> > +static const struct regmap_config bq25792_regmap_config = {
> > +     .reg_bits = 8,
> > +     .val_bits = 8,
> > +     .max_register = BQ25792_REG48_PART_INFORMATION,
> > +     .cache_type = REGCACHE_MAPLE,
> > +     .wr_table = &bq25792_writeable_regs,
> > +     .volatile_table = &bq25792_volatile_regs,
> > +};
> > +
> > +static const struct bq257xx_match_data bq25792_match_data = {
> > +     .plat = { .type = BQ25792 },
> > +     .regmap_config = &bq25792_regmap_config,
> > +};
> > +
> > +static struct mfd_cell cells[] = {
> >       MFD_CELL_NAME("bq257xx-regulator"),
> >       MFD_CELL_NAME("bq257xx-charger"),
> >  };
> >
> >  static int bq257xx_probe(struct i2c_client *client)
> >  {
> > +     const struct bq257xx_match_data *md = device_get_match_data(&client->dev);
> >       struct bq257xx_device *ddata;
> > -     int ret;
> > +     int i, ret;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cells); i++) {
> > +             cells[i].platform_data = &md->plat;
> > +             cells[i].pdata_size = sizeof(md->plat);
> > +     }
> >
> >       ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
> >       if (!ddata)
> > @@ -55,7 +109,7 @@ static int bq257xx_probe(struct i2c_client *client)
> >
> >       ddata->client = client;
> >
> > -     ddata->regmap = devm_regmap_init_i2c(client, &bq25703_regmap_config);
> > +     ddata->regmap = devm_regmap_init_i2c(client, md->regmap_config);
> >       if (IS_ERR(ddata->regmap)) {
> >               return dev_err_probe(&client->dev, PTR_ERR(ddata->regmap),
> >                                    "Failed to allocate register map\n");
> > @@ -74,12 +128,14 @@ static int bq257xx_probe(struct i2c_client *client)
> >
> >  static const struct i2c_device_id bq257xx_i2c_ids[] = {
> >       { "bq25703a" },
> > +     { "bq25792" },
> >       {}
> >  };
> >  MODULE_DEVICE_TABLE(i2c, bq257xx_i2c_ids);
> >
> >  static const struct of_device_id bq257xx_of_match[] = {
> > -     { .compatible = "ti,bq25703a" },
> > +     { .compatible = "ti,bq25703a", .data = &bq25703a_match_data },
> > +     { .compatible = "ti,bq25792", .data = &bq25792_match_data },
>
> Instead of passing through the data structure, pass an identifier to
> match on, like:
>
>          { .compatible = "ti,bq25703a", .data = (void *)BQ25703A },
>
> Then add the ID to ddata which is already obtainable from the child
> devices via i2c_set_clientdata().  This saves passing it as separate
> platform data.
>
>         ddata->type = device_get_match_data(&client->dev);
>
> Then use that to initialise a local 'struct regmap_config'.
>
>         switch(ddata->type) {
>         case BQ25703A:
>                 regmap_config = &bq25703_regmap_config
>                 break;
>         [...]
>
> By doing so, you save on a lot of additional complexity and mitigate the
> introduction of some unnecessary data structures.

Thanks for the suggestion Lee, it does indeed make the code leaner.
Will wait for any additional feedback and send v4.

Best regards,
Alexey