Re: [PATCH v3 11/11] power: supply: bq257xx: Add support for BQ25792

[Alexey Charkov]

On Wed, Mar 11, 2026 at 12:52 PM Sebastian Reichel
<sebastian.reichel@xxxxxxxxxxxxx> wrote:
>
> Hello Alexey,
>
> On Tue, Mar 10, 2026 at 01:28:35PM +0400, Alexey Charkov wrote:
> > Add support for TI BQ25792 integrated battery charger and buck-boost
> > converter.
> >
> > It shares high-level logic of operation with the already supported
> > BQ25703A, but has a different register map, bit definitions and some of
> > the lower-level hardware states.
> >
> > Tested-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
> > ---
> >  drivers/power/supply/bq257xx_charger.c | 492 ++++++++++++++++++++++++++++++++-
> >  include/linux/mfd/bq257xx.h            |   6 +-
> >  2 files changed, 493 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/power/supply/bq257xx_charger.c b/drivers/power/supply/bq257xx_charger.c
> > index 951abd035fc5..0bbb0a8b5f55 100644
> > --- a/drivers/power/supply/bq257xx_charger.c
> > +++ b/drivers/power/supply/bq257xx_charger.c
> > @@ -5,6 +5,7 @@
> >   */
> >
> >  #include <linux/bitfield.h>
> > +#include <linux/byteorder/generic.h>
> >  #include <linux/i2c.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/mfd/bq257xx.h>
> > @@ -18,12 +19,19 @@ struct bq257xx_chg;
> >
> >  /**
> >   * struct bq257xx_chip_info - chip specific routines
> > + * @default_iindpm_uA: default input current limit in microamps
> >   * @bq257xx_hw_init: init function for hw
> >   * @bq257xx_hw_shutdown: shutdown function for hw
> >   * @bq257xx_get_state: get and update state of hardware
> > + * @bq257xx_get_ichg: get maximum charge current (in uA)
> >   * @bq257xx_set_ichg: set maximum charge current (in uA)
> > + * @bq257xx_get_vbatreg: get maximum charge voltage (in uV)
> >   * @bq257xx_set_vbatreg: set maximum charge voltage (in uV)
> > + * @bq257xx_get_iindpm: get maximum input current (in uA)
> >   * @bq257xx_set_iindpm: set maximum input current (in uA)
> > + * @bq257xx_get_cur: get battery current from ADC (in uA)
> > + * @bq257xx_get_vbat: get battery voltage from ADC (in uV)
> > + * @bq257xx_get_min_vsys: get minimum system voltage (in uV)
> >   */
> >  struct bq257xx_chip_info {
> >       int default_iindpm_uA;
> > @@ -47,8 +55,10 @@ struct bq257xx_chip_info {
> >   * @bq: parent MFD device
> >   * @charger: power supply device
> >   * @online: charger input is present
> > + * @charging: charger is actively charging the battery
> >   * @fast_charge: charger is in fast charge mode
> >   * @pre_charge: charger is in pre-charge mode
> > + * @overvoltage: overvoltage fault detected
> >   * @ov_fault: charger reports over voltage fault
> >   * @batoc_fault: charger reports battery over current fault
> >   * @oc_fault: charger reports over current fault
> > @@ -79,6 +89,53 @@ struct bq257xx_chg {
> >       u32 vsys_min;
> >  };
>
> The above belong into the previous patches that actually added the
> fields to the structs :)

Fair enough, will move it there. Thanks for spotting!

> > + * bq25792_read16() - Read a 16-bit value from device register
> > + * @pdata: driver platform data
> > + * @reg: register address to read from
> > + * @val: pointer to store the register value
> > + *
> > + * Read a 16-bit big-endian value from the BQ25792 device via regmap
> > + * and convert to CPU byte order.
> > + *
> > + * Return: Returns 0 on success or error on failure to read.
> > + */
> > +static int bq25792_read16(struct bq257xx_chg *pdata, unsigned int reg, u16 *val)
> > +{
> > +     __be16 regval;
> > +     int ret;
> > +
> > +     ret = regmap_raw_read(pdata->bq->regmap, reg, &regval, sizeof(regval));
> > +     if (ret)
> > +             return ret;
> > +
> > +     *val = be16_to_cpu(regval);
> > +     return 0;
> > +}
> > +
> > +/**
> > + * bq25792_write16() - Write a 16-bit value to device register
> > + * @pdata: driver platform data
> > + * @reg: register address to write to
> > + * @val: 16-bit value to write in CPU byte order
> > + *
> > + * Convert the value to big-endian and write a 16-bit value to the
> > + * BQ25792 device via regmap.
> > + *
> > + * Return: Returns 0 on success or error on failure to write.
> > + */
> > +static int bq25792_write16(struct bq257xx_chg *pdata, unsigned int reg, u16 val)
> > +{
> > +     __be16 regval = cpu_to_be16(val);
> > +     int ret;
> > +
> > +     ret = regmap_raw_write(pdata->bq->regmap, reg, &regval, sizeof(regval));
> > +     if (ret)
> > +             return ret;
> > +
> > +     return 0;
> > +}
>
> Are there big _and_ little endian registers on the bq25792? Otherwise I
> would expect this to be done by properly configuring regmap.

Oh it's such a pain actually. Most of its registers are 8-bit, but
select few are 16-bit big endian. So if one tries to configure the
regmap for 16-bit big-endian, it incorrectly accesses the 8-bit ones,
as the least significant byte is then not at the right address (and it
also spoils unrelated registers at write). Setting big endian with
8-bit registers in the regmap doesn't make much sense as there is no
endianness with bytes. So I ended up setting the regmap to 8-bit
registers without any endianness flags, and dealing with the
big-endian 16-bit ones manually as above.

If there is a more elegant way to do that please let me know, because
I'm not a fan of that manual endianness trickery myself.

Thanks a lot,
Alexey