lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c77d50a397054b20a4945140820a54e0@XBOX03.axis.com>
Date:   Thu, 17 Dec 2020 12:03:24 +0000
From:   Hermes Zhang <Hermes.Zhang@...s.com>
To:     Pali Rohár <pali@...nel.org>
CC:     Dan Murphy <dmurphy@...com>, Sebastian Reichel <sre@...nel.org>,
        kernel <kernel@...s.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for
 bq27z561/bq28z610/bq34z100

Hi Pali,

From the TI spec (e.g. https://www.ti.com/lit/ug/tidu077/tidu077.pdf?ts=1608206347022&ref_url=https%253A%252F%252Fwww.google.com%252F) , the NAC and RC (RemainingCapacity) are different:

4.5 NominalAvailableCapacity( ): 0x08/0x09
This read-only command pair returns the uncompensated (less than C/20 load) battery capacity
remaining. Units are mAh.

4.7 RemainingCapacity( ): 0x0c/0x0d
This read-only command pair returns the compensated battery capacity remaining. Units are mAh.

But for some chip e.g. bq27z561 it doesn't have the NAC Reg, so I prefer to use RC instead.

Best Regards,
Hermes

-----Original Message-----
From: Pali Rohár <pali@...nel.org> 
Sent: 2020年12月17日 19:57
To: Hermes Zhang <Hermes.Zhang@...s.com>
Cc: Dan Murphy <dmurphy@...com>; Sebastian Reichel <sre@...nel.org>; kernel <kernel@...s.com>; Hermes Zhang <Hermes.Zhang@...s.com>; linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: Re: [PATCH] power: supply: bq27xxx: Supporrt CHARGE_NOW for bq27z561/bq28z610/bq34z100

On Thursday 17 December 2020 19:47:37 Hermes Zhang wrote:
> From: Hermes Zhang <chenhuiz@...s.com>
> 
> The CHARGE_NOW is map to REG_NAC for all the gauge chips beofre. But 
> for some chips (e.g. bq27z561) which doesn't have the REG_NAC, we use 
> REG_RC (remaining capacity) for CHARGE_NOW.

Hello! What is the difference between NAC and RC? Is not it same thing?
I'm asking because for me from this patch for power supply API purpose it is the same thing... And therefore if it does not make sense to define for bq27z561 chip BQ27XXX_REG_NAC reg value to value which you used in BQ27XXX_REG_RC (to simplify whole implementation).

> Signed-off-by: Hermes Zhang <chenhuiz@...s.com>
> ---
>  drivers/power/supply/bq27xxx_battery.c | 35 
> +++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index 315e0909e6a4..c1a49a598e9b 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -110,6 +110,7 @@ enum bq27xxx_reg_index {
>  	BQ27XXX_REG_TTES,	/* Time-to-Empty Standby */
>  	BQ27XXX_REG_TTECP,	/* Time-to-Empty at Constant Power */
>  	BQ27XXX_REG_NAC,	/* Nominal Available Capacity */
> +	BQ27XXX_REG_RC,		/* Remaining Capacity */
>  	BQ27XXX_REG_FCC,	/* Full Charge Capacity */
>  	BQ27XXX_REG_CYCT,	/* Cycle Count */
>  	BQ27XXX_REG_AE,		/* Available Energy */
> @@ -145,6 +146,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = 0x26,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -169,6 +171,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = 0x26,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -193,6 +196,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1a,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -215,6 +219,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = 0x26,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -237,6 +242,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1a,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x1e,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -257,6 +263,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = 0x26,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -277,6 +284,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = 0x26,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -297,6 +305,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = 0x26,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -317,6 +326,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1c,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x1e,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -337,6 +347,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -361,6 +372,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -382,6 +394,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -405,6 +418,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x0c,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -425,6 +439,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = 0x08,
> +		[BQ27XXX_REG_RC] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_FCC] = 0x0e,
>  		[BQ27XXX_REG_CYCT] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_AE] = INVALID_REG_ADDR, @@ -450,6 +465,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_RC] = 0x10,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -470,6 +486,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_RC] = 0x10,
>  		[BQ27XXX_REG_FCC] = 0x12,
>  		[BQ27XXX_REG_CYCT] = 0x2a,
>  		[BQ27XXX_REG_AE] = 0x22,
> @@ -490,6 +507,7 @@ static u8
>  		[BQ27XXX_REG_TTES] = 0x1e,
>  		[BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
>  		[BQ27XXX_REG_NAC] = INVALID_REG_ADDR,
> +		[BQ27XXX_REG_RC] = 0x04,
>  		[BQ27XXX_REG_FCC] = 0x06,
>  		[BQ27XXX_REG_CYCT] = 0x2c,
>  		[BQ27XXX_REG_AE] = 0x24,
> @@ -745,6 +763,7 @@ static enum power_supply_property bq27z561_props[] = {
>  	POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
>  	POWER_SUPPLY_PROP_TECHNOLOGY,
>  	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
>  	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
>  	POWER_SUPPLY_PROP_CYCLE_COUNT,
>  	POWER_SUPPLY_PROP_POWER_AVG,
> @@ -764,6 +783,7 @@ static enum power_supply_property bq28z610_props[] = {
>  	POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
>  	POWER_SUPPLY_PROP_TECHNOLOGY,
>  	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
>  	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
>  	POWER_SUPPLY_PROP_CYCLE_COUNT,
>  	POWER_SUPPLY_PROP_POWER_AVG,
> @@ -784,6 +804,7 @@ static enum power_supply_property bq34z100_props[] = {
>  	POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
>  	POWER_SUPPLY_PROP_TECHNOLOGY,
>  	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
>  	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
>  	POWER_SUPPLY_PROP_CYCLE_COUNT,
>  	POWER_SUPPLY_PROP_ENERGY_NOW,
> @@ -1518,6 +1539,15 @@ static inline int bq27xxx_battery_read_nac(struct bq27xxx_device_info *di)
>  	return bq27xxx_battery_read_charge(di, BQ27XXX_REG_NAC);  }
>  
> +/*
> + * Return the battery Remaining Capacity in µAh
> + * Or < 0 if something fails.
> + */
> +static inline int bq27xxx_battery_read_rc(struct bq27xxx_device_info 
> +*di) {
> +	return bq27xxx_battery_read_charge(di, BQ27XXX_REG_RC); }
> +
>  /*
>   * Return the battery Full Charge Capacity in µAh
>   * Or < 0 if something fails.
> @@ -1965,7 +1995,10 @@ static int bq27xxx_battery_get_property(struct power_supply *psy,
>  			val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
>  		break;
>  	case POWER_SUPPLY_PROP_CHARGE_NOW:
> -		ret = bq27xxx_simple_value(bq27xxx_battery_read_nac(di), val);
> +		if (di->regs[BQ27XXX_REG_NAC] != INVALID_REG_ADDR)
> +			ret = bq27xxx_simple_value(bq27xxx_battery_read_nac(di), val);
> +		else
> +			ret = bq27xxx_simple_value(bq27xxx_battery_read_rc(di), val);
>  		break;
>  	case POWER_SUPPLY_PROP_CHARGE_FULL:
>  		ret = bq27xxx_simple_value(di->cache.charge_full, val);
> --
> 2.20.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ