linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Liam Breck <liam@networkimprov.net>
To: "Andrew F. Davis" <afd@ti.com>
Cc: Sebastian Reichel <sre@kernel.org>,
	linux-pm@vger.kernel.org,
	Matt Ranostay <matt@ranostay.consulting>,
	Liam Breck <kernel@networkimprov.net>
Subject: Re: [PATCH v11 07/10] power: bq27xxx_battery: Keep track of specific chip id
Date: Thu, 23 Mar 2017 14:44:58 -0700	[thread overview]
Message-ID: <CAKvHMgR=bcw2kO_8QLQfyAm241Ynk2woMw81pPnVGUDCu7vMBA@mail.gmail.com> (raw)
In-Reply-To: <f8e49417-ea2a-3486-26c2-63eb3306990a@ti.com>

On Thu, Mar 23, 2017 at 7:49 AM, Andrew F. Davis <afd@ti.com> wrote:
> On 03/23/2017 05:35 AM, Liam Breck wrote:
>> On Thu, Mar 23, 2017 at 3:28 AM, Sebastian Reichel <sre@kernel.org> wrote:
>>> Hi,
>>>
>>> On Mon, Mar 20, 2017 at 02:43:32AM -0700, Liam Breck wrote:
>>>> From: Liam Breck <kernel@networkimprov.net>
>>>>
>>>> Pass actual chip ID into _setup(), which translates it to a group ID,
>>>> to allow support for all chips by the power_supply_battery_info code.
>>>> There are no functional changes to the driver.
>>>>
>>>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>>>
>>> This is really ugly. If we need chip ID and group ID let's store
>>> them in different variables. For example put the detailed chipid
>>> into di->realchip and then do
>>
>> I tried this in a previous version, and Andrew rejected it as
>> confusing to have two IDs. That was while you were away on business
>> :-)
>>
>
> I rejected two tables, to indexes is what I've been pushing for a while.

On March 9, you wrote, "each chip should have one unique ID, I think
what ever way you prefer would work for the table structure, lets just
keep the chip ID enums clean"

Anyway, we have a solution - realid via argument.

>>> switch(di->realchip) {
>>> case FOO:
>>>     di->chip = GRP_FOO;
>>>     break;
>>> case BAR:
>>>     di->chip = GRP_BAR;
>>>     break;
>>> default:
>>>     di->chip = di->realchip;
>>>     break;
>>> }
>>>
>>> -- Sebastian
>>>
>>>>  drivers/power/supply/bq27xxx_battery.c     | 27 +++++++++++++++++++++++++++
>>>>  drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++++++++--------
>>>>  include/linux/power/bq27xxx_battery.h      | 11 +++++++++++
>>>>  3 files changed, 46 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>>>> index f36a6f1..db1a388 100644
>>>> --- a/drivers/power/supply/bq27xxx_battery.c
>>>> +++ b/drivers/power/supply/bq27xxx_battery.c
>>>> @@ -1391,6 +1391,33 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>>>>       struct power_supply_desc *psy_desc;
>>>>       struct power_supply_config psy_cfg = { .drv_data = di, };
>>>>
>>>> +     switch(di->chip) {
>>>> +     case BQ27000:
>>>> +     case BQ27010:
>>>> +     case BQ2750X:
>>>> +     case BQ2751X:
>>>> +     case BQ27500:
>>>> +     case BQ27510G1:
>>>> +     case BQ27510G2:
>>>> +     case BQ27510G3:
>>>> +     case BQ27520G1:
>>>> +     case BQ27520G2:
>>>> +     case BQ27520G3:
>>>> +     case BQ27520G4:
>>>> +     case BQ27530:
>>>> +     case BQ27541:
>>>> +     case BQ27545:
>>>> +     case BQ27421: break;
>>>> +     case BQ2752X: di->chip = BQ2751X; break;
>>>> +     case BQ27531: di->chip = BQ27530; break;
>>>> +     case BQ27542: di->chip = BQ27541; break;
>>>> +     case BQ27546: di->chip = BQ27541; break;
>>>> +     case BQ27742: di->chip = BQ27541; break;
>>>> +     case BQ27425: di->chip = BQ27421; break;
>>>> +     case BQ27441: di->chip = BQ27421; break;
>>>> +     case BQ27621: di->chip = BQ27421; break;
>>>> +     }
>>>> +
>>>>       INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>>>>       mutex_init(&di->lock);
>>>>       di->regs = bq27xxx_regs[di->chip];
>>>> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
>>>> index c68fbc3..b3f2494 100644
>>>> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
>>>> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
>>>> @@ -150,7 +150,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 },
>>>> @@ -160,16 +160,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 c3369fa..96cec17 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() */
>>>> +     BQ2752X, /* deprecated alias */
>>>> +     BQ27531,
>>>> +     BQ27542,
>>>> +     BQ27546,
>>>> +     BQ27742,
>>>> +     BQ27425,
>>>> +     BQ27441,
>>>> +     BQ27621,
>>>>  };
>>>>
>>>>  /**
>>>> --
>>>> 2.9.3
>>>>

  reply	other threads:[~2017-03-23 21:45 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-20  9:43 [PATCH v11 0/10] devicetree battery support and client bq27xxx_battery Liam Breck
2017-03-20  9:43 ` [PATCH v11 02/10] devicetree: property-units: Add uWh and uAh units Liam Breck
2017-05-01 15:06   ` Sebastian Reichel
     [not found] ` <20170320094335.19224-1-liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org>
2017-03-20  9:43   ` [PATCH v11 01/10] devicetree: power: Add battery.txt Liam Breck
2017-03-24 15:55     ` Rob Herring
2017-03-24 21:11       ` Liam Breck
     [not found]     ` <20170320094335.19224-2-liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org>
2017-03-23 10:20       ` Sebastian Reichel
2017-03-23 10:30         ` Liam Breck
2017-03-23 12:18           ` Sebastian Reichel
2017-04-07 19:23       ` Liam Breck
     [not found]         ` <CAKvHMgTxRJXgfpDgqQ_+=ErFEZAh9CodjVn5ZOoCfqPqSZWetA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-14  0:33           ` Sebastian Reichel
2017-05-01 15:09       ` Sebastian Reichel
2017-03-20  9:43   ` [PATCH v11 03/10] devicetree: power: bq27xxx: Add monitored-battery documentation Liam Breck
2017-05-01 15:10     ` Sebastian Reichel
2017-03-20  9:43 ` [PATCH v11 04/10] power: power_supply: Add power_supply_battery_info and API Liam Breck
2017-03-20 20:59   ` Liam Breck
2017-03-23 10:17     ` Sebastian Reichel
2017-03-25 23:19       ` Liam Breck
2017-03-29 13:50         ` Sebastian Reichel
2017-05-01 15:11   ` Sebastian Reichel
2017-03-20  9:43 ` [PATCH v11 05/10] power: bq27xxx_battery: Make error reporting consistent Liam Breck
2017-03-20 16:57   ` Andrew F. Davis
2017-03-20 17:59     ` Liam Breck
2017-03-23 10:11       ` Sebastian Reichel
2017-03-23 10:24         ` Liam Breck
2017-03-23 10:33           ` Sebastian Reichel
2017-03-23 12:49             ` Liam Breck
2017-03-20  9:43 ` [PATCH v11 06/10] power: bq27xxx_battery: Define access methods to write chip registers Liam Breck
2017-03-21 20:54   ` kbuild test robot
2017-03-22 19:56   ` Andrew F. Davis
2017-03-20  9:43 ` [PATCH v11 07/10] power: bq27xxx_battery: Keep track of specific chip id Liam Breck
2017-03-23 10:28   ` Sebastian Reichel
2017-03-23 10:35     ` Liam Breck
2017-03-23 12:30       ` Sebastian Reichel
2017-03-23 12:39         ` Liam Breck
2017-03-23 13:06           ` Sebastian Reichel
2017-03-23 14:49       ` Andrew F. Davis
2017-03-23 21:44         ` Liam Breck [this message]
2017-03-24 11:48           ` Andrew F. Davis
2017-03-24 12:20             ` Liam Breck
2017-03-20  9:43 ` [PATCH v11 08/10] power: bq27xxx_battery: Add power_supply_battery_info support Liam Breck
2017-03-20  9:43 ` [PATCH v11 09/10] power: bq27xxx_battery_i2c: Add I2C bulk read/write functions Liam Breck
2017-03-20  9:43 ` [PATCH v11 10/10] power: bq27xxx_battery: Remove duplicate register arrays Liam Breck
     [not found]   ` <CAKvHMgS9ajgE3ZpseSkT=BiW5yBL03s-8XNFeHE=V0gz7W7x3w@mail.gmail.com>
2017-03-21  8:58     ` Fwd: " 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='CAKvHMgR=bcw2kO_8QLQfyAm241Ynk2woMw81pPnVGUDCu7vMBA@mail.gmail.com' \
    --to=liam@networkimprov.net \
    --cc=afd@ti.com \
    --cc=kernel@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;
as well as URLs for NNTP newsgroup(s).