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

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 4376 bytes --]

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@hotmail.com>
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---
>  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 :)

> +/**
> + * 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.

> [...]

The remaining part looks fine.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]