* [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table
2017-08-07 6:22 [RFC v2 0/5] bq27xxx_battery data memory update Liam Breck
@ 2017-08-07 6:22 ` Liam Breck
2017-08-09 16:08 ` Sebastian Reichel
2017-08-07 6:22 ` [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips Liam Breck
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-07 6:22 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohár, linux-pm
Cc: Paul Kocialkowski, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
To support new features which require different data for each chip, we
unify the bq27xxx_regs and bq27xxx_battery_props tables into a single one.
No functional changes to the driver.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 121 ++++++++++++++++-----------------
1 file changed, 60 insertions(+), 61 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index ed44439d..dd84e3d9 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -132,8 +132,8 @@ enum bq27xxx_reg_index {
[BQ27XXX_DM_CKSUM] = 0x60
/* Register mappings */
-static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
- [BQ27000] = {
+static u8
+ bq27000_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -157,7 +157,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
[BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
},
- [BQ27010] = {
+ bq27010_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -181,7 +181,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
[BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
},
- [BQ2750X] = {
+ bq2750x_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -201,7 +201,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ2751X] = {
+ bq2751x_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -221,7 +221,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27500] = {
+ bq27500_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -241,7 +241,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G1] = {
+ bq27510g1_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -261,7 +261,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G2] = {
+ bq27510g2_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -281,7 +281,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27510G3] = {
+ bq27510g3_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -301,7 +301,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G1] = {
+ bq27520g1_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
@@ -321,7 +321,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G2] = {
+ bq27520g2_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x36,
@@ -341,7 +341,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G3] = {
+ bq27520g3_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x36,
@@ -361,7 +361,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27520G4] = {
+ bq27520g4_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -381,7 +381,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27530] = {
+ bq27530_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x32,
@@ -401,7 +401,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27541] = {
+ bq27541_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -421,7 +421,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27545] = {
+ bq27545_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
[BQ27XXX_REG_INT_TEMP] = 0x28,
@@ -441,7 +441,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- [BQ27421] = {
+ bq27421_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x02,
[BQ27XXX_REG_INT_TEMP] = 0x1e,
@@ -460,10 +460,9 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_DCAP] = 0x3c,
[BQ27XXX_REG_AP] = 0x18,
BQ27XXX_DM_REG_ROWS,
- },
-};
+ };
-static enum power_supply_property bq27000_battery_props[] = {
+static enum power_supply_property bq27000_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -485,7 +484,7 @@ static enum power_supply_property bq27000_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27010_battery_props[] = {
+static enum power_supply_property bq27010_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -505,7 +504,7 @@ static enum power_supply_property bq27010_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq2750x_battery_props[] = {
+static enum power_supply_property bq2750x_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -523,7 +522,7 @@ static enum power_supply_property bq2750x_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq2751x_battery_props[] = {
+static enum power_supply_property bq2751x_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -541,7 +540,7 @@ static enum power_supply_property bq2751x_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27500_battery_props[] = {
+static enum power_supply_property bq27500_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -562,7 +561,7 @@ static enum power_supply_property bq27500_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g1_battery_props[] = {
+static enum power_supply_property bq27510g1_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -583,7 +582,7 @@ static enum power_supply_property bq27510g1_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g2_battery_props[] = {
+static enum power_supply_property bq27510g2_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -604,7 +603,7 @@ static enum power_supply_property bq27510g2_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27510g3_battery_props[] = {
+static enum power_supply_property bq27510g3_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -622,7 +621,7 @@ static enum power_supply_property bq27510g3_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g1_battery_props[] = {
+static enum power_supply_property bq27520g1_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -642,7 +641,7 @@ static enum power_supply_property bq27520g1_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g2_battery_props[] = {
+static enum power_supply_property bq27520g2_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -663,7 +662,7 @@ static enum power_supply_property bq27520g2_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g3_battery_props[] = {
+static enum power_supply_property bq27520g3_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -683,7 +682,7 @@ static enum power_supply_property bq27520g3_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27520g4_battery_props[] = {
+static enum power_supply_property bq27520g4_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -700,7 +699,7 @@ static enum power_supply_property bq27520g4_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27530_battery_props[] = {
+static enum power_supply_property bq27530_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -718,7 +717,7 @@ static enum power_supply_property bq27530_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27541_battery_props[] = {
+static enum power_supply_property bq27541_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -737,7 +736,7 @@ static enum power_supply_property bq27541_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27545_battery_props[] = {
+static enum power_supply_property bq27545_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -755,7 +754,7 @@ static enum power_supply_property bq27545_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq27421_battery_props[] = {
+static enum power_supply_property bq27421_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_VOLTAGE_NOW,
@@ -770,32 +769,32 @@ static enum power_supply_property bq27421_battery_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-#define BQ27XXX_PROP(_id, _prop) \
- [_id] = { \
- .props = _prop, \
- .size = ARRAY_SIZE(_prop), \
- }
+#define BQ27XXX_DATA(ref) { \
+ .regs = ref##_regs, \
+ .props = ref##_props, \
+ .props_size = ARRAY_SIZE(ref##_props) }
static struct {
+ u8 *regs;
enum power_supply_property *props;
- size_t size;
-} bq27xxx_battery_props[] = {
- BQ27XXX_PROP(BQ27000, bq27000_battery_props),
- BQ27XXX_PROP(BQ27010, bq27010_battery_props),
- BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
- BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
- BQ27XXX_PROP(BQ27500, bq27500_battery_props),
- BQ27XXX_PROP(BQ27510G1, bq27510g1_battery_props),
- BQ27XXX_PROP(BQ27510G2, bq27510g2_battery_props),
- BQ27XXX_PROP(BQ27510G3, bq27510g3_battery_props),
- BQ27XXX_PROP(BQ27520G1, bq27520g1_battery_props),
- BQ27XXX_PROP(BQ27520G2, bq27520g2_battery_props),
- BQ27XXX_PROP(BQ27520G3, bq27520g3_battery_props),
- BQ27XXX_PROP(BQ27520G4, bq27520g4_battery_props),
- BQ27XXX_PROP(BQ27530, bq27530_battery_props),
- BQ27XXX_PROP(BQ27541, bq27541_battery_props),
- BQ27XXX_PROP(BQ27545, bq27545_battery_props),
- BQ27XXX_PROP(BQ27421, bq27421_battery_props),
+ size_t props_size;
+} bq27xxx_chip_data[] = {
+ [BQ27000] = BQ27XXX_DATA(bq27000),
+ [BQ27010] = BQ27XXX_DATA(bq27010),
+ [BQ2750X] = BQ27XXX_DATA(bq2750x),
+ [BQ2751X] = BQ27XXX_DATA(bq2751x),
+ [BQ27500] = BQ27XXX_DATA(bq27500),
+ [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
+ [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
+ [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
+ [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
+ [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
+ [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
+ [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
+ [BQ27530] = BQ27XXX_DATA(bq27530),
+ [BQ27541] = BQ27XXX_DATA(bq27541),
+ [BQ27545] = BQ27XXX_DATA(bq27545),
+ [BQ27421] = BQ27XXX_DATA(bq27421),
};
static DEFINE_MUTEX(bq27xxx_list_lock);
@@ -1884,7 +1883,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
- di->regs = bq27xxx_regs[di->chip];
+ di->regs = bq27xxx_chip_data[di->chip].regs;
psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
if (!psy_desc)
@@ -1892,8 +1891,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
psy_desc->name = di->name;
psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
- psy_desc->properties = bq27xxx_battery_props[di->chip].props;
- psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
+ psy_desc->properties = bq27xxx_chip_data[di->chip].props;
+ psy_desc->num_properties = bq27xxx_chip_data[di->chip].props_size;
psy_desc->get_property = bq27xxx_battery_get_property;
psy_desc->external_power_changed = bq27xxx_external_power_changed;
--
2.13.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table
2017-08-07 6:22 ` [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table Liam Breck
@ 2017-08-09 16:08 ` Sebastian Reichel
2017-08-12 0:52 ` Liam Breck
0 siblings, 1 reply; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-09 16:08 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 14720 bytes --]
Hi,
On Sun, Aug 06, 2017 at 11:22:12PM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> To support new features which require different data for each chip, we
> unify the bq27xxx_regs and bq27xxx_battery_props tables into a single one.
>
> No functional changes to the driver.
>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
Looks fine to me.
-- Sebastian
> ---
> drivers/power/supply/bq27xxx_battery.c | 121 ++++++++++++++++-----------------
> 1 file changed, 60 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index ed44439d..dd84e3d9 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -132,8 +132,8 @@ enum bq27xxx_reg_index {
> [BQ27XXX_DM_CKSUM] = 0x60
>
> /* Register mappings */
> -static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> - [BQ27000] = {
> +static u8
> + bq27000_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -157,7 +157,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
> [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
> },
> - [BQ27010] = {
> + bq27010_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -181,7 +181,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_DM_DATA] = INVALID_REG_ADDR,
> [BQ27XXX_DM_CKSUM] = INVALID_REG_ADDR,
> },
> - [BQ2750X] = {
> + bq2750x_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -201,7 +201,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ2751X] = {
> + bq2751x_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -221,7 +221,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27500] = {
> + bq27500_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -241,7 +241,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27510G1] = {
> + bq27510g1_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -261,7 +261,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27510G2] = {
> + bq27510g2_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -281,7 +281,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27510G3] = {
> + bq27510g3_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -301,7 +301,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G1] = {
> + bq27520g1_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> @@ -321,7 +321,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G2] = {
> + bq27520g2_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x36,
> @@ -341,7 +341,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G3] = {
> + bq27520g3_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x36,
> @@ -361,7 +361,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27520G4] = {
> + bq27520g4_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -381,7 +381,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27530] = {
> + bq27530_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x32,
> @@ -401,7 +401,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27541] = {
> + bq27541_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -421,7 +421,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27545] = {
> + bq27545_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> [BQ27XXX_REG_INT_TEMP] = 0x28,
> @@ -441,7 +441,7 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - [BQ27421] = {
> + bq27421_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x02,
> [BQ27XXX_REG_INT_TEMP] = 0x1e,
> @@ -460,10 +460,9 @@ static u8 bq27xxx_regs[][BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_DCAP] = 0x3c,
> [BQ27XXX_REG_AP] = 0x18,
> BQ27XXX_DM_REG_ROWS,
> - },
> -};
> + };
>
> -static enum power_supply_property bq27000_battery_props[] = {
> +static enum power_supply_property bq27000_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -485,7 +484,7 @@ static enum power_supply_property bq27000_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27010_battery_props[] = {
> +static enum power_supply_property bq27010_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -505,7 +504,7 @@ static enum power_supply_property bq27010_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq2750x_battery_props[] = {
> +static enum power_supply_property bq2750x_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -523,7 +522,7 @@ static enum power_supply_property bq2750x_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq2751x_battery_props[] = {
> +static enum power_supply_property bq2751x_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -541,7 +540,7 @@ static enum power_supply_property bq2751x_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27500_battery_props[] = {
> +static enum power_supply_property bq27500_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -562,7 +561,7 @@ static enum power_supply_property bq27500_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27510g1_battery_props[] = {
> +static enum power_supply_property bq27510g1_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -583,7 +582,7 @@ static enum power_supply_property bq27510g1_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27510g2_battery_props[] = {
> +static enum power_supply_property bq27510g2_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -604,7 +603,7 @@ static enum power_supply_property bq27510g2_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27510g3_battery_props[] = {
> +static enum power_supply_property bq27510g3_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -622,7 +621,7 @@ static enum power_supply_property bq27510g3_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g1_battery_props[] = {
> +static enum power_supply_property bq27520g1_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -642,7 +641,7 @@ static enum power_supply_property bq27520g1_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g2_battery_props[] = {
> +static enum power_supply_property bq27520g2_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -663,7 +662,7 @@ static enum power_supply_property bq27520g2_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g3_battery_props[] = {
> +static enum power_supply_property bq27520g3_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -683,7 +682,7 @@ static enum power_supply_property bq27520g3_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27520g4_battery_props[] = {
> +static enum power_supply_property bq27520g4_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -700,7 +699,7 @@ static enum power_supply_property bq27520g4_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27530_battery_props[] = {
> +static enum power_supply_property bq27530_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -718,7 +717,7 @@ static enum power_supply_property bq27530_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27541_battery_props[] = {
> +static enum power_supply_property bq27541_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -737,7 +736,7 @@ static enum power_supply_property bq27541_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27545_battery_props[] = {
> +static enum power_supply_property bq27545_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -755,7 +754,7 @@ static enum power_supply_property bq27545_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq27421_battery_props[] = {
> +static enum power_supply_property bq27421_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> @@ -770,32 +769,32 @@ static enum power_supply_property bq27421_battery_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -#define BQ27XXX_PROP(_id, _prop) \
> - [_id] = { \
> - .props = _prop, \
> - .size = ARRAY_SIZE(_prop), \
> - }
> +#define BQ27XXX_DATA(ref) { \
> + .regs = ref##_regs, \
> + .props = ref##_props, \
> + .props_size = ARRAY_SIZE(ref##_props) }
>
> static struct {
> + u8 *regs;
> enum power_supply_property *props;
> - size_t size;
> -} bq27xxx_battery_props[] = {
> - BQ27XXX_PROP(BQ27000, bq27000_battery_props),
> - BQ27XXX_PROP(BQ27010, bq27010_battery_props),
> - BQ27XXX_PROP(BQ2750X, bq2750x_battery_props),
> - BQ27XXX_PROP(BQ2751X, bq2751x_battery_props),
> - BQ27XXX_PROP(BQ27500, bq27500_battery_props),
> - BQ27XXX_PROP(BQ27510G1, bq27510g1_battery_props),
> - BQ27XXX_PROP(BQ27510G2, bq27510g2_battery_props),
> - BQ27XXX_PROP(BQ27510G3, bq27510g3_battery_props),
> - BQ27XXX_PROP(BQ27520G1, bq27520g1_battery_props),
> - BQ27XXX_PROP(BQ27520G2, bq27520g2_battery_props),
> - BQ27XXX_PROP(BQ27520G3, bq27520g3_battery_props),
> - BQ27XXX_PROP(BQ27520G4, bq27520g4_battery_props),
> - BQ27XXX_PROP(BQ27530, bq27530_battery_props),
> - BQ27XXX_PROP(BQ27541, bq27541_battery_props),
> - BQ27XXX_PROP(BQ27545, bq27545_battery_props),
> - BQ27XXX_PROP(BQ27421, bq27421_battery_props),
> + size_t props_size;
> +} bq27xxx_chip_data[] = {
> + [BQ27000] = BQ27XXX_DATA(bq27000),
> + [BQ27010] = BQ27XXX_DATA(bq27010),
> + [BQ2750X] = BQ27XXX_DATA(bq2750x),
> + [BQ2751X] = BQ27XXX_DATA(bq2751x),
> + [BQ27500] = BQ27XXX_DATA(bq27500),
> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
> + [BQ27530] = BQ27XXX_DATA(bq27530),
> + [BQ27541] = BQ27XXX_DATA(bq27541),
> + [BQ27545] = BQ27XXX_DATA(bq27545),
> + [BQ27421] = BQ27XXX_DATA(bq27421),
> };
>
> static DEFINE_MUTEX(bq27xxx_list_lock);
> @@ -1884,7 +1883,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>
> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
> mutex_init(&di->lock);
> - di->regs = bq27xxx_regs[di->chip];
> + di->regs = bq27xxx_chip_data[di->chip].regs;
>
> psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
> if (!psy_desc)
> @@ -1892,8 +1891,8 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>
> psy_desc->name = di->name;
> psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> - psy_desc->properties = bq27xxx_battery_props[di->chip].props;
> - psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
> + psy_desc->properties = bq27xxx_chip_data[di->chip].props;
> + psy_desc->num_properties = bq27xxx_chip_data[di->chip].props_size;
> psy_desc->get_property = bq27xxx_battery_get_property;
> psy_desc->external_power_changed = bq27xxx_external_power_changed;
>
> --
> 2.13.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table
2017-08-09 16:08 ` Sebastian Reichel
@ 2017-08-12 0:52 ` Liam Breck
2017-08-12 16:39 ` Sebastian Reichel
0 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-12 0:52 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Wed, Aug 9, 2017 at 9:08 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> Hi,
>
> On Sun, Aug 06, 2017 at 11:22:12PM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> To support new features which require different data for each chip, we
>> unify the bq27xxx_regs and bq27xxx_battery_props tables into a single one.
>>
>> No functional changes to the driver.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>
> Looks fine to me.
Regarding the whole series? If so, I'll post a fully-tested "PATCH" version...
> -- Sebastian
>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 121 ++++++++++++++++-----------------
>> 1 file changed, 60 insertions(+), 61 deletions(-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table
2017-08-12 0:52 ` Liam Breck
@ 2017-08-12 16:39 ` Sebastian Reichel
0 siblings, 0 replies; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 16:39 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 796 bytes --]
Hi,
On Fri, Aug 11, 2017 at 05:52:50PM -0700, Liam Breck wrote:
> On Wed, Aug 9, 2017 at 9:08 AM, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
> > On Sun, Aug 06, 2017 at 11:22:12PM -0700, Liam Breck wrote:
> >> From: Liam Breck <kernel@networkimprov.net>
> >>
> >> To support new features which require different data for each chip, we
> >> unify the bq27xxx_regs and bq27xxx_battery_props tables into a single one.
> >>
> >> No functional changes to the driver.
> >>
> >> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> >
> > Looks fine to me.
>
> Regarding the whole series? If so, I'll post a fully-tested
> "PATCH" version...
I simply ran out of time to review the remaining ones. You should
have replies on all patches now.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips
2017-08-07 6:22 [RFC v2 0/5] bq27xxx_battery data memory update Liam Breck
2017-08-07 6:22 ` [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table Liam Breck
@ 2017-08-07 6:22 ` Liam Breck
2017-08-12 15:02 ` Sebastian Reichel
2017-08-07 6:22 ` [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-07 6:22 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohár, linux-pm
Cc: Paul Kocialkowski, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
For the existing feature set, these chips act like ones already listed,
so they had been given false but functional IDs. We will be adding features
which obsolete that shadowing, so the following IDs are added:
BQ2752X, 531, 542, 546, 742, 425, 441, 621
Chip-specific features are now tracked by BQ27XXX_O_* flags in di->opts.
No functional changes to the driver.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 120 +++++++++++++++++------------
drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++--
include/linux/power/bq27xxx_battery.h | 9 +++
3 files changed, 86 insertions(+), 59 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index dd84e3d9..b186216d 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -221,6 +221,7 @@ static u8
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
+#define bq2752x_regs bq2751x_regs
bq27500_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -401,6 +402,7 @@ static u8
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
+#define bq27531_regs bq27530_regs
bq27541_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -421,6 +423,9 @@ static u8
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
+#define bq27542_regs bq27541_regs
+#define bq27546_regs bq27541_regs
+#define bq27742_regs bq27541_regs
bq27545_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -461,6 +466,9 @@ static u8
[BQ27XXX_REG_AP] = 0x18,
BQ27XXX_DM_REG_ROWS,
};
+#define bq27425_regs bq27421_regs
+#define bq27441_regs bq27421_regs
+#define bq27621_regs bq27421_regs
static enum power_supply_property bq27000_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -539,6 +547,7 @@ static enum power_supply_property bq2751x_props[] = {
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq2752x_props bq2751x_props
static enum power_supply_property bq27500_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -716,6 +725,7 @@ static enum power_supply_property bq27530_props[] = {
POWER_SUPPLY_PROP_CYCLE_COUNT,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq27531_props bq27530_props
static enum power_supply_property bq27541_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -735,6 +745,9 @@ static enum power_supply_property bq27541_props[] = {
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq27542_props bq27541_props
+#define bq27546_props bq27541_props
+#define bq27742_props bq27541_props
static enum power_supply_property bq27545_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -768,33 +781,50 @@ static enum power_supply_property bq27421_props[] = {
POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
POWER_SUPPLY_PROP_MANUFACTURER,
};
+#define bq27425_props bq27421_props
+#define bq27441_props bq27421_props
+#define bq27621_props bq27421_props
-#define BQ27XXX_DATA(ref) { \
+#define BQ27XXX_O_ZERO 0x00000001
+#define BQ27XXX_O_OTDC 0x00000002
+#define BQ27XXX_O_UTOT 0x00000004
+
+#define BQ27XXX_DATA(ref, opt) { \
+ .opts = (opt), \
.regs = ref##_regs, \
.props = ref##_props, \
.props_size = ARRAY_SIZE(ref##_props) }
static struct {
+ u32 opts;
u8 *regs;
enum power_supply_property *props;
size_t props_size;
} bq27xxx_chip_data[] = {
- [BQ27000] = BQ27XXX_DATA(bq27000),
- [BQ27010] = BQ27XXX_DATA(bq27010),
- [BQ2750X] = BQ27XXX_DATA(bq2750x),
- [BQ2751X] = BQ27XXX_DATA(bq2751x),
- [BQ27500] = BQ27XXX_DATA(bq27500),
- [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
- [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
- [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
- [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
- [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
- [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
- [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
- [BQ27530] = BQ27XXX_DATA(bq27530),
- [BQ27541] = BQ27XXX_DATA(bq27541),
- [BQ27545] = BQ27XXX_DATA(bq27545),
- [BQ27421] = BQ27XXX_DATA(bq27421),
+ [BQ27000] = BQ27XXX_DATA(bq27000, BQ27XXX_O_ZERO),
+ [BQ27010] = BQ27XXX_DATA(bq27010, BQ27XXX_O_ZERO),
+ [BQ2750X] = BQ27XXX_DATA(bq2750x, BQ27XXX_O_OTDC),
+ [BQ2751X] = BQ27XXX_DATA(bq2751x, BQ27XXX_O_OTDC),
+ [BQ2752X] = BQ27XXX_DATA(bq2752x, BQ27XXX_O_OTDC),
+ [BQ27500] = BQ27XXX_DATA(bq27500, BQ27XXX_O_OTDC),
+ [BQ27510G1] = BQ27XXX_DATA(bq27510g1, BQ27XXX_O_OTDC),
+ [BQ27510G2] = BQ27XXX_DATA(bq27510g2, BQ27XXX_O_OTDC),
+ [BQ27510G3] = BQ27XXX_DATA(bq27510g3, BQ27XXX_O_OTDC),
+ [BQ27520G1] = BQ27XXX_DATA(bq27520g1, BQ27XXX_O_OTDC),
+ [BQ27520G2] = BQ27XXX_DATA(bq27520g2, BQ27XXX_O_OTDC),
+ [BQ27520G3] = BQ27XXX_DATA(bq27520g3, BQ27XXX_O_OTDC),
+ [BQ27520G4] = BQ27XXX_DATA(bq27520g4, BQ27XXX_O_OTDC),
+ [BQ27530] = BQ27XXX_DATA(bq27530, BQ27XXX_O_UTOT),
+ [BQ27531] = BQ27XXX_DATA(bq27531, BQ27XXX_O_UTOT),
+ [BQ27541] = BQ27XXX_DATA(bq27541, BQ27XXX_O_OTDC),
+ [BQ27542] = BQ27XXX_DATA(bq27542, BQ27XXX_O_OTDC),
+ [BQ27546] = BQ27XXX_DATA(bq27546, BQ27XXX_O_OTDC),
+ [BQ27742] = BQ27XXX_DATA(bq27742, BQ27XXX_O_OTDC),
+ [BQ27545] = BQ27XXX_DATA(bq27545, BQ27XXX_O_OTDC),
+ [BQ27421] = BQ27XXX_DATA(bq27421, BQ27XXX_O_UTOT),
+ [BQ27425] = BQ27XXX_DATA(bq27425, BQ27XXX_O_UTOT),
+ [BQ27441] = BQ27XXX_DATA(bq27441, BQ27XXX_O_UTOT),
+ [BQ27621] = BQ27XXX_DATA(bq27621, BQ27XXX_O_UTOT),
};
static DEFINE_MUTEX(bq27xxx_list_lock);
@@ -1327,7 +1357,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
{
int soc;
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
soc = bq27xxx_read(di, BQ27XXX_REG_SOC, true);
else
soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
@@ -1353,7 +1383,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
return charge;
}
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
charge *= BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
else
charge *= 1000;
@@ -1369,7 +1399,7 @@ static inline int bq27xxx_battery_read_nac(struct bq27xxx_device_info *di)
{
int flags;
- if (di->chip == BQ27000 || di->chip == BQ27010) {
+ if (di->opts & BQ27XXX_O_ZERO) {
flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
if (flags >= 0 && (flags & BQ27000_FLAG_CI))
return -ENODATA;
@@ -1395,7 +1425,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
{
int dcap;
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, true);
else
dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
@@ -1405,7 +1435,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
return dcap;
}
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
dcap = (dcap << 8) * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
else
dcap *= 1000;
@@ -1427,7 +1457,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
return ae;
}
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
ae *= BQ27XXX_POWER_CONSTANT / BQ27XXX_RS;
else
ae *= 1000;
@@ -1449,7 +1479,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
return temp;
}
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
temp = 5 * temp / 2;
return temp;
@@ -1506,7 +1536,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
return tval;
}
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
return (tval * BQ27XXX_POWER_CONSTANT) / BQ27XXX_RS;
else
return tval;
@@ -1517,26 +1547,12 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
*/
static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
{
- switch (di->chip) {
- case BQ2750X:
- case BQ2751X:
- case BQ27500:
- case BQ27510G1:
- case BQ27510G2:
- case BQ27510G3:
- case BQ27520G1:
- case BQ27520G2:
- case BQ27520G3:
- case BQ27520G4:
- case BQ27541:
- case BQ27545:
+ if (di->opts & BQ27XXX_O_OTDC)
return flags & (BQ27XXX_FLAG_OTC | BQ27XXX_FLAG_OTD);
- case BQ27530:
- case BQ27421:
+ if (di->opts & BQ27XXX_O_UTOT)
return flags & BQ27XXX_FLAG_OT;
- default:
- return false;
- }
+
+ return false;
}
/*
@@ -1544,7 +1560,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
*/
static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
{
- if (di->chip == BQ27530 || di->chip == BQ27421)
+ if (di->opts & BQ27XXX_O_UTOT)
return flags & BQ27XXX_FLAG_UT;
return false;
@@ -1555,7 +1571,7 @@ static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
*/
static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
{
- if (di->chip == BQ27000 || di->chip == BQ27010)
+ if (di->opts & BQ27XXX_O_ZERO)
return flags & (BQ27000_FLAG_EDV1 | BQ27000_FLAG_EDVF);
else
return flags & (BQ27XXX_FLAG_SOC1 | BQ27XXX_FLAG_SOCF);
@@ -1568,7 +1584,7 @@ static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
{
int flags;
- bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010;
+ bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
if (flags < 0) {
@@ -1590,8 +1606,8 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
void bq27xxx_battery_update(struct bq27xxx_device_info *di)
{
struct bq27xxx_reg_cache cache = {0, };
- bool has_ci_flag = di->chip == BQ27000 || di->chip == BQ27010;
- bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010;
+ bool has_ci_flag = di->opts & BQ27XXX_O_ZERO;
+ bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
if ((cache.flags & 0xff) == 0xff)
@@ -1669,7 +1685,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
return curr;
}
- if (di->chip == BQ27000 || di->chip == BQ27010) {
+ if (di->opts & BQ27XXX_O_ZERO) {
flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
if (flags & BQ27000_FLAG_CHGS) {
dev_dbg(di->dev, "negative current!\n");
@@ -1690,7 +1706,7 @@ static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
{
int status;
- if (di->chip == BQ27000 || di->chip == BQ27010) {
+ if (di->opts & BQ27XXX_O_ZERO) {
if (di->cache.flags & BQ27000_FLAG_FC)
status = POWER_SUPPLY_STATUS_FULL;
else if (di->cache.flags & BQ27000_FLAG_CHGS)
@@ -1718,7 +1734,7 @@ static int bq27xxx_battery_capacity_level(struct bq27xxx_device_info *di,
{
int level;
- if (di->chip == BQ27000 || di->chip == BQ27010) {
+ if (di->opts & BQ27XXX_O_ZERO) {
if (di->cache.flags & BQ27000_FLAG_FC)
level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
else if (di->cache.flags & BQ27000_FLAG_EDV1)
@@ -1883,7 +1899,9 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
+
di->regs = bq27xxx_chip_data[di->chip].regs;
+ di->opts = bq27xxx_chip_data[di->chip].opts;
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 a5972214..0b11ed47 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 11e11685..77fe94f1 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -6,6 +6,7 @@ enum bq27xxx_chip {
BQ27010, /* bq27010, bq27210 */
BQ2750X, /* bq27500 deprecated alias */
BQ2751X, /* bq27510, bq27520 deprecated alias */
+ BQ2752X,
BQ27500, /* bq27500/1 */
BQ27510G1, /* bq27510G1 */
BQ27510G2, /* bq27510G2 */
@@ -15,9 +16,16 @@ enum bq27xxx_chip {
BQ27520G3, /* bq27520G3 */
BQ27520G4, /* bq27520G4 */
BQ27530, /* bq27530, bq27531 */
+ BQ27531,
BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
+ BQ27542,
+ BQ27546,
+ BQ27742,
BQ27545, /* bq27545 */
BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
+ BQ27425,
+ BQ27441,
+ BQ27621,
};
/**
@@ -64,6 +72,7 @@ struct bq27xxx_device_info {
int id;
enum bq27xxx_chip chip;
bool ram_chip;
+ u32 opts;
const char *name;
struct bq27xxx_dm_reg *dm_regs;
u32 unseal_key;
--
2.13.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips
2017-08-07 6:22 ` [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips Liam Breck
@ 2017-08-12 15:02 ` Sebastian Reichel
2017-08-12 17:56 ` Liam Breck
0 siblings, 1 reply; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 15:02 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 15075 bytes --]
Hi,
On Sun, Aug 06, 2017 at 11:22:13PM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> For the existing feature set, these chips act like ones already listed,
> so they had been given false but functional IDs. We will be adding features
> which obsolete that shadowing, so the following IDs are added:
> BQ2752X, 531, 542, 546, 742, 425, 441, 621
Patch looks ok, but the patch description does not match the patch?
-- Sebastian
> Chip-specific features are now tracked by BQ27XXX_O_* flags in di->opts.
>
> No functional changes to the driver.
>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq27xxx_battery.c | 120 +++++++++++++++++------------
> drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++--
> include/linux/power/bq27xxx_battery.h | 9 +++
> 3 files changed, 86 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index dd84e3d9..b186216d 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -221,6 +221,7 @@ static u8
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> +#define bq2752x_regs bq2751x_regs
> bq27500_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> @@ -401,6 +402,7 @@ static u8
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> +#define bq27531_regs bq27530_regs
> bq27541_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> @@ -421,6 +423,9 @@ static u8
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> +#define bq27542_regs bq27541_regs
> +#define bq27546_regs bq27541_regs
> +#define bq27742_regs bq27541_regs
> bq27545_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> @@ -461,6 +466,9 @@ static u8
> [BQ27XXX_REG_AP] = 0x18,
> BQ27XXX_DM_REG_ROWS,
> };
> +#define bq27425_regs bq27421_regs
> +#define bq27441_regs bq27421_regs
> +#define bq27621_regs bq27421_regs
>
> static enum power_supply_property bq27000_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> @@ -539,6 +547,7 @@ static enum power_supply_property bq2751x_props[] = {
> POWER_SUPPLY_PROP_HEALTH,
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
> +#define bq2752x_props bq2751x_props
>
> static enum power_supply_property bq27500_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> @@ -716,6 +725,7 @@ static enum power_supply_property bq27530_props[] = {
> POWER_SUPPLY_PROP_CYCLE_COUNT,
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
> +#define bq27531_props bq27530_props
>
> static enum power_supply_property bq27541_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> @@ -735,6 +745,9 @@ static enum power_supply_property bq27541_props[] = {
> POWER_SUPPLY_PROP_HEALTH,
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
> +#define bq27542_props bq27541_props
> +#define bq27546_props bq27541_props
> +#define bq27742_props bq27541_props
>
> static enum power_supply_property bq27545_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> @@ -768,33 +781,50 @@ static enum power_supply_property bq27421_props[] = {
> POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
> +#define bq27425_props bq27421_props
> +#define bq27441_props bq27421_props
> +#define bq27621_props bq27421_props
>
> -#define BQ27XXX_DATA(ref) { \
> +#define BQ27XXX_O_ZERO 0x00000001
> +#define BQ27XXX_O_OTDC 0x00000002
> +#define BQ27XXX_O_UTOT 0x00000004
> +
> +#define BQ27XXX_DATA(ref, opt) { \
> + .opts = (opt), \
> .regs = ref##_regs, \
> .props = ref##_props, \
> .props_size = ARRAY_SIZE(ref##_props) }
>
> static struct {
> + u32 opts;
> u8 *regs;
> enum power_supply_property *props;
> size_t props_size;
> } bq27xxx_chip_data[] = {
> - [BQ27000] = BQ27XXX_DATA(bq27000),
> - [BQ27010] = BQ27XXX_DATA(bq27010),
> - [BQ2750X] = BQ27XXX_DATA(bq2750x),
> - [BQ2751X] = BQ27XXX_DATA(bq2751x),
> - [BQ27500] = BQ27XXX_DATA(bq27500),
> - [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
> - [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
> - [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
> - [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
> - [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
> - [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
> - [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
> - [BQ27530] = BQ27XXX_DATA(bq27530),
> - [BQ27541] = BQ27XXX_DATA(bq27541),
> - [BQ27545] = BQ27XXX_DATA(bq27545),
> - [BQ27421] = BQ27XXX_DATA(bq27421),
> + [BQ27000] = BQ27XXX_DATA(bq27000, BQ27XXX_O_ZERO),
> + [BQ27010] = BQ27XXX_DATA(bq27010, BQ27XXX_O_ZERO),
> + [BQ2750X] = BQ27XXX_DATA(bq2750x, BQ27XXX_O_OTDC),
> + [BQ2751X] = BQ27XXX_DATA(bq2751x, BQ27XXX_O_OTDC),
> + [BQ2752X] = BQ27XXX_DATA(bq2752x, BQ27XXX_O_OTDC),
> + [BQ27500] = BQ27XXX_DATA(bq27500, BQ27XXX_O_OTDC),
> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1, BQ27XXX_O_OTDC),
> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2, BQ27XXX_O_OTDC),
> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3, BQ27XXX_O_OTDC),
> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1, BQ27XXX_O_OTDC),
> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2, BQ27XXX_O_OTDC),
> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3, BQ27XXX_O_OTDC),
> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4, BQ27XXX_O_OTDC),
> + [BQ27530] = BQ27XXX_DATA(bq27530, BQ27XXX_O_UTOT),
> + [BQ27531] = BQ27XXX_DATA(bq27531, BQ27XXX_O_UTOT),
> + [BQ27541] = BQ27XXX_DATA(bq27541, BQ27XXX_O_OTDC),
> + [BQ27542] = BQ27XXX_DATA(bq27542, BQ27XXX_O_OTDC),
> + [BQ27546] = BQ27XXX_DATA(bq27546, BQ27XXX_O_OTDC),
> + [BQ27742] = BQ27XXX_DATA(bq27742, BQ27XXX_O_OTDC),
> + [BQ27545] = BQ27XXX_DATA(bq27545, BQ27XXX_O_OTDC),
> + [BQ27421] = BQ27XXX_DATA(bq27421, BQ27XXX_O_UTOT),
> + [BQ27425] = BQ27XXX_DATA(bq27425, BQ27XXX_O_UTOT),
> + [BQ27441] = BQ27XXX_DATA(bq27441, BQ27XXX_O_UTOT),
> + [BQ27621] = BQ27XXX_DATA(bq27621, BQ27XXX_O_UTOT),
> };
>
> static DEFINE_MUTEX(bq27xxx_list_lock);
> @@ -1327,7 +1357,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
> {
> int soc;
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> soc = bq27xxx_read(di, BQ27XXX_REG_SOC, true);
> else
> soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
> @@ -1353,7 +1383,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
> return charge;
> }
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> charge *= BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
> else
> charge *= 1000;
> @@ -1369,7 +1399,7 @@ static inline int bq27xxx_battery_read_nac(struct bq27xxx_device_info *di)
> {
> int flags;
>
> - if (di->chip == BQ27000 || di->chip == BQ27010) {
> + if (di->opts & BQ27XXX_O_ZERO) {
> flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
> if (flags >= 0 && (flags & BQ27000_FLAG_CI))
> return -ENODATA;
> @@ -1395,7 +1425,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
> {
> int dcap;
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, true);
> else
> dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
> @@ -1405,7 +1435,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
> return dcap;
> }
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> dcap = (dcap << 8) * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
> else
> dcap *= 1000;
> @@ -1427,7 +1457,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
> return ae;
> }
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> ae *= BQ27XXX_POWER_CONSTANT / BQ27XXX_RS;
> else
> ae *= 1000;
> @@ -1449,7 +1479,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
> return temp;
> }
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> temp = 5 * temp / 2;
>
> return temp;
> @@ -1506,7 +1536,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
> return tval;
> }
>
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> return (tval * BQ27XXX_POWER_CONSTANT) / BQ27XXX_RS;
> else
> return tval;
> @@ -1517,26 +1547,12 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
> */
> static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
> {
> - switch (di->chip) {
> - case BQ2750X:
> - case BQ2751X:
> - case BQ27500:
> - case BQ27510G1:
> - case BQ27510G2:
> - case BQ27510G3:
> - case BQ27520G1:
> - case BQ27520G2:
> - case BQ27520G3:
> - case BQ27520G4:
> - case BQ27541:
> - case BQ27545:
> + if (di->opts & BQ27XXX_O_OTDC)
> return flags & (BQ27XXX_FLAG_OTC | BQ27XXX_FLAG_OTD);
> - case BQ27530:
> - case BQ27421:
> + if (di->opts & BQ27XXX_O_UTOT)
> return flags & BQ27XXX_FLAG_OT;
> - default:
> - return false;
> - }
> +
> + return false;
> }
>
> /*
> @@ -1544,7 +1560,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
> */
> static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
> {
> - if (di->chip == BQ27530 || di->chip == BQ27421)
> + if (di->opts & BQ27XXX_O_UTOT)
> return flags & BQ27XXX_FLAG_UT;
>
> return false;
> @@ -1555,7 +1571,7 @@ static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
> */
> static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
> {
> - if (di->chip == BQ27000 || di->chip == BQ27010)
> + if (di->opts & BQ27XXX_O_ZERO)
> return flags & (BQ27000_FLAG_EDV1 | BQ27000_FLAG_EDVF);
> else
> return flags & (BQ27XXX_FLAG_SOC1 | BQ27XXX_FLAG_SOCF);
> @@ -1568,7 +1584,7 @@ static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
> static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
> {
> int flags;
> - bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010;
> + bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
>
> flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
> if (flags < 0) {
> @@ -1590,8 +1606,8 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
> void bq27xxx_battery_update(struct bq27xxx_device_info *di)
> {
> struct bq27xxx_reg_cache cache = {0, };
> - bool has_ci_flag = di->chip == BQ27000 || di->chip == BQ27010;
> - bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010;
> + bool has_ci_flag = di->opts & BQ27XXX_O_ZERO;
> + bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
>
> cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
> if ((cache.flags & 0xff) == 0xff)
> @@ -1669,7 +1685,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
> return curr;
> }
>
> - if (di->chip == BQ27000 || di->chip == BQ27010) {
> + if (di->opts & BQ27XXX_O_ZERO) {
> flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
> if (flags & BQ27000_FLAG_CHGS) {
> dev_dbg(di->dev, "negative current!\n");
> @@ -1690,7 +1706,7 @@ static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
> {
> int status;
>
> - if (di->chip == BQ27000 || di->chip == BQ27010) {
> + if (di->opts & BQ27XXX_O_ZERO) {
> if (di->cache.flags & BQ27000_FLAG_FC)
> status = POWER_SUPPLY_STATUS_FULL;
> else if (di->cache.flags & BQ27000_FLAG_CHGS)
> @@ -1718,7 +1734,7 @@ static int bq27xxx_battery_capacity_level(struct bq27xxx_device_info *di,
> {
> int level;
>
> - if (di->chip == BQ27000 || di->chip == BQ27010) {
> + if (di->opts & BQ27XXX_O_ZERO) {
> if (di->cache.flags & BQ27000_FLAG_FC)
> level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
> else if (di->cache.flags & BQ27000_FLAG_EDV1)
> @@ -1883,7 +1899,9 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>
> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
> mutex_init(&di->lock);
> +
> di->regs = bq27xxx_chip_data[di->chip].regs;
> + di->opts = bq27xxx_chip_data[di->chip].opts;
>
> 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 a5972214..0b11ed47 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 11e11685..77fe94f1 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -6,6 +6,7 @@ enum bq27xxx_chip {
> BQ27010, /* bq27010, bq27210 */
> BQ2750X, /* bq27500 deprecated alias */
> BQ2751X, /* bq27510, bq27520 deprecated alias */
> + BQ2752X,
> BQ27500, /* bq27500/1 */
> BQ27510G1, /* bq27510G1 */
> BQ27510G2, /* bq27510G2 */
> @@ -15,9 +16,16 @@ enum bq27xxx_chip {
> BQ27520G3, /* bq27520G3 */
> BQ27520G4, /* bq27520G4 */
> BQ27530, /* bq27530, bq27531 */
> + BQ27531,
> BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
> + BQ27542,
> + BQ27546,
> + BQ27742,
> BQ27545, /* bq27545 */
> BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
> + BQ27425,
> + BQ27441,
> + BQ27621,
> };
>
> /**
> @@ -64,6 +72,7 @@ struct bq27xxx_device_info {
> int id;
> enum bq27xxx_chip chip;
> bool ram_chip;
> + u32 opts;
> const char *name;
> struct bq27xxx_dm_reg *dm_regs;
> u32 unseal_key;
> --
> 2.13.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips
2017-08-12 15:02 ` Sebastian Reichel
@ 2017-08-12 17:56 ` Liam Breck
2017-08-12 18:45 ` Sebastian Reichel
0 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-12 17:56 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Sat, Aug 12, 2017 at 8:02 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Sun, Aug 06, 2017 at 11:22:13PM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> For the existing feature set, these chips act like ones already listed,
>> so they had been given false but functional IDs. We will be adding features
>> which obsolete that shadowing, so the following IDs are added:
>> BQ2752X, 531, 542, 546, 742, 425, 441, 621
>
> Patch looks ok, but the patch description does not match the patch?
Does this clarify it?
For the existing driver features, the chips below act like others already ID'd,
so they had been given false but functional IDs. We will be adding features
which obsolete that ID shadowing, so the following IDs are added:
BQ2752X, 531, 542, 546, 742, 425, 441, 621
>> Chip-specific features are now tracked by BQ27XXX_O_* flags in di->opts.
>>
>> No functional changes to the driver.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 120 +++++++++++++++++------------
>> drivers/power/supply/bq27xxx_battery_i2c.c | 16 ++--
>> include/linux/power/bq27xxx_battery.h | 9 +++
>> 3 files changed, 86 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index dd84e3d9..b186216d 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -221,6 +221,7 @@ static u8
>> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> +#define bq2752x_regs bq2751x_regs
>> bq27500_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> @@ -401,6 +402,7 @@ static u8
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> +#define bq27531_regs bq27530_regs
>> bq27541_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> @@ -421,6 +423,9 @@ static u8
>> [BQ27XXX_REG_AP] = 0x24,
>> BQ27XXX_DM_REG_ROWS,
>> },
>> +#define bq27542_regs bq27541_regs
>> +#define bq27546_regs bq27541_regs
>> +#define bq27742_regs bq27541_regs
>> bq27545_regs[BQ27XXX_REG_MAX] = {
>> [BQ27XXX_REG_CTRL] = 0x00,
>> [BQ27XXX_REG_TEMP] = 0x06,
>> @@ -461,6 +466,9 @@ static u8
>> [BQ27XXX_REG_AP] = 0x18,
>> BQ27XXX_DM_REG_ROWS,
>> };
>> +#define bq27425_regs bq27421_regs
>> +#define bq27441_regs bq27421_regs
>> +#define bq27621_regs bq27421_regs
>>
>> static enum power_supply_property bq27000_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> @@ -539,6 +547,7 @@ static enum power_supply_property bq2751x_props[] = {
>> POWER_SUPPLY_PROP_HEALTH,
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>> +#define bq2752x_props bq2751x_props
>>
>> static enum power_supply_property bq27500_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> @@ -716,6 +725,7 @@ static enum power_supply_property bq27530_props[] = {
>> POWER_SUPPLY_PROP_CYCLE_COUNT,
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>> +#define bq27531_props bq27530_props
>>
>> static enum power_supply_property bq27541_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> @@ -735,6 +745,9 @@ static enum power_supply_property bq27541_props[] = {
>> POWER_SUPPLY_PROP_HEALTH,
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>> +#define bq27542_props bq27541_props
>> +#define bq27546_props bq27541_props
>> +#define bq27742_props bq27541_props
>>
>> static enum power_supply_property bq27545_props[] = {
>> POWER_SUPPLY_PROP_STATUS,
>> @@ -768,33 +781,50 @@ static enum power_supply_property bq27421_props[] = {
>> POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
>> POWER_SUPPLY_PROP_MANUFACTURER,
>> };
>> +#define bq27425_props bq27421_props
>> +#define bq27441_props bq27421_props
>> +#define bq27621_props bq27421_props
>>
>> -#define BQ27XXX_DATA(ref) { \
>> +#define BQ27XXX_O_ZERO 0x00000001
>> +#define BQ27XXX_O_OTDC 0x00000002
>> +#define BQ27XXX_O_UTOT 0x00000004
>> +
>> +#define BQ27XXX_DATA(ref, opt) { \
>> + .opts = (opt), \
>> .regs = ref##_regs, \
>> .props = ref##_props, \
>> .props_size = ARRAY_SIZE(ref##_props) }
>>
>> static struct {
>> + u32 opts;
>> u8 *regs;
>> enum power_supply_property *props;
>> size_t props_size;
>> } bq27xxx_chip_data[] = {
>> - [BQ27000] = BQ27XXX_DATA(bq27000),
>> - [BQ27010] = BQ27XXX_DATA(bq27010),
>> - [BQ2750X] = BQ27XXX_DATA(bq2750x),
>> - [BQ2751X] = BQ27XXX_DATA(bq2751x),
>> - [BQ27500] = BQ27XXX_DATA(bq27500),
>> - [BQ27510G1] = BQ27XXX_DATA(bq27510g1),
>> - [BQ27510G2] = BQ27XXX_DATA(bq27510g2),
>> - [BQ27510G3] = BQ27XXX_DATA(bq27510g3),
>> - [BQ27520G1] = BQ27XXX_DATA(bq27520g1),
>> - [BQ27520G2] = BQ27XXX_DATA(bq27520g2),
>> - [BQ27520G3] = BQ27XXX_DATA(bq27520g3),
>> - [BQ27520G4] = BQ27XXX_DATA(bq27520g4),
>> - [BQ27530] = BQ27XXX_DATA(bq27530),
>> - [BQ27541] = BQ27XXX_DATA(bq27541),
>> - [BQ27545] = BQ27XXX_DATA(bq27545),
>> - [BQ27421] = BQ27XXX_DATA(bq27421),
>> + [BQ27000] = BQ27XXX_DATA(bq27000, BQ27XXX_O_ZERO),
>> + [BQ27010] = BQ27XXX_DATA(bq27010, BQ27XXX_O_ZERO),
>> + [BQ2750X] = BQ27XXX_DATA(bq2750x, BQ27XXX_O_OTDC),
>> + [BQ2751X] = BQ27XXX_DATA(bq2751x, BQ27XXX_O_OTDC),
>> + [BQ2752X] = BQ27XXX_DATA(bq2752x, BQ27XXX_O_OTDC),
>> + [BQ27500] = BQ27XXX_DATA(bq27500, BQ27XXX_O_OTDC),
>> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1, BQ27XXX_O_OTDC),
>> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2, BQ27XXX_O_OTDC),
>> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3, BQ27XXX_O_OTDC),
>> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1, BQ27XXX_O_OTDC),
>> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2, BQ27XXX_O_OTDC),
>> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3, BQ27XXX_O_OTDC),
>> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4, BQ27XXX_O_OTDC),
>> + [BQ27530] = BQ27XXX_DATA(bq27530, BQ27XXX_O_UTOT),
>> + [BQ27531] = BQ27XXX_DATA(bq27531, BQ27XXX_O_UTOT),
>> + [BQ27541] = BQ27XXX_DATA(bq27541, BQ27XXX_O_OTDC),
>> + [BQ27542] = BQ27XXX_DATA(bq27542, BQ27XXX_O_OTDC),
>> + [BQ27546] = BQ27XXX_DATA(bq27546, BQ27XXX_O_OTDC),
>> + [BQ27742] = BQ27XXX_DATA(bq27742, BQ27XXX_O_OTDC),
>> + [BQ27545] = BQ27XXX_DATA(bq27545, BQ27XXX_O_OTDC),
>> + [BQ27421] = BQ27XXX_DATA(bq27421, BQ27XXX_O_UTOT),
>> + [BQ27425] = BQ27XXX_DATA(bq27425, BQ27XXX_O_UTOT),
>> + [BQ27441] = BQ27XXX_DATA(bq27441, BQ27XXX_O_UTOT),
>> + [BQ27621] = BQ27XXX_DATA(bq27621, BQ27XXX_O_UTOT),
>> };
>>
>> static DEFINE_MUTEX(bq27xxx_list_lock);
>> @@ -1327,7 +1357,7 @@ static int bq27xxx_battery_read_soc(struct bq27xxx_device_info *di)
>> {
>> int soc;
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> soc = bq27xxx_read(di, BQ27XXX_REG_SOC, true);
>> else
>> soc = bq27xxx_read(di, BQ27XXX_REG_SOC, false);
>> @@ -1353,7 +1383,7 @@ static int bq27xxx_battery_read_charge(struct bq27xxx_device_info *di, u8 reg)
>> return charge;
>> }
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> charge *= BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
>> else
>> charge *= 1000;
>> @@ -1369,7 +1399,7 @@ static inline int bq27xxx_battery_read_nac(struct bq27xxx_device_info *di)
>> {
>> int flags;
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010) {
>> + if (di->opts & BQ27XXX_O_ZERO) {
>> flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
>> if (flags >= 0 && (flags & BQ27000_FLAG_CI))
>> return -ENODATA;
>> @@ -1395,7 +1425,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
>> {
>> int dcap;
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, true);
>> else
>> dcap = bq27xxx_read(di, BQ27XXX_REG_DCAP, false);
>> @@ -1405,7 +1435,7 @@ static int bq27xxx_battery_read_dcap(struct bq27xxx_device_info *di)
>> return dcap;
>> }
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> dcap = (dcap << 8) * BQ27XXX_CURRENT_CONSTANT / BQ27XXX_RS;
>> else
>> dcap *= 1000;
>> @@ -1427,7 +1457,7 @@ static int bq27xxx_battery_read_energy(struct bq27xxx_device_info *di)
>> return ae;
>> }
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> ae *= BQ27XXX_POWER_CONSTANT / BQ27XXX_RS;
>> else
>> ae *= 1000;
>> @@ -1449,7 +1479,7 @@ static int bq27xxx_battery_read_temperature(struct bq27xxx_device_info *di)
>> return temp;
>> }
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> temp = 5 * temp / 2;
>>
>> return temp;
>> @@ -1506,7 +1536,7 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
>> return tval;
>> }
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> return (tval * BQ27XXX_POWER_CONSTANT) / BQ27XXX_RS;
>> else
>> return tval;
>> @@ -1517,26 +1547,12 @@ static int bq27xxx_battery_read_pwr_avg(struct bq27xxx_device_info *di)
>> */
>> static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
>> {
>> - switch (di->chip) {
>> - case BQ2750X:
>> - case BQ2751X:
>> - case BQ27500:
>> - case BQ27510G1:
>> - case BQ27510G2:
>> - case BQ27510G3:
>> - case BQ27520G1:
>> - case BQ27520G2:
>> - case BQ27520G3:
>> - case BQ27520G4:
>> - case BQ27541:
>> - case BQ27545:
>> + if (di->opts & BQ27XXX_O_OTDC)
>> return flags & (BQ27XXX_FLAG_OTC | BQ27XXX_FLAG_OTD);
>> - case BQ27530:
>> - case BQ27421:
>> + if (di->opts & BQ27XXX_O_UTOT)
>> return flags & BQ27XXX_FLAG_OT;
>> - default:
>> - return false;
>> - }
>> +
>> + return false;
>> }
>>
>> /*
>> @@ -1544,7 +1560,7 @@ static bool bq27xxx_battery_overtemp(struct bq27xxx_device_info *di, u16 flags)
>> */
>> static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
>> {
>> - if (di->chip == BQ27530 || di->chip == BQ27421)
>> + if (di->opts & BQ27XXX_O_UTOT)
>> return flags & BQ27XXX_FLAG_UT;
>>
>> return false;
>> @@ -1555,7 +1571,7 @@ static bool bq27xxx_battery_undertemp(struct bq27xxx_device_info *di, u16 flags)
>> */
>> static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
>> {
>> - if (di->chip == BQ27000 || di->chip == BQ27010)
>> + if (di->opts & BQ27XXX_O_ZERO)
>> return flags & (BQ27000_FLAG_EDV1 | BQ27000_FLAG_EDVF);
>> else
>> return flags & (BQ27XXX_FLAG_SOC1 | BQ27XXX_FLAG_SOCF);
>> @@ -1568,7 +1584,7 @@ static bool bq27xxx_battery_dead(struct bq27xxx_device_info *di, u16 flags)
>> static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
>> {
>> int flags;
>> - bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010;
>> + bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
>>
>> flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
>> if (flags < 0) {
>> @@ -1590,8 +1606,8 @@ static int bq27xxx_battery_read_health(struct bq27xxx_device_info *di)
>> void bq27xxx_battery_update(struct bq27xxx_device_info *di)
>> {
>> struct bq27xxx_reg_cache cache = {0, };
>> - bool has_ci_flag = di->chip == BQ27000 || di->chip == BQ27010;
>> - bool has_singe_flag = di->chip == BQ27000 || di->chip == BQ27010;
>> + bool has_ci_flag = di->opts & BQ27XXX_O_ZERO;
>> + bool has_singe_flag = di->opts & BQ27XXX_O_ZERO;
>>
>> cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
>> if ((cache.flags & 0xff) == 0xff)
>> @@ -1669,7 +1685,7 @@ static int bq27xxx_battery_current(struct bq27xxx_device_info *di,
>> return curr;
>> }
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010) {
>> + if (di->opts & BQ27XXX_O_ZERO) {
>> flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, true);
>> if (flags & BQ27000_FLAG_CHGS) {
>> dev_dbg(di->dev, "negative current!\n");
>> @@ -1690,7 +1706,7 @@ static int bq27xxx_battery_status(struct bq27xxx_device_info *di,
>> {
>> int status;
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010) {
>> + if (di->opts & BQ27XXX_O_ZERO) {
>> if (di->cache.flags & BQ27000_FLAG_FC)
>> status = POWER_SUPPLY_STATUS_FULL;
>> else if (di->cache.flags & BQ27000_FLAG_CHGS)
>> @@ -1718,7 +1734,7 @@ static int bq27xxx_battery_capacity_level(struct bq27xxx_device_info *di,
>> {
>> int level;
>>
>> - if (di->chip == BQ27000 || di->chip == BQ27010) {
>> + if (di->opts & BQ27XXX_O_ZERO) {
>> if (di->cache.flags & BQ27000_FLAG_FC)
>> level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
>> else if (di->cache.flags & BQ27000_FLAG_EDV1)
>> @@ -1883,7 +1899,9 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>>
>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>> mutex_init(&di->lock);
>> +
>> di->regs = bq27xxx_chip_data[di->chip].regs;
>> + di->opts = bq27xxx_chip_data[di->chip].opts;
>>
>> 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 a5972214..0b11ed47 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 11e11685..77fe94f1 100644
>> --- a/include/linux/power/bq27xxx_battery.h
>> +++ b/include/linux/power/bq27xxx_battery.h
>> @@ -6,6 +6,7 @@ enum bq27xxx_chip {
>> BQ27010, /* bq27010, bq27210 */
>> BQ2750X, /* bq27500 deprecated alias */
>> BQ2751X, /* bq27510, bq27520 deprecated alias */
>> + BQ2752X,
>> BQ27500, /* bq27500/1 */
>> BQ27510G1, /* bq27510G1 */
>> BQ27510G2, /* bq27510G2 */
>> @@ -15,9 +16,16 @@ enum bq27xxx_chip {
>> BQ27520G3, /* bq27520G3 */
>> BQ27520G4, /* bq27520G4 */
>> BQ27530, /* bq27530, bq27531 */
>> + BQ27531,
>> BQ27541, /* bq27541, bq27542, bq27546, bq27742 */
>> + BQ27542,
>> + BQ27546,
>> + BQ27742,
>> BQ27545, /* bq27545 */
>> BQ27421, /* bq27421, bq27425, bq27441, bq27621 */
>> + BQ27425,
>> + BQ27441,
>> + BQ27621,
>> };
>>
>> /**
>> @@ -64,6 +72,7 @@ struct bq27xxx_device_info {
>> int id;
>> enum bq27xxx_chip chip;
>> bool ram_chip;
>> + u32 opts;
>> const char *name;
>> struct bq27xxx_dm_reg *dm_regs;
>> u32 unseal_key;
>> --
>> 2.13.2
>>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips
2017-08-12 17:56 ` Liam Breck
@ 2017-08-12 18:45 ` Sebastian Reichel
0 siblings, 0 replies; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 18:45 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]
Hi,
On Sat, Aug 12, 2017 at 10:56:09AM -0700, Liam Breck wrote:
> Hi Sebastian,
>
> On Sat, Aug 12, 2017 at 8:02 AM, Sebastian Reichel <sre@kernel.org> wrote:
> > Hi,
> >
> > On Sun, Aug 06, 2017 at 11:22:13PM -0700, Liam Breck wrote:
> >> From: Liam Breck <kernel@networkimprov.net>
> >>
> >> For the existing feature set, these chips act like ones already listed,
> >> so they had been given false but functional IDs. We will be adding features
> >> which obsolete that shadowing, so the following IDs are added:
> >> BQ2752X, 531, 542, 546, 742, 425, 441, 621
> >
> > Patch looks ok, but the patch description does not match the patch?
>
> Does this clarify it?
>
> For the existing driver features, the chips below act like others already ID'd,
> so they had been given false but functional IDs. We will be adding features
> which obsolete that ID shadowing, so the following IDs are added:
> BQ2752X, 531, 542, 546, 742, 425, 441, 621
>
> >> Chip-specific features are now tracked by BQ27XXX_O_* flags in di->opts.
> >>
> >> No functional changes to the driver.
I was too tired when I read that yesterday in the airplane. I'm
fine with both texts and throw in a third suggestion:
Add explicit chip IDs for some chips, that are currently using a
compatible chip ID. This is required due to incompatible advanced
features, that will be added later. To keep chip handling simple,
the chip features are now tracked using flags in di->opts. Driver
functionality is not supposed to change.
Choose the one you like most.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips
2017-08-07 6:22 [RFC v2 0/5] bq27xxx_battery data memory update Liam Breck
2017-08-07 6:22 ` [RFC v2 1/5] power: supply: bq27xxx: Create single chip data table Liam Breck
2017-08-07 6:22 ` [RFC v2 2/5] power: supply: bq27xxx: Add chip IDs for previously shadowed chips Liam Breck
@ 2017-08-07 6:22 ` Liam Breck
2017-08-12 16:04 ` Sebastian Reichel
2017-08-07 6:22 ` [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode Liam Breck
2017-08-07 6:22 ` [RFC v2 5/5] power: supply: bq27xxx: Remove duplicate chip data arrays Liam Breck
4 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-07 6:22 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohár, linux-pm
Cc: Paul Kocialkowski, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
Support data memory update on BQ27425. Parameters from TI datasheets are also
provided for BQ27500, 545, 421, 441, 621; however these are commented out,
as they are not tested.
Add BQ27XXX_O_CFGUP & _O_RAM for use in bq27xxx_chip_data[n].opts
and by data memory update functions.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 171 ++++++++++++++++++++++++---------
include/linux/power/bq27xxx_battery.h | 1 -
2 files changed, 126 insertions(+), 46 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index b186216d..8e535890 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -58,7 +58,7 @@
#include <linux/power/bq27xxx_battery.h>
-#define DRIVER_VERSION "1.2.0"
+#define DRIVER_VERSION "1.3.0"
#define BQ27XXX_MANUFACTURER "Texas Instruments"
@@ -785,46 +785,138 @@ static enum power_supply_property bq27421_props[] = {
#define bq27441_props bq27421_props
#define bq27621_props bq27421_props
+struct bq27xxx_dm_reg {
+ u8 subclass_id;
+ u8 offset;
+ u8 bytes;
+ u16 min, max;
+};
+
+enum bq27xxx_dm_reg_id {
+ BQ27XXX_DM_DESIGN_CAPACITY = 0,
+ BQ27XXX_DM_DESIGN_ENERGY,
+ BQ27XXX_DM_TERMINATE_VOLTAGE,
+};
+
+#define bq27000_dm_regs 0
+#define bq27010_dm_regs 0
+#define bq2750x_dm_regs 0
+#define bq2751x_dm_regs 0
+#define bq2752x_dm_regs 0
+
+#if 0 /* not yet tested */
+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 },
+};
+#else
+#define bq27500_dm_regs 0
+#endif
+
+/* todo create data memory definitions from datasheets and test on chips */
+#define bq27510g1_dm_regs 0
+#define bq27510g2_dm_regs 0
+#define bq27510g3_dm_regs 0
+#define bq27520g1_dm_regs 0
+#define bq27520g2_dm_regs 0
+#define bq27520g3_dm_regs 0
+#define bq27520g4_dm_regs 0
+#define bq27530_dm_regs 0
+#define bq27531_dm_regs 0
+#define bq27541_dm_regs 0
+#define bq27542_dm_regs 0
+#define bq27546_dm_regs 0
+#define bq27742_dm_regs 0
+
+#if 0 /* not yet tested */
+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 },
+};
+#else
+#define bq27545_dm_regs 0
+#endif
+
+#if 0 /* not yet tested */
+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 },
+};
+#else
+#define bq27421_dm_regs 0
+#endif
+
+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 },
+};
+
+#if 0 /* not yet tested */
+#define bq27441_dm_regs bq27421_dm_regs
+#else
+#define bq27441_dm_regs 0
+#endif
+
+#if 0 /* not yet tested */
+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 },
+};
+#else
+#define bq27621_dm_regs 0
+#endif
+
#define BQ27XXX_O_ZERO 0x00000001
#define BQ27XXX_O_OTDC 0x00000002
#define BQ27XXX_O_UTOT 0x00000004
+#define BQ27XXX_O_CFGUP 0x00000008
+#define BQ27XXX_O_RAM 0x00000010
-#define BQ27XXX_DATA(ref, opt) { \
+#define BQ27XXX_DATA(ref, key, opt) { \
.opts = (opt), \
+ .unseal_key = key, \
.regs = ref##_regs, \
+ .dm_regs = ref##_dm_regs, \
.props = ref##_props, \
.props_size = ARRAY_SIZE(ref##_props) }
static struct {
u32 opts;
+ u32 unseal_key;
u8 *regs;
+ struct bq27xxx_dm_reg *dm_regs;
enum power_supply_property *props;
size_t props_size;
} bq27xxx_chip_data[] = {
- [BQ27000] = BQ27XXX_DATA(bq27000, BQ27XXX_O_ZERO),
- [BQ27010] = BQ27XXX_DATA(bq27010, BQ27XXX_O_ZERO),
- [BQ2750X] = BQ27XXX_DATA(bq2750x, BQ27XXX_O_OTDC),
- [BQ2751X] = BQ27XXX_DATA(bq2751x, BQ27XXX_O_OTDC),
- [BQ2752X] = BQ27XXX_DATA(bq2752x, BQ27XXX_O_OTDC),
- [BQ27500] = BQ27XXX_DATA(bq27500, BQ27XXX_O_OTDC),
- [BQ27510G1] = BQ27XXX_DATA(bq27510g1, BQ27XXX_O_OTDC),
- [BQ27510G2] = BQ27XXX_DATA(bq27510g2, BQ27XXX_O_OTDC),
- [BQ27510G3] = BQ27XXX_DATA(bq27510g3, BQ27XXX_O_OTDC),
- [BQ27520G1] = BQ27XXX_DATA(bq27520g1, BQ27XXX_O_OTDC),
- [BQ27520G2] = BQ27XXX_DATA(bq27520g2, BQ27XXX_O_OTDC),
- [BQ27520G3] = BQ27XXX_DATA(bq27520g3, BQ27XXX_O_OTDC),
- [BQ27520G4] = BQ27XXX_DATA(bq27520g4, BQ27XXX_O_OTDC),
- [BQ27530] = BQ27XXX_DATA(bq27530, BQ27XXX_O_UTOT),
- [BQ27531] = BQ27XXX_DATA(bq27531, BQ27XXX_O_UTOT),
- [BQ27541] = BQ27XXX_DATA(bq27541, BQ27XXX_O_OTDC),
- [BQ27542] = BQ27XXX_DATA(bq27542, BQ27XXX_O_OTDC),
- [BQ27546] = BQ27XXX_DATA(bq27546, BQ27XXX_O_OTDC),
- [BQ27742] = BQ27XXX_DATA(bq27742, BQ27XXX_O_OTDC),
- [BQ27545] = BQ27XXX_DATA(bq27545, BQ27XXX_O_OTDC),
- [BQ27421] = BQ27XXX_DATA(bq27421, BQ27XXX_O_UTOT),
- [BQ27425] = BQ27XXX_DATA(bq27425, BQ27XXX_O_UTOT),
- [BQ27441] = BQ27XXX_DATA(bq27441, BQ27XXX_O_UTOT),
- [BQ27621] = BQ27XXX_DATA(bq27621, BQ27XXX_O_UTOT),
+ [BQ27000] = BQ27XXX_DATA(bq27000, 0 , BQ27XXX_O_ZERO),
+ [BQ27010] = BQ27XXX_DATA(bq27010, 0 , BQ27XXX_O_ZERO),
+ [BQ2750X] = BQ27XXX_DATA(bq2750x, 0 , BQ27XXX_O_OTDC),
+ [BQ2751X] = BQ27XXX_DATA(bq2751x, 0 , BQ27XXX_O_OTDC),
+ [BQ2752X] = BQ27XXX_DATA(bq2752x, 0 , BQ27XXX_O_OTDC),
+ [BQ27500] = BQ27XXX_DATA(bq27500, 0x04143672, BQ27XXX_O_OTDC),
+ [BQ27510G1] = BQ27XXX_DATA(bq27510g1, 0 , BQ27XXX_O_OTDC),
+ [BQ27510G2] = BQ27XXX_DATA(bq27510g2, 0 , BQ27XXX_O_OTDC),
+ [BQ27510G3] = BQ27XXX_DATA(bq27510g3, 0 , BQ27XXX_O_OTDC),
+ [BQ27520G1] = BQ27XXX_DATA(bq27520g1, 0 , BQ27XXX_O_OTDC),
+ [BQ27520G2] = BQ27XXX_DATA(bq27520g2, 0 , BQ27XXX_O_OTDC),
+ [BQ27520G3] = BQ27XXX_DATA(bq27520g3, 0 , BQ27XXX_O_OTDC),
+ [BQ27520G4] = BQ27XXX_DATA(bq27520g4, 0 , BQ27XXX_O_OTDC),
+ [BQ27530] = BQ27XXX_DATA(bq27530, 0 , BQ27XXX_O_UTOT),
+ [BQ27531] = BQ27XXX_DATA(bq27531, 0 , BQ27XXX_O_UTOT),
+ [BQ27541] = BQ27XXX_DATA(bq27541, 0 , BQ27XXX_O_OTDC),
+ [BQ27542] = BQ27XXX_DATA(bq27542, 0 , BQ27XXX_O_OTDC),
+ [BQ27546] = BQ27XXX_DATA(bq27546, 0 , BQ27XXX_O_OTDC),
+ [BQ27742] = BQ27XXX_DATA(bq27742, 0 , BQ27XXX_O_OTDC),
+ [BQ27545] = BQ27XXX_DATA(bq27545, 0x04143672, BQ27XXX_O_OTDC),
+ [BQ27421] = BQ27XXX_DATA(bq27421, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
+ [BQ27425] = BQ27XXX_DATA(bq27425, 0x04143672, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP),
+ [BQ27441] = BQ27XXX_DATA(bq27441, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
+ [BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
};
static DEFINE_MUTEX(bq27xxx_list_lock);
@@ -834,13 +926,6 @@ static LIST_HEAD(bq27xxx_battery_devices);
#define BQ27XXX_DM_SZ 32
-struct bq27xxx_dm_reg {
- u8 subclass_id;
- u8 offset;
- u8 bytes;
- u16 min, max;
-};
-
/**
* struct bq27xxx_dm_buf - chip data memory buffer
* @class: data memory subclass_id
@@ -873,12 +958,6 @@ static inline u16 *bq27xxx_dm_reg_ptr(struct bq27xxx_dm_buf *buf,
return NULL;
}
-enum bq27xxx_dm_reg_id {
- BQ27XXX_DM_DESIGN_CAPACITY = 0,
- BQ27XXX_DM_DESIGN_ENERGY,
- BQ27XXX_DM_TERMINATE_VOLTAGE,
-};
-
static const char * const bq27xxx_dm_reg_name[] = {
[BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
[BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
@@ -1121,9 +1200,9 @@ static void bq27xxx_battery_update_dm_block(struct bq27xxx_device_info *di,
}
#ifdef CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
- if (!di->ram_chip && !bq27xxx_dt_to_nvm) {
+ if (!(di->opts & BQ27XXX_O_RAM) && !bq27xxx_dt_to_nvm) {
#else
- if (!di->ram_chip) {
+ if (!(di->opts & BQ27XXX_O_RAM)) {
#endif
/* devicetree and NVM differ; defer to NVM */
dev_warn(di->dev, "%s has %u; update to %u disallowed "
@@ -1191,7 +1270,7 @@ static inline int bq27xxx_battery_soft_reset(struct bq27xxx_device_info *di)
static int bq27xxx_battery_write_dm_block(struct bq27xxx_device_info *di,
struct bq27xxx_dm_buf *buf)
{
- bool cfgup = di->chip == BQ27421; /* assume related chips need cfgupdate */
+ bool cfgup = di->opts & BQ27XXX_O_CFGUP;
int ret;
if (!buf->dirty)
@@ -1290,7 +1369,7 @@ static void bq27xxx_battery_set_config(struct bq27xxx_device_info *di,
bq27xxx_battery_seal(di);
- if (updated && di->chip != BQ27421) { /* not a cfgupdate chip, so reset */
+ if (updated && !(di->opts & BQ27XXX_O_CFGUP)) {
bq27xxx_write(di, BQ27XXX_REG_CTRL, BQ27XXX_RESET, false);
BQ27XXX_MSLEEP(300); /* reset time is not documented */
}
@@ -1900,8 +1979,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
- di->regs = bq27xxx_chip_data[di->chip].regs;
- di->opts = bq27xxx_chip_data[di->chip].opts;
+ di->regs = bq27xxx_chip_data[di->chip].regs;
+ di->unseal_key = bq27xxx_chip_data[di->chip].unseal_key;
+ di->dm_regs = bq27xxx_chip_data[di->chip].dm_regs;
+ di->opts = bq27xxx_chip_data[di->chip].opts;
psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
if (!psy_desc)
diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
index 77fe94f1..8a8a3f61 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -71,7 +71,6 @@ struct bq27xxx_device_info {
struct device *dev;
int id;
enum bq27xxx_chip chip;
- bool ram_chip;
u32 opts;
const char *name;
struct bq27xxx_dm_reg *dm_regs;
--
2.13.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips
2017-08-07 6:22 ` [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
@ 2017-08-12 16:04 ` Sebastian Reichel
2017-08-12 18:00 ` Liam Breck
0 siblings, 1 reply; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 16:04 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 11341 bytes --]
Hi,
Looks mostly fine to me.
On Sun, Aug 06, 2017 at 11:22:14PM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> Support data memory update on BQ27425. Parameters from TI datasheets are also
> provided for BQ27500, 545, 421, 441, 621; however these are commented out,
> as they are not tested.
>
> Add BQ27XXX_O_CFGUP & _O_RAM for use in bq27xxx_chip_data[n].opts
> and by data memory update functions.
>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq27xxx_battery.c | 171 ++++++++++++++++++++++++---------
> include/linux/power/bq27xxx_battery.h | 1 -
> 2 files changed, 126 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index b186216d..8e535890 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -58,7 +58,7 @@
>
> #include <linux/power/bq27xxx_battery.h>
>
> -#define DRIVER_VERSION "1.2.0"
> +#define DRIVER_VERSION "1.3.0"
I think we should just drop the driver version thing. The correct
driver version is the kernel version.
> #define BQ27XXX_MANUFACTURER "Texas Instruments"
>
> @@ -785,46 +785,138 @@ static enum power_supply_property bq27421_props[] = {
> #define bq27441_props bq27421_props
> #define bq27621_props bq27421_props
>
> +struct bq27xxx_dm_reg {
> + u8 subclass_id;
> + u8 offset;
> + u8 bytes;
> + u16 min, max;
> +};
> +
> +enum bq27xxx_dm_reg_id {
> + BQ27XXX_DM_DESIGN_CAPACITY = 0,
> + BQ27XXX_DM_DESIGN_ENERGY,
> + BQ27XXX_DM_TERMINATE_VOLTAGE,
> +};
> +
> +#define bq27000_dm_regs 0
> +#define bq27010_dm_regs 0
> +#define bq2750x_dm_regs 0
> +#define bq2751x_dm_regs 0
> +#define bq2752x_dm_regs 0
> +
> +#if 0 /* not yet tested */
> +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 },
> +};
> +#else
> +#define bq27500_dm_regs 0
> +#endif
I guess we can just drop the untested bits and add them once
somebody needs/tested them. The values are directly from the
datasheet anyways, right?
> +/* todo create data memory definitions from datasheets and test on chips */
> +#define bq27510g1_dm_regs 0
> +#define bq27510g2_dm_regs 0
> +#define bq27510g3_dm_regs 0
> +#define bq27520g1_dm_regs 0
> +#define bq27520g2_dm_regs 0
> +#define bq27520g3_dm_regs 0
> +#define bq27520g4_dm_regs 0
> +#define bq27530_dm_regs 0
> +#define bq27531_dm_regs 0
> +#define bq27541_dm_regs 0
> +#define bq27542_dm_regs 0
> +#define bq27546_dm_regs 0
> +#define bq27742_dm_regs 0
> +
> +#if 0 /* not yet tested */
> +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 },
> +};
> +#else
> +#define bq27545_dm_regs 0
> +#endif
> +
> +#if 0 /* not yet tested */
> +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 },
> +};
> +#else
> +#define bq27421_dm_regs 0
> +#endif
> +
> +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 },
> +};
> +
> +#if 0 /* not yet tested */
> +#define bq27441_dm_regs bq27421_dm_regs
> +#else
> +#define bq27441_dm_regs 0
> +#endif
> +
> +#if 0 /* not yet tested */
> +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 },
> +};
> +#else
> +#define bq27621_dm_regs 0
> +#endif
> +
> #define BQ27XXX_O_ZERO 0x00000001
> #define BQ27XXX_O_OTDC 0x00000002
> #define BQ27XXX_O_UTOT 0x00000004
> +#define BQ27XXX_O_CFGUP 0x00000008
> +#define BQ27XXX_O_RAM 0x00000010
>
> -#define BQ27XXX_DATA(ref, opt) { \
> +#define BQ27XXX_DATA(ref, key, opt) { \
> .opts = (opt), \
> + .unseal_key = key, \
> .regs = ref##_regs, \
> + .dm_regs = ref##_dm_regs, \
> .props = ref##_props, \
> .props_size = ARRAY_SIZE(ref##_props) }
>
> static struct {
> u32 opts;
> + u32 unseal_key;
> u8 *regs;
> + struct bq27xxx_dm_reg *dm_regs;
> enum power_supply_property *props;
> size_t props_size;
> } bq27xxx_chip_data[] = {
> - [BQ27000] = BQ27XXX_DATA(bq27000, BQ27XXX_O_ZERO),
> - [BQ27010] = BQ27XXX_DATA(bq27010, BQ27XXX_O_ZERO),
> - [BQ2750X] = BQ27XXX_DATA(bq2750x, BQ27XXX_O_OTDC),
> - [BQ2751X] = BQ27XXX_DATA(bq2751x, BQ27XXX_O_OTDC),
> - [BQ2752X] = BQ27XXX_DATA(bq2752x, BQ27XXX_O_OTDC),
> - [BQ27500] = BQ27XXX_DATA(bq27500, BQ27XXX_O_OTDC),
> - [BQ27510G1] = BQ27XXX_DATA(bq27510g1, BQ27XXX_O_OTDC),
> - [BQ27510G2] = BQ27XXX_DATA(bq27510g2, BQ27XXX_O_OTDC),
> - [BQ27510G3] = BQ27XXX_DATA(bq27510g3, BQ27XXX_O_OTDC),
> - [BQ27520G1] = BQ27XXX_DATA(bq27520g1, BQ27XXX_O_OTDC),
> - [BQ27520G2] = BQ27XXX_DATA(bq27520g2, BQ27XXX_O_OTDC),
> - [BQ27520G3] = BQ27XXX_DATA(bq27520g3, BQ27XXX_O_OTDC),
> - [BQ27520G4] = BQ27XXX_DATA(bq27520g4, BQ27XXX_O_OTDC),
> - [BQ27530] = BQ27XXX_DATA(bq27530, BQ27XXX_O_UTOT),
> - [BQ27531] = BQ27XXX_DATA(bq27531, BQ27XXX_O_UTOT),
> - [BQ27541] = BQ27XXX_DATA(bq27541, BQ27XXX_O_OTDC),
> - [BQ27542] = BQ27XXX_DATA(bq27542, BQ27XXX_O_OTDC),
> - [BQ27546] = BQ27XXX_DATA(bq27546, BQ27XXX_O_OTDC),
> - [BQ27742] = BQ27XXX_DATA(bq27742, BQ27XXX_O_OTDC),
> - [BQ27545] = BQ27XXX_DATA(bq27545, BQ27XXX_O_OTDC),
> - [BQ27421] = BQ27XXX_DATA(bq27421, BQ27XXX_O_UTOT),
> - [BQ27425] = BQ27XXX_DATA(bq27425, BQ27XXX_O_UTOT),
> - [BQ27441] = BQ27XXX_DATA(bq27441, BQ27XXX_O_UTOT),
> - [BQ27621] = BQ27XXX_DATA(bq27621, BQ27XXX_O_UTOT),
> + [BQ27000] = BQ27XXX_DATA(bq27000, 0 , BQ27XXX_O_ZERO),
> + [BQ27010] = BQ27XXX_DATA(bq27010, 0 , BQ27XXX_O_ZERO),
> + [BQ2750X] = BQ27XXX_DATA(bq2750x, 0 , BQ27XXX_O_OTDC),
> + [BQ2751X] = BQ27XXX_DATA(bq2751x, 0 , BQ27XXX_O_OTDC),
> + [BQ2752X] = BQ27XXX_DATA(bq2752x, 0 , BQ27XXX_O_OTDC),
> + [BQ27500] = BQ27XXX_DATA(bq27500, 0x04143672, BQ27XXX_O_OTDC),
> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1, 0 , BQ27XXX_O_OTDC),
> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2, 0 , BQ27XXX_O_OTDC),
> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3, 0 , BQ27XXX_O_OTDC),
> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1, 0 , BQ27XXX_O_OTDC),
> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2, 0 , BQ27XXX_O_OTDC),
> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3, 0 , BQ27XXX_O_OTDC),
> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4, 0 , BQ27XXX_O_OTDC),
> + [BQ27530] = BQ27XXX_DATA(bq27530, 0 , BQ27XXX_O_UTOT),
> + [BQ27531] = BQ27XXX_DATA(bq27531, 0 , BQ27XXX_O_UTOT),
> + [BQ27541] = BQ27XXX_DATA(bq27541, 0 , BQ27XXX_O_OTDC),
> + [BQ27542] = BQ27XXX_DATA(bq27542, 0 , BQ27XXX_O_OTDC),
> + [BQ27546] = BQ27XXX_DATA(bq27546, 0 , BQ27XXX_O_OTDC),
> + [BQ27742] = BQ27XXX_DATA(bq27742, 0 , BQ27XXX_O_OTDC),
> + [BQ27545] = BQ27XXX_DATA(bq27545, 0x04143672, BQ27XXX_O_OTDC),
> + [BQ27421] = BQ27XXX_DATA(bq27421, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
> + [BQ27425] = BQ27XXX_DATA(bq27425, 0x04143672, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP),
> + [BQ27441] = BQ27XXX_DATA(bq27441, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
> + [BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
> };
>
> static DEFINE_MUTEX(bq27xxx_list_lock);
> @@ -834,13 +926,6 @@ static LIST_HEAD(bq27xxx_battery_devices);
>
> #define BQ27XXX_DM_SZ 32
>
> -struct bq27xxx_dm_reg {
> - u8 subclass_id;
> - u8 offset;
> - u8 bytes;
> - u16 min, max;
> -};
> -
> /**
> * struct bq27xxx_dm_buf - chip data memory buffer
> * @class: data memory subclass_id
> @@ -873,12 +958,6 @@ static inline u16 *bq27xxx_dm_reg_ptr(struct bq27xxx_dm_buf *buf,
> return NULL;
> }
>
> -enum bq27xxx_dm_reg_id {
> - BQ27XXX_DM_DESIGN_CAPACITY = 0,
> - BQ27XXX_DM_DESIGN_ENERGY,
> - BQ27XXX_DM_TERMINATE_VOLTAGE,
> -};
> -
> static const char * const bq27xxx_dm_reg_name[] = {
> [BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
> [BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
> @@ -1121,9 +1200,9 @@ static void bq27xxx_battery_update_dm_block(struct bq27xxx_device_info *di,
> }
>
> #ifdef CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
> - if (!di->ram_chip && !bq27xxx_dt_to_nvm) {
> + if (!(di->opts & BQ27XXX_O_RAM) && !bq27xxx_dt_to_nvm) {
> #else
> - if (!di->ram_chip) {
> + if (!(di->opts & BQ27XXX_O_RAM)) {
> #endif
> /* devicetree and NVM differ; defer to NVM */
> dev_warn(di->dev, "%s has %u; update to %u disallowed "
> @@ -1191,7 +1270,7 @@ static inline int bq27xxx_battery_soft_reset(struct bq27xxx_device_info *di)
> static int bq27xxx_battery_write_dm_block(struct bq27xxx_device_info *di,
> struct bq27xxx_dm_buf *buf)
> {
> - bool cfgup = di->chip == BQ27421; /* assume related chips need cfgupdate */
> + bool cfgup = di->opts & BQ27XXX_O_CFGUP;
> int ret;
>
> if (!buf->dirty)
> @@ -1290,7 +1369,7 @@ static void bq27xxx_battery_set_config(struct bq27xxx_device_info *di,
>
> bq27xxx_battery_seal(di);
>
> - if (updated && di->chip != BQ27421) { /* not a cfgupdate chip, so reset */
> + if (updated && !(di->opts & BQ27XXX_O_CFGUP)) {
> bq27xxx_write(di, BQ27XXX_REG_CTRL, BQ27XXX_RESET, false);
> BQ27XXX_MSLEEP(300); /* reset time is not documented */
> }
> @@ -1900,8 +1979,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
> mutex_init(&di->lock);
>
> - di->regs = bq27xxx_chip_data[di->chip].regs;
> - di->opts = bq27xxx_chip_data[di->chip].opts;
> + di->regs = bq27xxx_chip_data[di->chip].regs;
> + di->unseal_key = bq27xxx_chip_data[di->chip].unseal_key;
> + di->dm_regs = bq27xxx_chip_data[di->chip].dm_regs;
> + di->opts = bq27xxx_chip_data[di->chip].opts;
>
> psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
> if (!psy_desc)
> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
> index 77fe94f1..8a8a3f61 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -71,7 +71,6 @@ struct bq27xxx_device_info {
> struct device *dev;
> int id;
> enum bq27xxx_chip chip;
> - bool ram_chip;
> u32 opts;
> const char *name;
> struct bq27xxx_dm_reg *dm_regs;
> --
> 2.13.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips
2017-08-12 16:04 ` Sebastian Reichel
@ 2017-08-12 18:00 ` Liam Breck
2017-08-12 18:57 ` Sebastian Reichel
0 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-12 18:00 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Sat, Aug 12, 2017 at 9:04 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> Looks mostly fine to me.
>
> On Sun, Aug 06, 2017 at 11:22:14PM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> Support data memory update on BQ27425. Parameters from TI datasheets are also
>> provided for BQ27500, 545, 421, 441, 621; however these are commented out,
>> as they are not tested.
>>
>> Add BQ27XXX_O_CFGUP & _O_RAM for use in bq27xxx_chip_data[n].opts
>> and by data memory update functions.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>> drivers/power/supply/bq27xxx_battery.c | 171 ++++++++++++++++++++++++---------
>> include/linux/power/bq27xxx_battery.h | 1 -
>> 2 files changed, 126 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index b186216d..8e535890 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -58,7 +58,7 @@
>>
>> #include <linux/power/bq27xxx_battery.h>
>>
>> -#define DRIVER_VERSION "1.2.0"
>> +#define DRIVER_VERSION "1.3.0"
>
> I think we should just drop the driver version thing. The correct
> driver version is the kernel version.
OK
>> #define BQ27XXX_MANUFACTURER "Texas Instruments"
>>
>> @@ -785,46 +785,138 @@ static enum power_supply_property bq27421_props[] = {
>> #define bq27441_props bq27421_props
>> #define bq27621_props bq27421_props
>>
>> +struct bq27xxx_dm_reg {
>> + u8 subclass_id;
>> + u8 offset;
>> + u8 bytes;
>> + u16 min, max;
>> +};
>> +
>> +enum bq27xxx_dm_reg_id {
>> + BQ27XXX_DM_DESIGN_CAPACITY = 0,
>> + BQ27XXX_DM_DESIGN_ENERGY,
>> + BQ27XXX_DM_TERMINATE_VOLTAGE,
>> +};
>> +
>> +#define bq27000_dm_regs 0
>> +#define bq27010_dm_regs 0
>> +#define bq2750x_dm_regs 0
>> +#define bq2751x_dm_regs 0
>> +#define bq2752x_dm_regs 0
>> +
>> +#if 0 /* not yet tested */
>> +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 },
>> +};
>> +#else
>> +#define bq27500_dm_regs 0
>> +#endif
>
> I guess we can just drop the untested bits and add them once
> somebody needs/tested them. The values are directly from the
> datasheet anyways, right?
These parameters are dispersed among lots of others in the datasheets,
and the dispersion varies by datasheet, so not that easy to find. Can
we leave them in?
>> +/* todo create data memory definitions from datasheets and test on chips */
>> +#define bq27510g1_dm_regs 0
>> +#define bq27510g2_dm_regs 0
>> +#define bq27510g3_dm_regs 0
>> +#define bq27520g1_dm_regs 0
>> +#define bq27520g2_dm_regs 0
>> +#define bq27520g3_dm_regs 0
>> +#define bq27520g4_dm_regs 0
>> +#define bq27530_dm_regs 0
>> +#define bq27531_dm_regs 0
>> +#define bq27541_dm_regs 0
>> +#define bq27542_dm_regs 0
>> +#define bq27546_dm_regs 0
>> +#define bq27742_dm_regs 0
>> +
>> +#if 0 /* not yet tested */
>> +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 },
>> +};
>> +#else
>> +#define bq27545_dm_regs 0
>> +#endif
>> +
>> +#if 0 /* not yet tested */
>> +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 },
>> +};
>> +#else
>> +#define bq27421_dm_regs 0
>> +#endif
>> +
>> +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 },
>> +};
>> +
>> +#if 0 /* not yet tested */
>> +#define bq27441_dm_regs bq27421_dm_regs
>> +#else
>> +#define bq27441_dm_regs 0
>> +#endif
>> +
>> +#if 0 /* not yet tested */
>> +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 },
>> +};
>> +#else
>> +#define bq27621_dm_regs 0
>> +#endif
>> +
>> #define BQ27XXX_O_ZERO 0x00000001
>> #define BQ27XXX_O_OTDC 0x00000002
>> #define BQ27XXX_O_UTOT 0x00000004
>> +#define BQ27XXX_O_CFGUP 0x00000008
>> +#define BQ27XXX_O_RAM 0x00000010
>>
>> -#define BQ27XXX_DATA(ref, opt) { \
>> +#define BQ27XXX_DATA(ref, key, opt) { \
>> .opts = (opt), \
>> + .unseal_key = key, \
>> .regs = ref##_regs, \
>> + .dm_regs = ref##_dm_regs, \
>> .props = ref##_props, \
>> .props_size = ARRAY_SIZE(ref##_props) }
>>
>> static struct {
>> u32 opts;
>> + u32 unseal_key;
>> u8 *regs;
>> + struct bq27xxx_dm_reg *dm_regs;
>> enum power_supply_property *props;
>> size_t props_size;
>> } bq27xxx_chip_data[] = {
>> - [BQ27000] = BQ27XXX_DATA(bq27000, BQ27XXX_O_ZERO),
>> - [BQ27010] = BQ27XXX_DATA(bq27010, BQ27XXX_O_ZERO),
>> - [BQ2750X] = BQ27XXX_DATA(bq2750x, BQ27XXX_O_OTDC),
>> - [BQ2751X] = BQ27XXX_DATA(bq2751x, BQ27XXX_O_OTDC),
>> - [BQ2752X] = BQ27XXX_DATA(bq2752x, BQ27XXX_O_OTDC),
>> - [BQ27500] = BQ27XXX_DATA(bq27500, BQ27XXX_O_OTDC),
>> - [BQ27510G1] = BQ27XXX_DATA(bq27510g1, BQ27XXX_O_OTDC),
>> - [BQ27510G2] = BQ27XXX_DATA(bq27510g2, BQ27XXX_O_OTDC),
>> - [BQ27510G3] = BQ27XXX_DATA(bq27510g3, BQ27XXX_O_OTDC),
>> - [BQ27520G1] = BQ27XXX_DATA(bq27520g1, BQ27XXX_O_OTDC),
>> - [BQ27520G2] = BQ27XXX_DATA(bq27520g2, BQ27XXX_O_OTDC),
>> - [BQ27520G3] = BQ27XXX_DATA(bq27520g3, BQ27XXX_O_OTDC),
>> - [BQ27520G4] = BQ27XXX_DATA(bq27520g4, BQ27XXX_O_OTDC),
>> - [BQ27530] = BQ27XXX_DATA(bq27530, BQ27XXX_O_UTOT),
>> - [BQ27531] = BQ27XXX_DATA(bq27531, BQ27XXX_O_UTOT),
>> - [BQ27541] = BQ27XXX_DATA(bq27541, BQ27XXX_O_OTDC),
>> - [BQ27542] = BQ27XXX_DATA(bq27542, BQ27XXX_O_OTDC),
>> - [BQ27546] = BQ27XXX_DATA(bq27546, BQ27XXX_O_OTDC),
>> - [BQ27742] = BQ27XXX_DATA(bq27742, BQ27XXX_O_OTDC),
>> - [BQ27545] = BQ27XXX_DATA(bq27545, BQ27XXX_O_OTDC),
>> - [BQ27421] = BQ27XXX_DATA(bq27421, BQ27XXX_O_UTOT),
>> - [BQ27425] = BQ27XXX_DATA(bq27425, BQ27XXX_O_UTOT),
>> - [BQ27441] = BQ27XXX_DATA(bq27441, BQ27XXX_O_UTOT),
>> - [BQ27621] = BQ27XXX_DATA(bq27621, BQ27XXX_O_UTOT),
>> + [BQ27000] = BQ27XXX_DATA(bq27000, 0 , BQ27XXX_O_ZERO),
>> + [BQ27010] = BQ27XXX_DATA(bq27010, 0 , BQ27XXX_O_ZERO),
>> + [BQ2750X] = BQ27XXX_DATA(bq2750x, 0 , BQ27XXX_O_OTDC),
>> + [BQ2751X] = BQ27XXX_DATA(bq2751x, 0 , BQ27XXX_O_OTDC),
>> + [BQ2752X] = BQ27XXX_DATA(bq2752x, 0 , BQ27XXX_O_OTDC),
>> + [BQ27500] = BQ27XXX_DATA(bq27500, 0x04143672, BQ27XXX_O_OTDC),
>> + [BQ27510G1] = BQ27XXX_DATA(bq27510g1, 0 , BQ27XXX_O_OTDC),
>> + [BQ27510G2] = BQ27XXX_DATA(bq27510g2, 0 , BQ27XXX_O_OTDC),
>> + [BQ27510G3] = BQ27XXX_DATA(bq27510g3, 0 , BQ27XXX_O_OTDC),
>> + [BQ27520G1] = BQ27XXX_DATA(bq27520g1, 0 , BQ27XXX_O_OTDC),
>> + [BQ27520G2] = BQ27XXX_DATA(bq27520g2, 0 , BQ27XXX_O_OTDC),
>> + [BQ27520G3] = BQ27XXX_DATA(bq27520g3, 0 , BQ27XXX_O_OTDC),
>> + [BQ27520G4] = BQ27XXX_DATA(bq27520g4, 0 , BQ27XXX_O_OTDC),
>> + [BQ27530] = BQ27XXX_DATA(bq27530, 0 , BQ27XXX_O_UTOT),
>> + [BQ27531] = BQ27XXX_DATA(bq27531, 0 , BQ27XXX_O_UTOT),
>> + [BQ27541] = BQ27XXX_DATA(bq27541, 0 , BQ27XXX_O_OTDC),
>> + [BQ27542] = BQ27XXX_DATA(bq27542, 0 , BQ27XXX_O_OTDC),
>> + [BQ27546] = BQ27XXX_DATA(bq27546, 0 , BQ27XXX_O_OTDC),
>> + [BQ27742] = BQ27XXX_DATA(bq27742, 0 , BQ27XXX_O_OTDC),
>> + [BQ27545] = BQ27XXX_DATA(bq27545, 0x04143672, BQ27XXX_O_OTDC),
>> + [BQ27421] = BQ27XXX_DATA(bq27421, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
>> + [BQ27425] = BQ27XXX_DATA(bq27425, 0x04143672, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP),
>> + [BQ27441] = BQ27XXX_DATA(bq27441, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
>> + [BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
>> };
>>
>> static DEFINE_MUTEX(bq27xxx_list_lock);
>> @@ -834,13 +926,6 @@ static LIST_HEAD(bq27xxx_battery_devices);
>>
>> #define BQ27XXX_DM_SZ 32
>>
>> -struct bq27xxx_dm_reg {
>> - u8 subclass_id;
>> - u8 offset;
>> - u8 bytes;
>> - u16 min, max;
>> -};
>> -
>> /**
>> * struct bq27xxx_dm_buf - chip data memory buffer
>> * @class: data memory subclass_id
>> @@ -873,12 +958,6 @@ static inline u16 *bq27xxx_dm_reg_ptr(struct bq27xxx_dm_buf *buf,
>> return NULL;
>> }
>>
>> -enum bq27xxx_dm_reg_id {
>> - BQ27XXX_DM_DESIGN_CAPACITY = 0,
>> - BQ27XXX_DM_DESIGN_ENERGY,
>> - BQ27XXX_DM_TERMINATE_VOLTAGE,
>> -};
>> -
>> static const char * const bq27xxx_dm_reg_name[] = {
>> [BQ27XXX_DM_DESIGN_CAPACITY] = "design-capacity",
>> [BQ27XXX_DM_DESIGN_ENERGY] = "design-energy",
>> @@ -1121,9 +1200,9 @@ static void bq27xxx_battery_update_dm_block(struct bq27xxx_device_info *di,
>> }
>>
>> #ifdef CONFIG_BATTERY_BQ27XXX_DT_UPDATES_NVM
>> - if (!di->ram_chip && !bq27xxx_dt_to_nvm) {
>> + if (!(di->opts & BQ27XXX_O_RAM) && !bq27xxx_dt_to_nvm) {
>> #else
>> - if (!di->ram_chip) {
>> + if (!(di->opts & BQ27XXX_O_RAM)) {
>> #endif
>> /* devicetree and NVM differ; defer to NVM */
>> dev_warn(di->dev, "%s has %u; update to %u disallowed "
>> @@ -1191,7 +1270,7 @@ static inline int bq27xxx_battery_soft_reset(struct bq27xxx_device_info *di)
>> static int bq27xxx_battery_write_dm_block(struct bq27xxx_device_info *di,
>> struct bq27xxx_dm_buf *buf)
>> {
>> - bool cfgup = di->chip == BQ27421; /* assume related chips need cfgupdate */
>> + bool cfgup = di->opts & BQ27XXX_O_CFGUP;
>> int ret;
>>
>> if (!buf->dirty)
>> @@ -1290,7 +1369,7 @@ static void bq27xxx_battery_set_config(struct bq27xxx_device_info *di,
>>
>> bq27xxx_battery_seal(di);
>>
>> - if (updated && di->chip != BQ27421) { /* not a cfgupdate chip, so reset */
>> + if (updated && !(di->opts & BQ27XXX_O_CFGUP)) {
>> bq27xxx_write(di, BQ27XXX_REG_CTRL, BQ27XXX_RESET, false);
>> BQ27XXX_MSLEEP(300); /* reset time is not documented */
>> }
>> @@ -1900,8 +1979,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>> mutex_init(&di->lock);
>>
>> - di->regs = bq27xxx_chip_data[di->chip].regs;
>> - di->opts = bq27xxx_chip_data[di->chip].opts;
>> + di->regs = bq27xxx_chip_data[di->chip].regs;
>> + di->unseal_key = bq27xxx_chip_data[di->chip].unseal_key;
>> + di->dm_regs = bq27xxx_chip_data[di->chip].dm_regs;
>> + di->opts = bq27xxx_chip_data[di->chip].opts;
>>
>> psy_desc = devm_kzalloc(di->dev, sizeof(*psy_desc), GFP_KERNEL);
>> if (!psy_desc)
>> diff --git a/include/linux/power/bq27xxx_battery.h b/include/linux/power/bq27xxx_battery.h
>> index 77fe94f1..8a8a3f61 100644
>> --- a/include/linux/power/bq27xxx_battery.h
>> +++ b/include/linux/power/bq27xxx_battery.h
>> @@ -71,7 +71,6 @@ struct bq27xxx_device_info {
>> struct device *dev;
>> int id;
>> enum bq27xxx_chip chip;
>> - bool ram_chip;
>> u32 opts;
>> const char *name;
>> struct bq27xxx_dm_reg *dm_regs;
>> --
>> 2.13.2
>>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips
2017-08-12 18:00 ` Liam Breck
@ 2017-08-12 18:57 ` Sebastian Reichel
0 siblings, 0 replies; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 18:57 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 842 bytes --]
Hi,
On Sat, Aug 12, 2017 at 11:00:46AM -0700, Liam Breck wrote:
> >> +#if 0 /* not yet tested */
> >> +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 },
> >> +};
> >> +#else
> >> +#define bq27500_dm_regs 0
> >> +#endif
> >
> > I guess we can just drop the untested bits and add them once
> > somebody needs/tested them. The values are directly from the
> > datasheet anyways, right?
>
> These parameters are dispersed among lots of others in the datasheets,
> and the dispersion varies by datasheet, so not that easy to find. Can
> we leave them in?
sure, if it's hard to obtain them.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode
2017-08-07 6:22 [RFC v2 0/5] bq27xxx_battery data memory update Liam Breck
` (2 preceding siblings ...)
2017-08-07 6:22 ` [RFC v2 3/5] power: supply: bq27xxx: Enable data memory update for certain chips Liam Breck
@ 2017-08-07 6:22 ` Liam Breck
2017-08-12 16:25 ` Sebastian Reichel
2017-08-07 6:22 ` [RFC v2 5/5] power: supply: bq27xxx: Remove duplicate chip data arrays Liam Breck
4 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-07 6:22 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohár, linux-pm
Cc: Paul Kocialkowski, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
The driver has 13 unique register maps, several of which are shared
by multiple chips. When adding support for a new chip, it's easy to
add a duplicate map by mistake.
In debug mode we now scan bq27xxx_chip_data[n].regs/props/dm_regs for
duplicates.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 40 +++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 8e535890..d85f9ec2 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -885,7 +885,7 @@ static struct bq27xxx_dm_reg bq27621_dm_regs[] = {
.props = ref##_props, \
.props_size = ARRAY_SIZE(ref##_props) }
-static struct {
+static struct bq27xxx_chip_datum {
u32 opts;
u32 unseal_key;
u8 *regs;
@@ -919,6 +919,40 @@ static struct {
[BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
};
+static void __maybe_unused bq27xxx_battery_dbg_dupes(struct bq27xxx_device_info *di)
+{
+ static bool once = false;
+ const size_t max = ARRAY_SIZE(bq27xxx_chip_data);
+ const char * const msg = "bq27xxx_chip_data[%d].%s & [%d].%s are identical\n";
+ struct bq27xxx_chip_datum *a, *b;
+ int i, j;
+
+ if (once)
+ return;
+ once = true;
+
+ for (i = 1; i < max-1; i++) {
+ a = bq27xxx_chip_data + i;
+
+ for (j = i+1; j < max; j++) {
+ b = bq27xxx_chip_data + j;
+
+ if (a->regs != b->regs &&
+ !memcmp(a->regs, b->regs, sizeof(bq27000_regs)))
+ dev_warn(di->dev, msg, i, "regs", j, "regs");
+
+ if (a->props != b->props &&
+ a->props_size == b->props_size &&
+ !memcmp(a->props, b->props, a->props_size))
+ dev_warn(di->dev, msg, i, "props", j, "props");
+
+ if (a->dm_regs != b->dm_regs &&
+ !memcmp(a->dm_regs, b->dm_regs, sizeof(bq27500_dm_regs)))
+ dev_warn(di->dev, msg, i, "dm_regs", j, "dm_regs");
+ }
+ }
+}
+
static DEFINE_MUTEX(bq27xxx_list_lock);
static LIST_HEAD(bq27xxx_battery_devices);
@@ -1976,6 +2010,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
.drv_data = di,
};
+#ifdef DEBUG
+ bq27xxx_battery_dbg_dupes(di);
+#endif
+
INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
mutex_init(&di->lock);
--
2.13.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode
2017-08-07 6:22 ` [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode Liam Breck
@ 2017-08-12 16:25 ` Sebastian Reichel
2017-08-12 18:35 ` Liam Breck
2017-08-12 18:40 ` Liam Breck
0 siblings, 2 replies; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 16:25 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 2935 bytes --]
Hi,
On Sun, Aug 06, 2017 at 11:22:15PM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> The driver has 13 unique register maps, several of which are shared
> by multiple chips. When adding support for a new chip, it's easy to
> add a duplicate map by mistake.
>
> In debug mode we now scan bq27xxx_chip_data[n].regs/props/dm_regs for
> duplicates.
>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
This should be done by static code analysis using coccinelle or
something similar, so that it's catched by build bots and not by
users. Even worse the loop is called for each bq27xxx battery in the
system.
-- Sebastian
> drivers/power/supply/bq27xxx_battery.c | 40 +++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index 8e535890..d85f9ec2 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -885,7 +885,7 @@ static struct bq27xxx_dm_reg bq27621_dm_regs[] = {
> .props = ref##_props, \
> .props_size = ARRAY_SIZE(ref##_props) }
>
> -static struct {
> +static struct bq27xxx_chip_datum {
> u32 opts;
> u32 unseal_key;
> u8 *regs;
> @@ -919,6 +919,40 @@ static struct {
> [BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
> };
>
> +static void __maybe_unused bq27xxx_battery_dbg_dupes(struct bq27xxx_device_info *di)
> +{
> + static bool once = false;
> + const size_t max = ARRAY_SIZE(bq27xxx_chip_data);
> + const char * const msg = "bq27xxx_chip_data[%d].%s & [%d].%s are identical\n";
> + struct bq27xxx_chip_datum *a, *b;
> + int i, j;
> +
> + if (once)
> + return;
> + once = true;
> +
> + for (i = 1; i < max-1; i++) {
> + a = bq27xxx_chip_data + i;
> +
> + for (j = i+1; j < max; j++) {
> + b = bq27xxx_chip_data + j;
> +
> + if (a->regs != b->regs &&
> + !memcmp(a->regs, b->regs, sizeof(bq27000_regs)))
> + dev_warn(di->dev, msg, i, "regs", j, "regs");
> +
> + if (a->props != b->props &&
> + a->props_size == b->props_size &&
> + !memcmp(a->props, b->props, a->props_size))
> + dev_warn(di->dev, msg, i, "props", j, "props");
> +
> + if (a->dm_regs != b->dm_regs &&
> + !memcmp(a->dm_regs, b->dm_regs, sizeof(bq27500_dm_regs)))
> + dev_warn(di->dev, msg, i, "dm_regs", j, "dm_regs");
> + }
> + }
> +}
> +
> static DEFINE_MUTEX(bq27xxx_list_lock);
> static LIST_HEAD(bq27xxx_battery_devices);
>
> @@ -1976,6 +2010,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
> .drv_data = di,
> };
>
> +#ifdef DEBUG
> + bq27xxx_battery_dbg_dupes(di);
> +#endif
> +
> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
> mutex_init(&di->lock);
>
> --
> 2.13.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode
2017-08-12 16:25 ` Sebastian Reichel
@ 2017-08-12 18:35 ` Liam Breck
2017-08-12 18:40 ` Liam Breck
1 sibling, 0 replies; 19+ messages in thread
From: Liam Breck @ 2017-08-12 18:35 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Sat, Aug 12, 2017 at 9:25 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Sun, Aug 06, 2017 at 11:22:15PM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> The driver has 13 unique register maps, several of which are shared
>> by multiple chips. When adding support for a new chip, it's easy to
>> add a duplicate map by mistake.
>>
>> In debug mode we now scan bq27xxx_chip_data[n].regs/props/dm_regs for
>> duplicates.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>
> This should be done by static code analysis using coccinelle or
> something similar, so that it's catched by build bots and not by
> users. Even worse the loop is called for each bq27xxx battery in the
> system.
In your last feedback on this patch, you suggested a change to prevent
looping for each bq27xxx battery in system. I added that, see "once"
variable.
>From that feedback, I inferred that you would accept this. I'm sorry I
don't have cycles to learn coccinelle in the near-future. Could we
upstream this and make a note to move it to static analysis later?
>
>> drivers/power/supply/bq27xxx_battery.c | 40 +++++++++++++++++++++++++++++++++-
>> 1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index 8e535890..d85f9ec2 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -885,7 +885,7 @@ static struct bq27xxx_dm_reg bq27621_dm_regs[] = {
>> .props = ref##_props, \
>> .props_size = ARRAY_SIZE(ref##_props) }
>>
>> -static struct {
>> +static struct bq27xxx_chip_datum {
>> u32 opts;
>> u32 unseal_key;
>> u8 *regs;
>> @@ -919,6 +919,40 @@ static struct {
>> [BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
>> };
>>
>> +static void __maybe_unused bq27xxx_battery_dbg_dupes(struct bq27xxx_device_info *di)
>> +{
>> + static bool once = false;
>> + const size_t max = ARRAY_SIZE(bq27xxx_chip_data);
>> + const char * const msg = "bq27xxx_chip_data[%d].%s & [%d].%s are identical\n";
>> + struct bq27xxx_chip_datum *a, *b;
>> + int i, j;
>> +
>> + if (once)
>> + return;
>> + once = true;
>> +
>> + for (i = 1; i < max-1; i++) {
>> + a = bq27xxx_chip_data + i;
>> +
>> + for (j = i+1; j < max; j++) {
>> + b = bq27xxx_chip_data + j;
>> +
>> + if (a->regs != b->regs &&
>> + !memcmp(a->regs, b->regs, sizeof(bq27000_regs)))
>> + dev_warn(di->dev, msg, i, "regs", j, "regs");
>> +
>> + if (a->props != b->props &&
>> + a->props_size == b->props_size &&
>> + !memcmp(a->props, b->props, a->props_size))
>> + dev_warn(di->dev, msg, i, "props", j, "props");
>> +
>> + if (a->dm_regs != b->dm_regs &&
>> + !memcmp(a->dm_regs, b->dm_regs, sizeof(bq27500_dm_regs)))
>> + dev_warn(di->dev, msg, i, "dm_regs", j, "dm_regs");
>> + }
>> + }
>> +}
>> +
>> static DEFINE_MUTEX(bq27xxx_list_lock);
>> static LIST_HEAD(bq27xxx_battery_devices);
>>
>> @@ -1976,6 +2010,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> .drv_data = di,
>> };
>>
>> +#ifdef DEBUG
>> + bq27xxx_battery_dbg_dupes(di);
>> +#endif
>> +
>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>> mutex_init(&di->lock);
>>
>> --
>> 2.13.2
>>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode
2017-08-12 16:25 ` Sebastian Reichel
2017-08-12 18:35 ` Liam Breck
@ 2017-08-12 18:40 ` Liam Breck
1 sibling, 0 replies; 19+ messages in thread
From: Liam Breck @ 2017-08-12 18:40 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
Hi Sebastian,
On Sat, Aug 12, 2017 at 9:25 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Sun, Aug 06, 2017 at 11:22:15PM -0700, Liam Breck wrote:
>> From: Liam Breck <kernel@networkimprov.net>
>>
>> The driver has 13 unique register maps, several of which are shared
>> by multiple chips. When adding support for a new chip, it's easy to
>> add a duplicate map by mistake.
>>
>> In debug mode we now scan bq27xxx_chip_data[n].regs/props/dm_regs for
>> duplicates.
>>
>> Signed-off-by: Liam Breck <kernel@networkimprov.net>
>> ---
>
> This should be done by static code analysis using coccinelle or
> something similar, so that it's catched by build bots and not by
> users. Even worse the loop is called for each bq27xxx battery in the
> system.
In your last feedback on this patch, you suggested a change to prevent
looping for each bq27xxx battery in system. I added that, see "once"
variable.
>From that feedback, I inferred that you would accept this. I'm sorry I
don't have cycles to learn coccinelle in the near-future. Could we
upstream this and make a note to move it to static analysis later?
>
>> drivers/power/supply/bq27xxx_battery.c | 40 +++++++++++++++++++++++++++++++++-
>> 1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
>> index 8e535890..d85f9ec2 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -885,7 +885,7 @@ static struct bq27xxx_dm_reg bq27621_dm_regs[] = {
>> .props = ref##_props, \
>> .props_size = ARRAY_SIZE(ref##_props) }
>>
>> -static struct {
>> +static struct bq27xxx_chip_datum {
>> u32 opts;
>> u32 unseal_key;
>> u8 *regs;
>> @@ -919,6 +919,40 @@ static struct {
>> [BQ27621] = BQ27XXX_DATA(bq27621, 0x80008000, BQ27XXX_O_UTOT | BQ27XXX_O_CFGUP | BQ27XXX_O_RAM),
>> };
>>
>> +static void __maybe_unused bq27xxx_battery_dbg_dupes(struct bq27xxx_device_info *di)
>> +{
>> + static bool once = false;
>> + const size_t max = ARRAY_SIZE(bq27xxx_chip_data);
>> + const char * const msg = "bq27xxx_chip_data[%d].%s & [%d].%s are identical\n";
>> + struct bq27xxx_chip_datum *a, *b;
>> + int i, j;
>> +
>> + if (once)
>> + return;
>> + once = true;
>> +
>> + for (i = 1; i < max-1; i++) {
>> + a = bq27xxx_chip_data + i;
>> +
>> + for (j = i+1; j < max; j++) {
>> + b = bq27xxx_chip_data + j;
>> +
>> + if (a->regs != b->regs &&
>> + !memcmp(a->regs, b->regs, sizeof(bq27000_regs)))
>> + dev_warn(di->dev, msg, i, "regs", j, "regs");
>> +
>> + if (a->props != b->props &&
>> + a->props_size == b->props_size &&
>> + !memcmp(a->props, b->props, a->props_size))
>> + dev_warn(di->dev, msg, i, "props", j, "props");
>> +
>> + if (a->dm_regs != b->dm_regs &&
>> + !memcmp(a->dm_regs, b->dm_regs, sizeof(bq27500_dm_regs)))
>> + dev_warn(di->dev, msg, i, "dm_regs", j, "dm_regs");
>> + }
>> + }
>> +}
>> +
>> static DEFINE_MUTEX(bq27xxx_list_lock);
>> static LIST_HEAD(bq27xxx_battery_devices);
>>
>> @@ -1976,6 +2010,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>> .drv_data = di,
>> };
>>
>> +#ifdef DEBUG
>> + bq27xxx_battery_dbg_dupes(di);
>> +#endif
>> +
>> INIT_DELAYED_WORK(&di->work, bq27xxx_battery_poll);
>> mutex_init(&di->lock);
>>
>> --
>> 2.13.2
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v2 5/5] power: supply: bq27xxx: Remove duplicate chip data arrays
2017-08-07 6:22 [RFC v2 0/5] bq27xxx_battery data memory update Liam Breck
` (3 preceding siblings ...)
2017-08-07 6:22 ` [RFC v2 4/5] power: supply: bq27xxx: Flag identical chip data when in debug mode Liam Breck
@ 2017-08-07 6:22 ` Liam Breck
2017-08-12 16:22 ` Sebastian Reichel
4 siblings, 1 reply; 19+ messages in thread
From: Liam Breck @ 2017-08-07 6:22 UTC (permalink / raw)
To: Sebastian Reichel, Pali Rohár, linux-pm
Cc: Paul Kocialkowski, Liam Breck
From: Liam Breck <kernel@networkimprov.net>
BQ2751X & BQ27510G3 have identical regs & props.
BQ27500 & BQ27510G1 & BQ27510G2 have identical regs & props.
Remove the duplicate arrays.
No functional changes to the driver.
Signed-off-by: Liam Breck <kernel@networkimprov.net>
---
drivers/power/supply/bq27xxx_battery.c | 129 ++-------------------------------
1 file changed, 8 insertions(+), 121 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index d85f9ec2..b451d49a 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -201,27 +201,8 @@ static u8
[BQ27XXX_REG_AP] = INVALID_REG_ADDR,
BQ27XXX_DM_REG_ROWS,
},
- bq2751x_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] = INVALID_REG_ADDR,
- [BQ27XXX_REG_TTES] = 0x1a,
- [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
- [BQ27XXX_REG_NAC] = 0x0c,
- [BQ27XXX_REG_FCC] = 0x12,
- [BQ27XXX_REG_CYCT] = 0x1e,
- [BQ27XXX_REG_AE] = INVALID_REG_ADDR,
- [BQ27XXX_REG_SOC] = 0x20,
- [BQ27XXX_REG_DCAP] = 0x2e,
- [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
- BQ27XXX_DM_REG_ROWS,
- },
-#define bq2752x_regs bq2751x_regs
+#define bq2751x_regs bq27510g3_regs
+#define bq2752x_regs bq27510g3_regs
bq27500_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -242,46 +223,8 @@ static u8
[BQ27XXX_REG_AP] = 0x24,
BQ27XXX_DM_REG_ROWS,
},
- bq27510g1_regs[BQ27XXX_REG_MAX] = {
- [BQ27XXX_REG_CTRL] = 0x00,
- [BQ27XXX_REG_TEMP] = 0x06,
- [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
- [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] = 0x26,
- [BQ27XXX_REG_NAC] = 0x0c,
- [BQ27XXX_REG_FCC] = 0x12,
- [BQ27XXX_REG_CYCT] = 0x2a,
- [BQ27XXX_REG_AE] = 0x22,
- [BQ27XXX_REG_SOC] = 0x2c,
- [BQ27XXX_REG_DCAP] = 0x3c,
- [BQ27XXX_REG_AP] = 0x24,
- BQ27XXX_DM_REG_ROWS,
- },
- bq27510g2_regs[BQ27XXX_REG_MAX] = {
- [BQ27XXX_REG_CTRL] = 0x00,
- [BQ27XXX_REG_TEMP] = 0x06,
- [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
- [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] = 0x26,
- [BQ27XXX_REG_NAC] = 0x0c,
- [BQ27XXX_REG_FCC] = 0x12,
- [BQ27XXX_REG_CYCT] = 0x2a,
- [BQ27XXX_REG_AE] = 0x22,
- [BQ27XXX_REG_SOC] = 0x2c,
- [BQ27XXX_REG_DCAP] = 0x3c,
- [BQ27XXX_REG_AP] = 0x24,
- BQ27XXX_DM_REG_ROWS,
- },
+#define bq27510g1_regs bq27500_regs
+#define bq27510g2_regs bq27500_regs
bq27510g3_regs[BQ27XXX_REG_MAX] = {
[BQ27XXX_REG_CTRL] = 0x00,
[BQ27XXX_REG_TEMP] = 0x06,
@@ -530,24 +473,8 @@ static enum power_supply_property bq2750x_props[] = {
POWER_SUPPLY_PROP_MANUFACTURER,
};
-static enum power_supply_property bq2751x_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_TECHNOLOGY,
- POWER_SUPPLY_PROP_CHARGE_FULL,
- POWER_SUPPLY_PROP_CHARGE_NOW,
- POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
- POWER_SUPPLY_PROP_CYCLE_COUNT,
- POWER_SUPPLY_PROP_HEALTH,
- POWER_SUPPLY_PROP_MANUFACTURER,
-};
-#define bq2752x_props bq2751x_props
+#define bq2751x_props bq27510g3_props
+#define bq2752x_props bq27510g3_props
static enum power_supply_property bq27500_props[] = {
POWER_SUPPLY_PROP_STATUS,
@@ -569,48 +496,8 @@ static enum power_supply_property bq27500_props[] = {
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_MANUFACTURER,
};
-
-static enum power_supply_property bq27510g1_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_NOW,
- 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,
-};
-
-static enum power_supply_property bq27510g2_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_NOW,
- 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,
-};
+#define bq27510g1_props bq27500_props
+#define bq27510g2_props bq27500_props
static enum power_supply_property bq27510g3_props[] = {
POWER_SUPPLY_PROP_STATUS,
--
2.13.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC v2 5/5] power: supply: bq27xxx: Remove duplicate chip data arrays
2017-08-07 6:22 ` [RFC v2 5/5] power: supply: bq27xxx: Remove duplicate chip data arrays Liam Breck
@ 2017-08-12 16:22 ` Sebastian Reichel
0 siblings, 0 replies; 19+ messages in thread
From: Sebastian Reichel @ 2017-08-12 16:22 UTC (permalink / raw)
To: Liam Breck; +Cc: Pali Rohár, linux-pm, Paul Kocialkowski, Liam Breck
[-- Attachment #1: Type: text/plain, Size: 6090 bytes --]
Hi,
On Sun, Aug 06, 2017 at 11:22:16PM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> BQ2751X & BQ27510G3 have identical regs & props.
> BQ27500 & BQ27510G1 & BQ27510G2 have identical regs & props.
> Remove the duplicate arrays.
>
> No functional changes to the driver.
Looks fine to me.
-- Sebastian
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq27xxx_battery.c | 129 ++-------------------------------
> 1 file changed, 8 insertions(+), 121 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
> index d85f9ec2..b451d49a 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -201,27 +201,8 @@ static u8
> [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> BQ27XXX_DM_REG_ROWS,
> },
> - bq2751x_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] = INVALID_REG_ADDR,
> - [BQ27XXX_REG_TTES] = 0x1a,
> - [BQ27XXX_REG_TTECP] = INVALID_REG_ADDR,
> - [BQ27XXX_REG_NAC] = 0x0c,
> - [BQ27XXX_REG_FCC] = 0x12,
> - [BQ27XXX_REG_CYCT] = 0x1e,
> - [BQ27XXX_REG_AE] = INVALID_REG_ADDR,
> - [BQ27XXX_REG_SOC] = 0x20,
> - [BQ27XXX_REG_DCAP] = 0x2e,
> - [BQ27XXX_REG_AP] = INVALID_REG_ADDR,
> - BQ27XXX_DM_REG_ROWS,
> - },
> -#define bq2752x_regs bq2751x_regs
> +#define bq2751x_regs bq27510g3_regs
> +#define bq2752x_regs bq27510g3_regs
> bq27500_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> @@ -242,46 +223,8 @@ static u8
> [BQ27XXX_REG_AP] = 0x24,
> BQ27XXX_DM_REG_ROWS,
> },
> - bq27510g1_regs[BQ27XXX_REG_MAX] = {
> - [BQ27XXX_REG_CTRL] = 0x00,
> - [BQ27XXX_REG_TEMP] = 0x06,
> - [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> - [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] = 0x26,
> - [BQ27XXX_REG_NAC] = 0x0c,
> - [BQ27XXX_REG_FCC] = 0x12,
> - [BQ27XXX_REG_CYCT] = 0x2a,
> - [BQ27XXX_REG_AE] = 0x22,
> - [BQ27XXX_REG_SOC] = 0x2c,
> - [BQ27XXX_REG_DCAP] = 0x3c,
> - [BQ27XXX_REG_AP] = 0x24,
> - BQ27XXX_DM_REG_ROWS,
> - },
> - bq27510g2_regs[BQ27XXX_REG_MAX] = {
> - [BQ27XXX_REG_CTRL] = 0x00,
> - [BQ27XXX_REG_TEMP] = 0x06,
> - [BQ27XXX_REG_INT_TEMP] = INVALID_REG_ADDR,
> - [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] = 0x26,
> - [BQ27XXX_REG_NAC] = 0x0c,
> - [BQ27XXX_REG_FCC] = 0x12,
> - [BQ27XXX_REG_CYCT] = 0x2a,
> - [BQ27XXX_REG_AE] = 0x22,
> - [BQ27XXX_REG_SOC] = 0x2c,
> - [BQ27XXX_REG_DCAP] = 0x3c,
> - [BQ27XXX_REG_AP] = 0x24,
> - BQ27XXX_DM_REG_ROWS,
> - },
> +#define bq27510g1_regs bq27500_regs
> +#define bq27510g2_regs bq27500_regs
> bq27510g3_regs[BQ27XXX_REG_MAX] = {
> [BQ27XXX_REG_CTRL] = 0x00,
> [BQ27XXX_REG_TEMP] = 0x06,
> @@ -530,24 +473,8 @@ static enum power_supply_property bq2750x_props[] = {
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
>
> -static enum power_supply_property bq2751x_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_TECHNOLOGY,
> - POWER_SUPPLY_PROP_CHARGE_FULL,
> - POWER_SUPPLY_PROP_CHARGE_NOW,
> - POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> - POWER_SUPPLY_PROP_CYCLE_COUNT,
> - POWER_SUPPLY_PROP_HEALTH,
> - POWER_SUPPLY_PROP_MANUFACTURER,
> -};
> -#define bq2752x_props bq2751x_props
> +#define bq2751x_props bq27510g3_props
> +#define bq2752x_props bq27510g3_props
>
> static enum power_supply_property bq27500_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> @@ -569,48 +496,8 @@ static enum power_supply_property bq27500_props[] = {
> POWER_SUPPLY_PROP_HEALTH,
> POWER_SUPPLY_PROP_MANUFACTURER,
> };
> -
> -static enum power_supply_property bq27510g1_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_NOW,
> - 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,
> -};
> -
> -static enum power_supply_property bq27510g2_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_NOW,
> - 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,
> -};
> +#define bq27510g1_props bq27500_props
> +#define bq27510g2_props bq27500_props
>
> static enum power_supply_property bq27510g3_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> --
> 2.13.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread