Re: [PATCH] bq27xxx: add voltage min design for bq27000 and bq27200

[Pali Rohár]

On Monday 25 November 2024 17:12:58 Sicelo A. Mhlongo wrote:
> The bq27x00 gauges have an EEPROM register which contains the value of
> the voltage that should be considered to be zero battery capacity. Expose
> this to userspace using the VOLTAGE_MIN_DESIGN property.
> 
> Tested on Nokia N900 with bq27200.
> 
> Signed-off-by: Sicelo A. Mhlongo <absicsz@xxxxxxxxx>

Looks good,

Acked-by: Pali Rohár <pali@xxxxxxxxxx>

> ---
>  drivers/power/supply/bq27xxx_battery.c | 39 +++++++++++++++++++++++++-
>  include/linux/power/bq27xxx_battery.h  |  1 +
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 40c5ac7a1118..90a5bccfc6b9 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -123,6 +123,7 @@ enum bq27xxx_reg_index {
>  	BQ27XXX_DM_BLOCK,	/* Data Block */
>  	BQ27XXX_DM_DATA,	/* Block Data */
>  	BQ27XXX_DM_CKSUM,	/* Block Data Checksum */
> +	BQ27XXX_REG_SEDVF,	/* End-of-discharge Voltage */
>  	BQ27XXX_REG_MAX,	/* sentinel */
>  };
>  
> @@ -159,6 +160,7 @@ static u8
>  		[BQ27XXX_DM_BLOCK] = INVALID_REG_ADDR,
>  		[BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
>  		[BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_SEDVF] = 0x77,
>  	},
>  	bq27010_regs[BQ27XXX_REG_MAX] = {
>  		[BQ27XXX_REG_CTRL] = 0x00,
> @@ -184,6 +186,7 @@ static u8
>  		[BQ27XXX_DM_BLOCK] = INVALID_REG_ADDR,
>  		[BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
>  		[BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_SEDVF] = 0x77,
>  	},
>  	bq2750x_regs[BQ27XXX_REG_MAX] = {
>  		[BQ27XXX_REG_CTRL] = 0x00,
> @@ -579,6 +582,7 @@ static enum power_supply_property bq27000_props[] = {
>  	POWER_SUPPLY_PROP_POWER_AVG,
>  	POWER_SUPPLY_PROP_HEALTH,
>  	POWER_SUPPLY_PROP_MANUFACTURER,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
>  };
>  
>  static enum power_supply_property bq27010_props[] = {
> @@ -599,6 +603,7 @@ static enum power_supply_property bq27010_props[] = {
>  	POWER_SUPPLY_PROP_CYCLE_COUNT,
>  	POWER_SUPPLY_PROP_HEALTH,
>  	POWER_SUPPLY_PROP_MANUFACTURER,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
>  };
>  
>  #define bq2750x_props bq27510g3_props
> @@ -2039,6 +2044,36 @@ static int bq27xxx_battery_voltage(struct bq27xxx_device_info *di,
>  	return 0;
>  }
>  
> +/*
> + * Return the design minimum battery Voltage in microvolts
> + * Or < 0 if something fails.
> + */
> +static int bq27xxx_battery_read_dmin_volt(struct bq27xxx_device_info *di,
> +					  union power_supply_propval *val)
> +{
> +	int volt;
> +
> +	/* We only have to read design minimum voltage once */
> +	if (di->voltage_min_design > 0) {
> +		val->intval = di->voltage_min_design;
> +		return 0;
> +	}
> +
> +	volt = bq27xxx_read(di, BQ27XXX_REG_SEDVF, true);
> +	if (volt < 0) {
> +		dev_err(di->dev, "error reading design min voltage\n");
> +		return volt;
> +	}
> +
> +	/* SEDVF = Design EDVF / 8 - 256 */
> +	val->intval = volt * 8000 + 2048000;
> +
> +	/* Save for later reads */
> +	di->voltage_min_design = val->intval;
> +
> +	return 0;
> +}
> +
>  static int bq27xxx_simple_value(int value,
>  				union power_supply_propval *val)
>  {
> @@ -2119,8 +2154,10 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
>  	 * power_supply_battery_info visible in sysfs.
>  	 */
>  	case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN:
> -	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
>  		return -EINVAL;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> +		ret = bq27xxx_battery_read_dmin_volt(di, val);
> +		break;
>  	case POWER_SUPPLY_PROP_CYCLE_COUNT:
>  		ret = bq27xxx_battery_read_cyct(di, val);
>  		break;
> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
> index 5180dc9f1706..6b190639b08e 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -61,6 +61,7 @@ struct bq27xxx_device_info {
>  	struct bq27xxx_access_methods bus;
>  	struct bq27xxx_reg_cache cache;
>  	int charge_design_full;
> +	int voltage_min_design;
>  	bool removed;
>  	unsigned long last_update;
>  	union power_supply_propval last_status;
> -- 
> 2.45.2
>