From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liam Breck Subject: Re: [PATCH v12.2 8/9] power: bq27xxx_battery: Enable chip data memory update for certain chips Date: Tue, 4 Apr 2017 14:50:54 -0700 Message-ID: References: <20170330090216.12381-1-liam@networkimprov.net> <20170330090216.12381-5-liam@networkimprov.net> <38a00546-63b7-4deb-31c9-df70b3f7021b@ti.com> <3e4a3c30-70d2-bcd2-6ad2-13ab5f03fe1a@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-it0-f67.google.com ([209.85.214.67]:33210 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753834AbdDDVu4 (ORCPT ); Tue, 4 Apr 2017 17:50:56 -0400 Received: by mail-it0-f67.google.com with SMTP id w11so12414355itb.0 for ; Tue, 04 Apr 2017 14:50:55 -0700 (PDT) In-Reply-To: <3e4a3c30-70d2-bcd2-6ad2-13ab5f03fe1a@ti.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: "Andrew F. Davis" Cc: linux-pm@vger.kernel.org, Liam Breck On Tue, Apr 4, 2017 at 2:42 PM, Andrew F. Davis wrote: > On 04/04/2017 04:37 PM, Liam Breck wrote: >> On Tue, Apr 4, 2017 at 1:12 PM, Andrew F. Davis wrote: >>> On 03/30/2017 03:14 PM, Liam Breck wrote: >>>> On Thu, Mar 30, 2017 at 5:58 AM, Andrew F. Davis wrote: >>>>> On 03/30/2017 04:02 AM, Liam Breck wrote: >>>>>> From: Liam Breck >>>>>> >>>>>> Support data memory update of BQ27500, 545, 421, 425, 441, 621. >>>>>> >>>>>> Create IDs for for previously unID'd chips, to index arrays for unseal keys >>>>>> and data memory register tables. >>>>>> >>>>>> Signed-off-by: Liam Breck >>>>>> --- >>>>>> drivers/power/supply/bq27xxx_battery.c | 73 +++++++++++++++++++++++++++++- >>>>>> drivers/power/supply/bq27xxx_battery_i2c.c | 16 +++---- >>>>>> include/linux/power/bq27xxx_battery.h | 11 +++++ >>>>>> 3 files changed, 90 insertions(+), 10 deletions(-) >>>>>> >>>>>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c >>>>>> index 4c7a13e..70b8d1c 100644 >>>>>> --- a/drivers/power/supply/bq27xxx_battery.c >>>>>> +++ b/drivers/power/supply/bq27xxx_battery.c >>>>>> @@ -57,7 +57,7 @@ >>>>>> >>>>>> #include >>>>>> >>>>>> -#define DRIVER_VERSION "1.2.0" >>>>>> +#define DRIVER_VERSION "1.3.0" >>>>>> >>>>>> #define BQ27XXX_MANUFACTURER "Texas Instruments" >>>>>> >>>>>> @@ -894,6 +894,54 @@ static const char* bq27xxx_dm_reg_name[] = { >>>>>> [BQ27XXX_DM_TERMINATE_VOLTAGE] = "terminate-voltage", >>>>>> }; >>>>>> >>>>>> +static struct bq27xxx_dm_reg bq27500_dm_regs[] = { >>>>>> + [BQ27XXX_DM_DESIGN_CAPACITY] = { 48, 10, 2, 0, 65535 }, >>>>>> + [BQ27XXX_DM_DESIGN_ENERGY] = { }, /* missing on chip */ >>>>>> + [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 80, 48, 2, 1000, 32767 }, >>>>>> +}; >>>>>> + >>>>>> +static struct bq27xxx_dm_reg bq27545_dm_regs[] = { >>>>>> + [BQ27XXX_DM_DESIGN_CAPACITY] = { 48, 23, 2, 0, 32767 }, >>>>>> + [BQ27XXX_DM_DESIGN_ENERGY] = { 48, 25, 2, 0, 32767 }, >>>>>> + [BQ27XXX_DM_TERMINATE_VOLTAGE] = { 80, 67, 2, 2800, 3700 }, >>>>>> +}; >>>>>> + >>>>>> +static struct bq27xxx_dm_reg bq27421_dm_regs[] = { >>>>>> + [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 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 bq27621_dm_regs[] = { >>>>>> + [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 }, >>>>>> +}; >>>>>> + >>>>>> +static struct bq27xxx_dm_reg *bq27xxx_dm_regs[] = { >>>>>> + [BQ27500] = bq27500_dm_regs, >>>>>> + [BQ27545] = bq27545_dm_regs, >>>>>> + [BQ27421] = bq27421_dm_regs, >>>>>> + [BQ27425] = bq27425_dm_regs, >>>>>> + [BQ27441] = bq27421_dm_regs, >>>>>> + [BQ27621] = bq27621_dm_regs, >>>>>> +}; >>>>>> + >>>>>> +static u32 bq27xxx_unseal_keys[] = { >>>>>> + [BQ27500] = 0x04143672, >>>>>> + [BQ27545] = 0x04143672, >>>>>> + [BQ27421] = 0x80008000, >>>>>> + [BQ27425] = 0x04143672, >>>>>> + [BQ27441] = 0x80008000, >>>>>> + [BQ27621] = 0x80008000, >>>>>> +}; >>>>>> + >>>>>> >>>>>> static int poll_interval_param_set(const char *val, const struct kernel_param *kp) >>>>>> { >>>>>> @@ -1861,9 +1909,30 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di) >>>>>> .drv_data = di, >>>>>> }; >>>>>> >>>>>> + di->unseal_key = bq27xxx_unseal_keys[di->chip]; >>>>>> + di->dm_regs = bq27xxx_dm_regs[di->chip]; >>>>>> + >>>>>> + switch(di->chip) { /* translate members of categories */ >>>>>> + case BQ2752X: >>>>>> + di->chip = BQ27510G3; break; >>>>>> + case BQ27531: >>>>>> + di->chip = BQ27530; break; >>>>>> + case BQ27542: >>>>>> + case BQ27546: >>>>>> + case BQ27742: >>>>>> + di->chip = BQ27541; break; >>>>>> + case BQ27425: >>>>>> + case BQ27441: >>>>>> + case BQ27621: >>>>>> + di->chip = BQ27421; break; >>>>>> + default: >>>>>> + break; >>>>>> + } >>>>>> + >>>>>> + di->regs = bq27xxx_regs[di->chip]; >>>>>> + >>>>>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll); >>>>>> mutex_init(&di->lock); >>>>>> - di->regs = bq27xxx_regs[di->chip]; >>>>>> >>>>>> psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL); >>>>>> if (!psy_desc) >>>>>> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c >>>>>> index a597221..0b11ed4 100644 >>>>>> --- a/drivers/power/supply/bq27xxx_battery_i2c.c >>>>>> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c >>>>>> @@ -230,7 +230,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = { >>>>>> { "bq27210", BQ27010 }, >>>>>> { "bq27500", BQ2750X }, >>>>>> { "bq27510", BQ2751X }, >>>>>> - { "bq27520", BQ2751X }, >>>>>> + { "bq27520", BQ2752X }, >>>>>> { "bq27500-1", BQ27500 }, >>>>>> { "bq27510g1", BQ27510G1 }, >>>>>> { "bq27510g2", BQ27510G2 }, >>>>>> @@ -240,16 +240,16 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = { >>>>>> { "bq27520g3", BQ27520G3 }, >>>>>> { "bq27520g4", BQ27520G4 }, >>>>>> { "bq27530", BQ27530 }, >>>>>> - { "bq27531", BQ27530 }, >>>>>> + { "bq27531", BQ27531 }, >>>>>> { "bq27541", BQ27541 }, >>>>>> - { "bq27542", BQ27541 }, >>>>>> - { "bq27546", BQ27541 }, >>>>>> - { "bq27742", BQ27541 }, >>>>>> + { "bq27542", BQ27542 }, >>>>>> + { "bq27546", BQ27546 }, >>>>>> + { "bq27742", BQ27742 }, >>>>>> { "bq27545", BQ27545 }, >>>>>> { "bq27421", BQ27421 }, >>>>>> - { "bq27425", BQ27421 }, >>>>>> - { "bq27441", BQ27421 }, >>>>>> - { "bq27621", BQ27421 }, >>>>>> + { "bq27425", BQ27425 }, >>>>>> + { "bq27441", BQ27441 }, >>>>>> + { "bq27621", BQ27621 }, >>>>>> {}, >>>>>> }; >>>>>> MODULE_DEVICE_TABLE(i2c, bq27xxx_i2c_id_table); >>>>>> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h >>>>>> index 227eb08..014e59f 100644 >>>>>> --- a/include/linux/power/bq27xxx_battery.h >>>>>> +++ b/include/linux/power/bq27xxx_battery.h >>>>>> @@ -2,6 +2,7 @@ >>>>>> #define __LINUX_BQ27X00_BATTERY_H__ >>>>>> >>>>>> enum bq27xxx_chip { >>>>>> + /* categories; index for bq27xxx_regs[] */ >>>>>> BQ27000 = 1, /* bq27000, bq27200 */ >>>>>> BQ27010, /* bq27010, bq27210 */ >>>>>> BQ2750X, /* bq27500 deprecated alias */ >>>>>> @@ -18,6 +19,16 @@ enum bq27xxx_chip { >>>>>> BQ27541, /* bq27541, bq27542, bq27546, bq27742 */ >>>>>> BQ27545, /* bq27545 */ >>>>>> BQ27421, /* bq27421, bq27425, bq27441, bq27621 */ >>>>>> + >>>>>> + /* members of categories; translate these to category in _setup() */ >>>>> >>>>> These don't need to be separate from the others, these are valid chip >>>>> IDs, we may use another compatible ID for register array index right now >>>>> but these are not "categories". >>>> >>>> The above were indeed treated like categories, as the I2C ID table >>>> mapped other chips to them. We could call them category|class >>>> "leaders" (prototypes? models? patterns?) instead. >>>> >>>> The below are not valid in di->chip, they must be converted. So it >>>> makes sense to group them. >>>> >>> >>> They are only "not valid" because you did not add tables for them. If >>> you don't want to add register tables then think of a better way to map >>> chip to register table. This is overly complex and will cause problems >>> for people adding more chips. >> >> This very mapping from category member -> prototype ID was done by the >> I2C table previously. How is doing it in a setup() switch any more >> complex? >> >> The alternative mapping from chip to regs is this, which you said was >> there but replaced by a 2D array a while ago. Shall we revert to this? >> bq27xxx_regs[] = { >> [BQ27991] = bq27991_regs, // prototype >> [BQ27992] = bq27991_regs, // member >> ... >> }; >> > > It's either that or add a second index into di, up to you. The easiest > way is still, and always has been, to just add the extra tables. It > would get this series reviewed faster, and we can remove redundant ones > later when we pick a clean way of doing that. Oh, I think what you want is a new table mapping orig ID to chip ID, instead of a switch? >>>>>> + BQ2752X, /* deprecated alias */ >>>>>> + BQ27531, >>>>>> + BQ27542, >>>>>> + BQ27546, >>>>>> + BQ27742, >>>>>> + BQ27425, >>>>>> + BQ27441, >>>>>> + BQ27621, >>>>>> }; >>>>>> >>>>>> /** >>>>>>