2021-03-17 10:51:08

by LI Qingwu

[permalink] [raw]
Subject: [PATCH V6 0/2] power: bq27xxx: add bq78z100

Changes in V6:

Reword the commit message,
add result of cat "/sys/class/power_supply/<bat_name>/uevent"


LI Qingwu (2):
dt-bindings: power: bq27xxx: add bq78z100
power: supply: bq27xxx: Add support for BQ78Z100

.../bindings/power/supply/bq27xxx.yaml | 1 +
drivers/power/supply/bq27xxx_battery.c | 44 +++++++++++++++++++
drivers/power/supply/bq27xxx_battery_i2c.c | 2 +
include/linux/power/bq27xxx_battery.h | 1 +
4 files changed, 48 insertions(+)

--
2.17.1


2021-03-17 10:53:14

by LI Qingwu

[permalink] [raw]
Subject: [PATCH V6 1/2] dt-bindings: power: bq27xxx: add bq78z100

Add bindings for TI BQ78Z100. An 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.

Signed-off-by: LI Qingwu <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/power/supply/bq27xxx.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.yaml b/Documentation/devicetree/bindings/power/supply/bq27xxx.yaml
index 45beefccf31a..712e974b28b6 100644
--- a/Documentation/devicetree/bindings/power/supply/bq27xxx.yaml
+++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.yaml
@@ -52,6 +52,7 @@ properties:
- ti,bq27z561
- ti,bq28z610
- ti,bq34z100
+ - ti,bq78z100

reg:
maxItems: 1
--
2.17.1

2021-03-17 10:53:29

by LI Qingwu

[permalink] [raw]
Subject: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100

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 <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

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
POWER_SUPPLY_CAPACITY=28
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=259
POWER_SUPPLY_TIME_TO_EMPTY_NOW=1611000
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=65535
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments

CASE II : No discharging current:
POWER_SUPPLY_NAME=bq78z100-0
POWER_SUPPLY_STATUS=Discharging
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
---
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

2021-03-17 13:34:22

by Sebastian Reichel

[permalink] [raw]
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 <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> 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
>


Attachments:
(No filename) (6.83 kB)
signature.asc (849.00 B)
Download all attachments

2021-03-19 10:19:59

by LI Qingwu

[permalink] [raw]
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 <[email protected]>
Sent: Wednesday, March 17, 2021 9:32 PM
To: LI Qingwu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; TERTYCHNYI Grygorii <[email protected]>; ZHIZHIKIN Andrey <[email protected]>
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 <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> 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
>

2021-03-19 11:28:24

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100

Hi,

On Fri, Mar 19, 2021 at 10:16:51AM +0000, LI Qingwu wrote:
> 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,

IIUIC 4-5 mA is the expected current for your dummy load and
the test data looks ok?

> 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.

Ok.

> About " Discharging " for idle battery, I picked you change, and
> it is "Not Charging", yes fixed by you!

Great.

> One question, after I pick all the commits, the current goes to
> negative during discharging, is this correct?

Yes, as documented in 05f94eb98907 ("power: supply: document current
direction") the current should be negative when battery discharges
and positive when it charges.

> 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

That data looks consistent to me now.

Last but not least: Please don't top-post in kernel mailing
lists and use inline style instead, as stated in
Documentation/process/howto.rst:

> Remember to keep the context and the attribution of your replies intact,
> keep the "John Kernelhacker wrote ...:" lines at the top of your reply, and
> add your statements between the individual quoted sections instead of
> writing at the top of the mail.

Thanks,

-- Sebastian

> -----Original Message-----
> 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 <[email protected]>
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> >
> > 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
> >


Attachments:
(No filename) (9.26 kB)
signature.asc (849.00 B)
Download all attachments

2021-03-22 03:02:05

by LI Qingwu

[permalink] [raw]
Subject: RE: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100

Hi Sebastian,



On Friday, March 19, 2021 7:27 PM Sebastian Reichel wrote:
> Hi,
>
> On Fri, Mar 19, 2021 at 10:16:51AM +0000, LI Qingwu wrote:
> > 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,
>
> IIUIC 4-5 mA is the expected current for your dummy load and the test data
> looks ok?

Yes, the data is correct, we tested several different current and data are correct,
confirmed by our electronics engineer.

>
> > 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.
>
> Ok.
>
> > About " Discharging " for idle battery, I picked you change, and it is
> > "Not Charging", yes fixed by you!
>
> Great.
>
> > One question, after I pick all the commits, the current goes to
> > negative during discharging, is this correct?
>
> Yes, as documented in 05f94eb98907 ("power: supply: document current
> direction") the current should be negative when battery discharges and positive
> when it charges.

Thanks!

>
> > 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
>
> That data looks consistent to me now.
>
> Last but not least: Please don't top-post in kernel mailing lists and use inline style
> instead, as stated in
> Documentation/process/howto.rst:

Noted down, I will follow the documents, thanks.

> > Remember to keep the context and the attribution of your replies
> > intact, keep the "John Kernelhacker wrote ...:" lines at the top of
> > your reply, and add your statements between the individual quoted
> > sections instead of writing at the top of the mail.
>
> Thanks,
>
> -- Sebastian
>
> > -----Original Message-----
> > 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 <[email protected]>
> > > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > >
> > > 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
> > >