From: "Andrew F. Davis" <afd@ti.com>
To: Liam Breck <liam@networkimprov.net>
Cc: Sebastian Reichel <sre@kernel.org>,
linux-pm@vger.kernel.org,
Matt Ranostay <matt@ranostay.consulting>,
Liam Breck <kernel@networkimprov.net>
Subject: Re: [PATCH v8 7/9] power: bq27xxx_battery: Add power_supply_battery_info support
Date: Mon, 27 Feb 2017 15:47:46 -0600 [thread overview]
Message-ID: <880a1d69-dc06-b921-e0be-e3ebff75938d@ti.com> (raw)
In-Reply-To: <CAKvHMgSouKi=NU-R3fWXe7L-ckcTJMWiNLvi500jQfUxzdJtwQ@mail.gmail.com>
On 02/27/2017 03:35 PM, Liam Breck wrote:
> On Mon, Feb 27, 2017 at 1:21 PM, Andrew F. Davis <afd@ti.com> wrote:
>> On 02/27/2017 02:05 PM, Liam Breck wrote:
>>> On Mon, Feb 27, 2017 at 10:06 AM, Andrew F. Davis <afd@ti.com> wrote:
>>>> On 02/27/2017 01:11 AM, Liam Breck wrote:
>>>>> From: Liam Breck <kernel@networkimprov.net>
>>>>>
>>>>> Previously there was no way to configure chip registers in the event that the
>>>>> defaults didn't match the battery in question.
>>>>>
>>>>> BQ27xxx driver now calls power_supply_get_battery_info, checks the inputs,
>>>>> and writes battery data to chip RAM or non-volatile memory.
>>>>>
>>>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>>>>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>>>>> ---
>>>>> drivers/power/supply/bq27xxx_battery.c | 458 ++++++++++++++++++++++++++++++++-
>>>>> 1 file changed, 456 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>>>>> index 7475a5f..41d4ce7 100644
>>>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>>>> @@ -51,7 +51,7 @@
>>>>>
>>>>> #include <linux/power/bq27xxx_battery.h>
>>>>>
>>>>> -#define DRIVER_VERSION "1.2.0"
>>>>> +#define DRIVER_VERSION "1.3.0"
>>>>>
>>>>> #define BQ27XXX_MANUFACTURER "Texas Instruments"
>>>>>
>>>>> @@ -59,6 +59,7 @@
>>>>> #define BQ27XXX_FLAG_DSC BIT(0)
>>>>> #define BQ27XXX_FLAG_SOCF BIT(1) /* State-of-Charge threshold final */
>>>>> #define BQ27XXX_FLAG_SOC1 BIT(2) /* State-of-Charge threshold 1 */
>>>>> +#define BQ27XXX_FLAG_CFGUP BIT(5)
>>>>> #define BQ27XXX_FLAG_FC BIT(9)
>>>>> #define BQ27XXX_FLAG_OTD BIT(14)
>>>>> #define BQ27XXX_FLAG_OTC BIT(15)
>>>>> @@ -72,6 +73,11 @@
>>>>> #define BQ27000_FLAG_FC BIT(5)
>>>>> #define BQ27000_FLAG_CHGS BIT(7) /* Charge state flag */
>>>>>
>>>>> +/* control register params */
>>>>> +#define BQ27XXX_SEALED 0x20
>>>>> +#define BQ27XXX_SET_CFGUPDATE 0x13
>>>>> +#define BQ27XXX_SOFT_RESET 0x42
>>>>> +
>>>>> #define BQ27XXX_RS (20) /* Resistor sense mOhm */
>>>>> #define BQ27XXX_POWER_CONSTANT (29200) /* 29.2 µV^2 * 1000 */
>>>>> #define BQ27XXX_CURRENT_CONSTANT (3570) /* 3.57 µV * 1000 */
>>>>> @@ -102,6 +108,11 @@ enum bq27xxx_reg_index {
>>>>> BQ27XXX_REG_SOC, /* State-of-Charge */
>>>>> BQ27XXX_REG_DCAP, /* Design Capacity */
>>>>> BQ27XXX_REG_AP, /* Average Power */
>>>>> + BQ27XXX_DM_CTRL, /* BlockDataControl() */
>>>>
>>>> /* Block Data Control */
>>>>
>>>>> + BQ27XXX_DM_CLASS, /* DataClass() */
>>>>
>>>> /* Data Class */
>>>
>>> I used the terms from the docs, so a search will find meaningful results.
>>>
>>>>> + BQ27XXX_DM_BLOCK, /* DataBlock() */
>>>>> + BQ27XXX_DM_DATA, /* BlockData() */
>>>>> + BQ27XXX_DM_CKSUM, /* BlockDataChecksum() */
>>>>> BQ27XXX_REG_MAX, /* sentinel */
>>>>> };
>>>>>
>>>>> @@ -125,6 +136,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x0b,
>>>>> [BQ27XXX_REG_DCAP] = 0x76,
>>>>> [BQ27XXX_REG_AP] = 0x24,
>>>>> + [BQ27XXX_DM_CTRL] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CLASS] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_BLOCK] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
>>>>> },
>>>>> [BQ27010] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -144,6 +160,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x0b,
>>>>> [BQ27XXX_REG_DCAP] = 0x76,
>>>>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CTRL] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CLASS] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_BLOCK] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
>>>>> },
>>>>> [BQ27500] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -163,6 +184,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x2c,
>>>>> [BQ27XXX_REG_DCAP] = 0x3c,
>>>>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>> },
>>>>> [BQ27510] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -182,6 +208,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x20,
>>>>> [BQ27XXX_REG_DCAP] = 0x2e,
>>>>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>> },
>>>>> [BQ27530] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -201,6 +232,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x2c,
>>>>> [BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
>>>>> [BQ27XXX_REG_AP] = 0x24,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>> },
>>>>> [BQ27541] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -220,6 +256,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x2c,
>>>>> [BQ27XXX_REG_DCAP] = 0x3c,
>>>>> [BQ27XXX_REG_AP] = 0x24,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>> },
>>>>> [BQ27545] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -239,6 +280,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x2c,
>>>>> [BQ27XXX_REG_DCAP] = INVALID_REG_ADDR,
>>>>> [BQ27XXX_REG_AP] = 0x24,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>> },
>>>>> [BQ27421] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -258,6 +304,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x1c,
>>>>> [BQ27XXX_REG_DCAP] = 0x3c,
>>>>> [BQ27XXX_REG_AP] = 0x18,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>> },
>>>>> [BQ27425] = {
>>>>> [BQ27XXX_REG_CTRL] = 0x00,
>>>>> @@ -277,6 +328,11 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
>>>>> [BQ27XXX_REG_SOC] = 0x1c,
>>>>> [BQ27XXX_REG_DCAP] = 0x3c,
>>>>> [BQ27XXX_REG_AP] = 0x18,
>>>>> + [BQ27XXX_DM_CTRL] = 0x61,
>>>>> + [BQ27XXX_DM_CLASS] = 0x3e,
>>>>> + [BQ27XXX_DM_BLOCK] = 0x3f,
>>>>> + [BQ27XXX_DM_DATA] = 0x40,
>>>>> + [BQ27XXX_DM_CKSUM] = 0x60,
>>>>
>>>> That wasn't so painful was it :)
>>>
>>> It added 11% to the patch :-/
>>>
>>>>> },
>>>>> };
>>>>>
>>>>> @@ -452,6 +508,81 @@ static struct {
>>>>> static DEFINE_MUTEX(bq27xxx_list_lock);
>>>>> static LIST_HEAD(bq27xxx_battery_devices);
>>>>>
>>>>> +#define BQ27XXX_DM_SZ 32
>>>>> +
>>>>> +#define BQ27XXX_MSLEEP(i) usleep_range((i)*1000, (i)*1000+500)
>>>>> +
>>>>> +struct bq27xxx_dm_reg {
>>>>> + u8 subclass_id;
>>>>> + u8 offset;
>>>>> + u8 bytes;
>>>>> + u16 min, max;
>>>>> +};
>>>>> +
>>>>> +struct bq27xxx_dm_buf {
>>>>> + u8 class;
>>>>> + u8 block;
>>>>> + u8 a[BQ27XXX_DM_SZ];
>>>>> + bool full, updt;
>>>>> +};
>>>>> +
>>>>> +#define BQ27XXX_DM_BUF(di, i) { \
>>>>> + .class = bq27xxx_dm_regs[(di)->chip][i].subclass_id, \
>>>>> + .block = bq27xxx_dm_regs[(di)->chip][i].offset / BQ27XXX_DM_SZ, \
>>>>> +}
>>>>> +
>>>>> +static inline u16* bq27xxx_dm_buf_ptr(struct bq27xxx_dm_buf *buf,
>>>>> + struct bq27xxx_dm_reg *reg) {
>>>>> + if (buf->class == reg->subclass_id
>>>>> + && buf->block == reg->offset / BQ27XXX_DM_SZ)
>>>>> + return (u16*) (buf->a + reg->offset % BQ27XXX_DM_SZ);
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +enum bq27xxx_dm_reg_id {
>>>>> + BQ27XXX_DM_DESIGN_CAPACITY = 0,
>>>>> + BQ27XXX_DM_DESIGN_ENERGY,
>>>>> + BQ27XXX_DM_TERMINATE_VOLTAGE,
>>>>> +};
>>>>> +
>>>>> +static const char* bq27xxx_dm_reg_name[] = {
>>>>> + [BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
>>>>> + [BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
>>>>> + [BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage",
>>>>> +};
>>>>> +
>>>>> +static struct bq27xxx_dm_reg bq27425_dm_regs[] = {
>>>>> + [BQ27XXX_DM_DESIGN_CAPACITY] = { 82, 12, 2, 0, 32767 },
>>>>> + [BQ27XXX_DM_DESIGN_ENERGY] = { 82, 14, 2, 0, 32767 },
>>>>> + [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 82, 18, 2, 2800, 3700 },
>>>>> +};
>>>>> +
>>>>> +static struct bq27xxx_dm_reg bq27421_dm_regs[] = { /* not tested */
>>>>> + [BQ27XXX_DM_DESIGN_CAPACITY] = { 82, 10, 2, 0, 8000 },
>>>>> + [BQ27XXX_DM_DESIGN_ENERGY] = { 82, 12, 2, 0, 32767 },
>>>>> + [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 82, 16, 2, 2500, 3700 },
>>>>> +};
>>>>> +
>>>>> +//static struct bq27xxx_dm_reg bq27621_dm_regs[] = { /* not tested */
>>>>> +// [BQ27XXX_DM_DESIGN_CAPACITY] = { 82, 3, 2, 0, 8000 },
>>>>> +// [BQ27XXX_DM_DESIGN_ENERGY] = { 82, 5, 2, 0, 32767 },
>>>>> +// [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 82, 9, 2, 2500, 3700 },
>>>>> +//};
>>>>
>>>> I don't think this comment style is allowed.
>>>
>>> This is a to-do item.
>>>
>>>>> +
>>>>> +static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = {
>>>>> + [BQ27421] = bq27421_dm_regs, /* and BQ27441 */
>>>>> + [BQ27425] = bq27425_dm_regs,
>>>>> +// [BQ27621] = bq27621_dm_regs,
>>>>> +};
>>>>
>>>> I know it is not tested, but lets not comment it out, I'll do a round of
>>>> testing for this part when this series is ready.
>>>
>>> ID for 621 is not yet defined. We need an efficient way to add IDs.
>>> Perhaps via chip-id and group-id. Most current ID refs would be to
>>> group-id. I'd like to defer that, so we should only add inputs for the
>>> single-chip groups in this patchset: 500, 545, 425.
>>>
>>> We must NOT set one-time program (OTP) memory, mentioned in 421 & 441
>>> docs. It's not clear whether documented data-memory ops change OTP.
>>> Can you ask? The 425 has rewritable NVM. The 621 docs don't say. All
>>> the above have cfgupdate mode, which the rest of the family lacks.
>>>
>>
>> This would be a good question for e2e[0].
>>
>> [0] http://e2e.ti.com/support/power_management/battery_management/f/180
>
> Can I ask you to handle discussions with TI folks?
>
You can ask, doesn't mean I will :) but really I don't have any special
means of getting a hold of the gauge team that you don't have (except
maybe walking over to their cubes and bugging them).
We try to ask even internal inter-team questions on e2e, you can see
questions asked by TI employees to other employees on that site, the
idea is that if we have a problem, someday customers may have the same
problem, so it's best not no keep these answers buried in some internal
email chain. So the most I would do is post the same question to them on
e2e, you can avoid me being a middle-man by asking them directly.
Thanks,
Andrew
> Thanks :-)
>
next prev parent reply other threads:[~2017-02-27 21:48 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-27 7:11 [PATCH v8 0/9] devicetree battery support and client bq27xxx_battery Liam Breck
2017-02-27 7:11 ` [PATCH v8 2/9] devicetree: property-units: Add uWh and uAh units Liam Breck
[not found] ` <20170227071117.18934-1-liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org>
2017-02-27 7:11 ` [PATCH v8 1/9] devicetree: power: Add battery.txt Liam Breck
[not found] ` <20170227071117.18934-2-liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org>
2017-03-02 15:14 ` Rob Herring
2017-03-02 18:31 ` Liam Breck
2017-03-15 20:10 ` Rob Herring
[not found] ` <CAL_Jsq+8Y=GtyjVZA-suZLH+ySDshaoU9qh96BF+PeUYK3FZxg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-15 22:04 ` Liam Breck
2017-03-15 23:50 ` Rob Herring
[not found] ` <CAL_JsqJOFUSRY_KPzwNWrx4FF1_F6XNK8OfkqhqgBpxH9WrZeQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-16 6:45 ` Liam Breck
2017-03-16 13:31 ` Andrew F. Davis
[not found] ` <bb0928ce-6d29-3d09-2c5b-f4a084fe06e9-l0cyMroinI0@public.gmane.org>
2017-03-16 14:07 ` Liam Breck
[not found] ` <CAKvHMgQujE4uYNt0vPkt6qUOtVqdvzXs0zXTkDVRq9YLNvunBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-18 20:34 ` Rob Herring
2017-02-27 7:11 ` [PATCH v8 3/9] devicetree: power: bq27xxx: Add monitored-battery documentation Liam Breck
[not found] ` <20170227071117.18934-4-liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org>
2017-03-02 15:16 ` Rob Herring
2017-02-27 7:11 ` [PATCH v8 4/9] power: power_supply: Add power_supply_battery_info and API Liam Breck
2017-02-27 7:11 ` [PATCH v8 5/9] power: bq27xxx_battery: Define access methods to write chip registers Liam Breck
2017-02-27 7:11 ` [PATCH v8 6/9] power: bq27xxx_battery: Add BQ27425 chip id Liam Breck
2017-02-27 16:28 ` Andrew F. Davis
2017-02-27 7:11 ` [PATCH v8 7/9] power: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
2017-02-27 18:06 ` Andrew F. Davis
2017-02-27 20:05 ` Liam Breck
2017-02-27 21:21 ` Andrew F. Davis
2017-02-27 21:35 ` Liam Breck
2017-02-27 21:47 ` Andrew F. Davis [this message]
2017-02-27 22:14 ` Liam Breck
2017-02-27 22:37 ` Liam Breck
2017-03-01 23:09 ` Liam Breck
2017-03-03 21:36 ` Liam Breck
2017-03-03 21:51 ` Andrew F. Davis
2017-03-03 22:04 ` Liam Breck
2017-03-03 22:07 ` Andrew F. Davis
2017-03-03 22:13 ` Liam Breck
2017-03-06 16:42 ` Andrew F. Davis
2017-03-06 20:14 ` Liam Breck
2017-02-27 7:11 ` [PATCH v8 8/9] power: bq27xxx_battery: Add print_dm_blocks() to log chip memory Liam Breck
2017-02-27 18:07 ` Andrew F. Davis
2017-02-27 7:11 ` [PATCH v8 9/9] power: bq27xxx_battery_i2c: Add I2C bulk read/write functions Liam Breck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=880a1d69-dc06-b921-e0be-e3ebff75938d@ti.com \
--to=afd@ti.com \
--cc=kernel@networkimprov.net \
--cc=liam@networkimprov.net \
--cc=linux-pm@vger.kernel.org \
--cc=matt@ranostay.consulting \
--cc=sre@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox