| 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
| ||
|
Date: Fri, 19 Mar 2021 10:16:51 +0000
From: LI Qingwu <qing-wu.li@...ca-geosystems.com.cn>
To: Sebastian Reichel <sre@...nel.org>
CC: "robh+dt@...nel.org" <robh+dt@...nel.org>,
"pali@...nel.org" <pali@...nel.org>,
"krzk@...nel.org" <krzk@...nel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
TERTYCHNYI Grygorii <grygorii.tertychnyi@...ca-geosystems.com>,
ZHIZHIKIN Andrey <andrey.zhizhikin@...ca-geosystems.com>
Subject: RE: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
Hi Sebastian,
About 4mA current, it's not the expected current, since we are working remotely, the instrument was unattended.
With the real current, the battery will be empty in a working day, so we just connect a dummy load for test for a while,
You mentioned the POWER_AVG looks suspicious, yes, it's due to I didn't pick the all the commits from master into my code, after pick it's looks correct.
About " Discharging " for idle battery, I picked you change, and it is "Not Charging", yes fixed by you!
One question, after I pick all the commits, the current goes to negative during discharging, is this correct?
POWER_SUPPLY_NAME=bq78z100-0
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=3386000
POWER_SUPPLY_CURRENT_NOW=-5000
POWER_SUPPLY_CAPACITY=27
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=269
POWER_SUPPLY_TIME_TO_EMPTY_NOW=1249920
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=6494000
POWER_SUPPLY_CHARGE_NOW=1736000
POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
POWER_SUPPLY_CYCLE_COUNT=1
POWER_SUPPLY_POWER_AVG=-20000
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
Best Regards
Li Qingwu (Terry)
-----Original Message-----
From: Sebastian Reichel <sre@...nel.org>
Sent: Wednesday, March 17, 2021 9:32 PM
To: LI Qingwu <qing-wu.li@...ca-geosystems.com.cn>
Cc: robh+dt@...nel.org; pali@...nel.org; krzk@...nel.org; linux-pm@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; TERTYCHNYI Grygorii <grygorii.tertychnyi@...ca-geosystems.com>; ZHIZHIKIN Andrey <andrey.zhizhikin@...ca-geosystems.com>
Subject: Re: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
Hi,
On Wed, Mar 17, 2021 at 10:49:18AM +0000, LI Qingwu wrote:
> Add support for TI BQ78Z100, I2C interface gas gauge.
> It provides a fully integrated safety protection and authentication
> for 1 to 2-series cell Li-Ion and Li-Polymer battery packs.
>
> The patch was tested with BQ78Z100 equipment.
>
> Signed-off-by: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
> Reviewed-by: Krzysztof Kozlowski <krzk@...nel.org>
>
> result of cat "/sys/class/power_supply/<bat_name>/uevent"
>
> CASE I: Discharging current>0:
> POWER_SUPPLY_NAME=bq78z100-0
> POWER_SUPPLY_STATUS=Discharging
> POWER_SUPPLY_PRESENT=1
> POWER_SUPPLY_VOLTAGE_NOW=3405000
> POWER_SUPPLY_CURRENT_NOW=4000
4mA @ 3.4V is 13.6 mW, which seems really small to me.
Is this what you expect for your hardware?
> POWER_SUPPLY_CAPACITY=28
> POWER_SUPPLY_CAPACITY_LEVEL=Normal
> POWER_SUPPLY_TEMP=259
> POWER_SUPPLY_TIME_TO_EMPTY_NOW=1611000
I guess 18 days TTE is expected with such a small discharge rate.
> POWER_SUPPLY_TECHNOLOGY=Li-ion
> POWER_SUPPLY_CHARGE_FULL=6494000
> POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
> POWER_SUPPLY_CYCLE_COUNT=1
> POWER_SUPPLY_ENERGY_NOW=0
You are not feeding ENERGY_NOW with data, so do not expose it.
> POWER_SUPPLY_POWER_AVG=65535
That's a signed int16 -1 and looks suspicious. Especially since expected power average is around 13.6 mW. Is something wrong with the handling of BQ27XXX_REG_AP for your chip?
> POWER_SUPPLY_HEALTH=Good
> POWER_SUPPLY_MANUFACTURER=Texas Instruments
>
> CASE II : No discharging current:
> POWER_SUPPLY_NAME=bq78z100-0
> POWER_SUPPLY_STATUS=Discharging
That should actually be "Not Charging" for an idle battery. I suppose recent changes to bq27000, which I applied in the last few days to my for-next branch, might fix this.
> POWER_SUPPLY_PRESENT=1
> POWER_SUPPLY_VOLTAGE_NOW=3405000
> POWER_SUPPLY_CURRENT_NOW=0
> POWER_SUPPLY_CAPACITY=28
> POWER_SUPPLY_CAPACITY_LEVEL=Normal
> POWER_SUPPLY_TEMP=260
> POWER_SUPPLY_TECHNOLOGY=Li-ion
> POWER_SUPPLY_CHARGE_FULL=6494000
> POWER_SUPPLY_CHARGE_FULL_DESIGN=6000000
> POWER_SUPPLY_CYCLE_COUNT=1
> POWER_SUPPLY_ENERGY_NOW=0
> POWER_SUPPLY_POWER_AVG=0
> POWER_SUPPLY_HEALTH=Good
> POWER_SUPPLY_MANUFACTURER=Texas Instruments
> ---
You should expose POWER_SUPPLY_PROP_CHARGE_NOW for exposing BQ27XXX_REG_RC info, which is more precise than CAPACITY.
Thanks,
-- Sebastian
> drivers/power/supply/bq27xxx_battery.c | 44 ++++++++++++++++++++++
> drivers/power/supply/bq27xxx_battery_i2c.c | 2 +
> include/linux/power/bq27xxx_battery.h | 1 +
> 3 files changed, 47 insertions(+)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c
> b/drivers/power/supply/bq27xxx_battery.c
> index 4c4a7b1c64c5..05a4f9190160 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -39,6 +39,7 @@
> * https://www.ti.com/product/bq27z561
> * https://www.ti.com/product/bq28z610
> * https://www.ti.com/product/bq34z100-g1
> + * https://www.ti.com/product/bq78z100
> */
>
> #include <linux/device.h>
> @@ -515,6 +516,27 @@ static u8
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x22,
> BQ27XXX_DM_REG_ROWS,
> + },
> + bq78z100_regs[BQ27XXX_REG_MAX] = {
> + [BQ27XXX_REG_CTRL] = 0x00,
> + [BQ27XXX_REG_TEMP] = 0x06,
> + [BQ27XXX_REG_INT_TEMP] = 0x28,
> + [BQ27XXX_REG_VOLT] = 0x08,
> + [BQ27XXX_REG_AI] = 0x14,
> + [BQ27XXX_REG_FLAGS] = 0x0a,
> + [BQ27XXX_REG_TTE] = 0x16,
> + [BQ27XXX_REG_TTF] = 0x18,
> + [BQ27XXX_REG_TTES] = 0x1c,
> + [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] = INVALID_REG_ADDR,
> + [BQ27XXX_REG_SOC] = 0x2c,
> + [BQ27XXX_REG_DCAP] = 0x3c,
> + [BQ27XXX_REG_AP] = 0x22,
> + BQ27XXX_DM_REG_ROWS,
> };
>
> static enum power_supply_property bq27000_props[] = { @@ -813,6
> +835,26 @@ static enum power_supply_property bq34z100_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> +static enum power_supply_property bq78z100_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CURRENT_NOW,
> + POWER_SUPPLY_PROP_CAPACITY,
> + POWER_SUPPLY_PROP_CAPACITY_LEVEL,
> + POWER_SUPPLY_PROP_TEMP,
> + POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
> + POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
> + POWER_SUPPLY_PROP_TECHNOLOGY,
> + POWER_SUPPLY_PROP_CHARGE_FULL,
> + POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> + POWER_SUPPLY_PROP_CYCLE_COUNT,
> + POWER_SUPPLY_PROP_ENERGY_NOW,
> + POWER_SUPPLY_PROP_POWER_AVG,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> struct bq27xxx_dm_reg {
> u8 subclass_id;
> u8 offset;
> @@ -911,6 +953,7 @@ static struct bq27xxx_dm_reg bq27621_dm_regs[] = {
> #define bq27z561_dm_regs 0 #define bq28z610_dm_regs 0 #define
> bq34z100_dm_regs 0
> +#define bq78z100_dm_regs 0
>
> #define BQ27XXX_O_ZERO BIT(0)
> #define BQ27XXX_O_OTDC BIT(1) /* has OTC/OTD overtemperature flags */
> @@ -969,6 +1012,7 @@ static struct {
> [BQ28Z610] = BQ27XXX_DATA(bq28z610, 0 , BQ27Z561_O_BITS),
> [BQ34Z100] = BQ27XXX_DATA(bq34z100, 0 , BQ27XXX_O_OTDC | BQ27XXX_O_SOC_SI | \
> BQ27XXX_O_HAS_CI | BQ27XXX_O_MUL_CHEM),
> + [BQ78Z100] = BQ27XXX_DATA(bq78z100, 0x00000000, BQ27Z561_O_BITS),
> };
>
> static DEFINE_MUTEX(bq27xxx_list_lock); diff --git
> a/drivers/power/supply/bq27xxx_battery_i2c.c
> b/drivers/power/supply/bq27xxx_battery_i2c.c
> index eb4f4284982f..46f078350fd3 100644
> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
> @@ -248,6 +248,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
> { "bq27z561", BQ27Z561 },
> { "bq28z610", BQ28Z610 },
> { "bq34z100", BQ34Z100 },
> + { "bq78z100", BQ78Z100 },
> {},
> };
> MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table); @@ -284,6 +285,7 @@
> static const struct of_device_id bq27xxx_battery_i2c_of_match_table[] = {
> { .compatible = "ti,bq27z561" },
> { .compatible = "ti,bq28z610" },
> { .compatible = "ti,bq34z100" },
> + { .compatible = "ti,bq78z100" },
> {},
> };
> MODULE_DEVICE_TABLE(of, bq27xxx_battery_i2c_of_match_table);
> diff --git a/include/linux/power/bq27xxx_battery.h
> b/include/linux/power/bq27xxx_battery.h
> index 111a40d0d3d5..ac17618043b1 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -33,6 +33,7 @@ enum bq27xxx_chip {
> BQ27Z561,
> BQ28Z610,
> BQ34Z100,
> + BQ78Z100,
> };
>
> struct bq27xxx_device_info;
> --
> 2.17.1
>
Powered by blists - more mailing lists