* [PATCH V6 0/2] power: bq27xxx: add bq78z100
@ 2021-03-17 10:49 LI Qingwu
2021-03-17 10:49 ` [PATCH V6 1/2] dt-bindings: " LI Qingwu
2021-03-17 10:49 ` [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100 LI Qingwu
0 siblings, 2 replies; 7+ messages in thread
From: LI Qingwu @ 2021-03-17 10:49 UTC (permalink / raw)
To: sre, robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel
Cc: grygorii.tertychnyi, andrey.zhizhikin, LI Qingwu
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V6 1/2] dt-bindings: power: bq27xxx: add bq78z100
2021-03-17 10:49 [PATCH V6 0/2] power: bq27xxx: add bq78z100 LI Qingwu
@ 2021-03-17 10:49 ` LI Qingwu
2021-03-17 10:49 ` [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100 LI Qingwu
1 sibling, 0 replies; 7+ messages in thread
From: LI Qingwu @ 2021-03-17 10:49 UTC (permalink / raw)
To: sre, robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel
Cc: grygorii.tertychnyi, andrey.zhizhikin, LI Qingwu
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 <Qing-wu.Li@leica-geosystems.com.cn>
Acked-by: Rob Herring <robh@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
2021-03-17 10:49 [PATCH V6 0/2] power: bq27xxx: add bq78z100 LI Qingwu
2021-03-17 10:49 ` [PATCH V6 1/2] dt-bindings: " LI Qingwu
@ 2021-03-17 10:49 ` LI Qingwu
2021-03-17 13:32 ` Sebastian Reichel
1 sibling, 1 reply; 7+ messages in thread
From: LI Qingwu @ 2021-03-17 10:49 UTC (permalink / raw)
To: sre, robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel
Cc: grygorii.tertychnyi, andrey.zhizhikin, LI Qingwu
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@leica-geosystems.com.cn>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.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
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
2021-03-17 10:49 ` [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100 LI Qingwu
@ 2021-03-17 13:32 ` Sebastian Reichel
2021-03-19 10:16 ` LI Qingwu
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Reichel @ 2021-03-17 13:32 UTC (permalink / raw)
To: LI Qingwu
Cc: robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel,
grygorii.tertychnyi, andrey.zhizhikin
[-- Attachment #1: Type: text/plain, Size: 6995 bytes --]
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@leica-geosystems.com.cn>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
2021-03-17 13:32 ` Sebastian Reichel
@ 2021-03-19 10:16 ` LI Qingwu
2021-03-19 11:26 ` Sebastian Reichel
0 siblings, 1 reply; 7+ messages in thread
From: LI Qingwu @ 2021-03-19 10:16 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel,
TERTYCHNYI Grygorii, ZHIZHIKIN Andrey
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@kernel.org>
Sent: Wednesday, March 17, 2021 9:32 PM
To: LI Qingwu <qing-wu.li@leica-geosystems.com.cn>
Cc: robh+dt@kernel.org; pali@kernel.org; krzk@kernel.org; linux-pm@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; TERTYCHNYI Grygorii <grygorii.tertychnyi@leica-geosystems.com>; ZHIZHIKIN Andrey <andrey.zhizhikin@leica-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@leica-geosystems.com.cn>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
2021-03-19 10:16 ` LI Qingwu
@ 2021-03-19 11:26 ` Sebastian Reichel
2021-03-22 2:59 ` LI Qingwu
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Reichel @ 2021-03-19 11:26 UTC (permalink / raw)
To: LI Qingwu
Cc: robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel,
TERTYCHNYI Grygorii, ZHIZHIKIN Andrey
[-- Attachment #1: Type: text/plain, Size: 9480 bytes --]
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 <Qing-wu.Li@leica-geosystems.com.cn>
> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.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
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100
2021-03-19 11:26 ` Sebastian Reichel
@ 2021-03-22 2:59 ` LI Qingwu
0 siblings, 0 replies; 7+ messages in thread
From: LI Qingwu @ 2021-03-22 2:59 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh+dt, pali, krzk, linux-pm, devicetree, linux-kernel,
TERTYCHNYI Grygorii, ZHIZHIKIN Andrey
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 <Qing-wu.Li@leica-geosystems.com.cn>
> > > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.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
> > >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-22 3:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17 10:49 [PATCH V6 0/2] power: bq27xxx: add bq78z100 LI Qingwu
2021-03-17 10:49 ` [PATCH V6 1/2] dt-bindings: " LI Qingwu
2021-03-17 10:49 ` [PATCH V6 2/2] power: supply: bq27xxx: Add support for BQ78Z100 LI Qingwu
2021-03-17 13:32 ` Sebastian Reichel
2021-03-19 10:16 ` LI Qingwu
2021-03-19 11:26 ` Sebastian Reichel
2021-03-22 2:59 ` LI Qingwu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).