[PATCH 0/2] power: supply: ltc2941-battery-gauge: Support LTC2942

[PATCH 0/2] power: supply: ltc2941-battery-gauge: Support LTC2942

[Ladislav Michl]

LTC2942 is pin compatible with LTC2941 providing few more registers
holding information about battery voltage and temperature. It can
be runtime detected using bit A7 in the Status register.

PATCH 1/2 defines LTC2942 registers with no functional changes
while 2/2 does actually the job.

I'm not sure, whenever splitting support into two patches makes
sense, but it just makes reviewing easier. I'd happily
squash patches together.

Comments welcome,
	ladis

[PATCH 1/2] power: supply: ltc2941-battery-gauge: Define LTC2942 registers

[Ladislav Michl]

Add LTC2942 registers definitions. No functional changes.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 drivers/power/supply/ltc2941-battery-gauge.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
index 7efb908f4451..0a1b69bbca7f 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -34,16 +34,14 @@ enum ltc294x_reg {
 	LTC294X_REG_CONTROL		= 0x01,
 	LTC294X_REG_ACC_CHARGE_MSB	= 0x02,
 	LTC294X_REG_ACC_CHARGE_LSB	= 0x03,
-	LTC294X_REG_THRESH_HIGH_MSB	= 0x04,
-	LTC294X_REG_THRESH_HIGH_LSB	= 0x05,
-	LTC294X_REG_THRESH_LOW_MSB	= 0x06,
-	LTC294X_REG_THRESH_LOW_LSB	= 0x07,
-	LTC294X_REG_VOLTAGE_MSB	= 0x08,
-	LTC294X_REG_VOLTAGE_LSB	= 0x09,
-	LTC294X_REG_CURRENT_MSB	= 0x0E,
-	LTC294X_REG_CURRENT_LSB	= 0x0F,
-	LTC294X_REG_TEMPERATURE_MSB	= 0x14,
-	LTC294X_REG_TEMPERATURE_LSB	= 0x15,
+	LTC294X_REG_VOLTAGE_MSB		= 0x08,
+	LTC294X_REG_VOLTAGE_LSB		= 0x09,
+	LTC2942_REG_TEMPERATURE_MSB	= 0x0C,
+	LTC2942_REG_TEMPERATURE_LSB	= 0x0D,
+	LTC2943_REG_CURRENT_MSB		= 0x0E,
+	LTC2943_REG_CURRENT_LSB		= 0x0F,
+	LTC2943_REG_TEMPERATURE_MSB	= 0x14,
+	LTC2943_REG_TEMPERATURE_LSB	= 0x15,
 };
 
 #define LTC2943_REG_CONTROL_MODE_MASK (BIT(7) | BIT(6))
@@ -55,6 +53,7 @@ enum ltc294x_reg {
 #define LTC294X_REG_CONTROL_ALCC_CONFIG_DISABLED	0
 
 #define LTC2941_NUM_REGS	0x08
+#define LTC2942_NUM_REGS	0x10
 #define LTC2943_NUM_REGS	0x18
 
 struct ltc294x_info {
@@ -263,7 +262,7 @@ static int ltc294x_get_current(const struct ltc294x_info *info, int *val)
 	s32 value;
 
 	ret = ltc294x_read_regs(info->client,
-		LTC294X_REG_CURRENT_MSB, &datar[0], 2);
+		LTC2943_REG_CURRENT_MSB, &datar[0], 2);
 	value = (datar[0] << 8) | datar[1];
 	value -= 0x7FFF;
 	/* Value is in range -32k..+32k, r_sense is usually 10..50 mOhm,
@@ -280,7 +279,7 @@ static int ltc294x_get_temperature(const struct ltc294x_info *info, int *val)
 	u32 value;
 
 	ret = ltc294x_read_regs(info->client,
-		LTC294X_REG_TEMPERATURE_MSB, &datar[0], 2);
+		LTC2943_REG_TEMPERATURE_MSB, &datar[0], 2);
 	value = (datar[0] << 8) | datar[1];
 	/* Full-scale is 510 Kelvin, convert to centidegrees  */
 	*val = (((51000 * value) / 0xFFFF) - 27215);
@@ -424,10 +423,10 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 	info->client = client;
 	info->supply_desc.type = POWER_SUPPLY_TYPE_BATTERY;
 	info->supply_desc.properties = ltc294x_properties;
-	if (info->num_regs >= LTC294X_REG_TEMPERATURE_LSB)
+	if (info->num_regs >= LTC2943_REG_TEMPERATURE_LSB)
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties);
-	else if (info->num_regs >= LTC294X_REG_CURRENT_LSB)
+	else if (info->num_regs >= LTC2943_REG_CURRENT_LSB)
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties) - 1;
 	else if (info->num_regs >= LTC294X_REG_VOLTAGE_LSB)
-- 
2.11.0

[PATCH 2/2] power: supply: ltc2941-battery-gauge: Add support for LTC2942

[Ladislav Michl]

LTC2942 is pin compatible with LTC2941 providing additional
informations about battery voltage and temperature. It can
be runtime detected using bit A7 in the Status register.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 drivers/power/supply/ltc2941-battery-gauge.c | 56 ++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
index 0a1b69bbca7f..07e47478dbc6 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -44,6 +44,8 @@ enum ltc294x_reg {
 	LTC2943_REG_TEMPERATURE_LSB	= 0x15,
 };
 
+#define LTC2941_REG_STATUS_CHIP_ID	BIT(7)
+
 #define LTC2943_REG_CONTROL_MODE_MASK (BIT(7) | BIT(6))
 #define LTC2943_REG_CONTROL_MODE_SCAN BIT(7)
 #define LTC294X_REG_CONTROL_PRESCALER_MASK	(BIT(5) | BIT(4) | BIT(3))
@@ -250,8 +252,9 @@ static int ltc294x_get_voltage(const struct ltc294x_info *info, int *val)
 
 	ret = ltc294x_read_regs(info->client,
 		LTC294X_REG_VOLTAGE_MSB, &datar[0], 2);
-	value = (datar[0] << 8) | datar[1];
-	*val = ((value * 23600) / 0xFFFF) * 1000; /* in uV */
+	value = info->num_regs == LTC2943_NUM_REGS ? 23600 : 6000;
+	value *= (datar[0] << 8) | datar[1];
+	*val = 1000 * value / 0xFFFF; /* in uV */
 	return ret;
 }
 
@@ -274,15 +277,22 @@ static int ltc294x_get_current(const struct ltc294x_info *info, int *val)
 
 static int ltc294x_get_temperature(const struct ltc294x_info *info, int *val)
 {
+	enum ltc294x_reg reg;
 	int ret;
 	u8 datar[2];
 	u32 value;
 
-	ret = ltc294x_read_regs(info->client,
-		LTC2943_REG_TEMPERATURE_MSB, &datar[0], 2);
-	value = (datar[0] << 8) | datar[1];
-	/* Full-scale is 510 Kelvin, convert to centidegrees  */
-	*val = (((51000 * value) / 0xFFFF) - 27215);
+	if (info->num_regs == LTC2943_NUM_REGS) {
+		reg = LTC2943_REG_TEMPERATURE_MSB;
+		value = 51000;	/* Full-scale is 510 Kelvin */
+	} else {
+		reg = LTC2942_REG_TEMPERATURE_MSB;
+		value = 60000;	/* Full-scale is 600 Kelvin */
+	}
+	ret = ltc294x_read_regs(info->client, reg, &datar[0], 2);
+	value *= (datar[0] << 8) | datar[1];
+	/* Convert to centidegrees  */
+	*val = value / 0xFFFF - 27215;
 	return ret;
 }
 
@@ -356,8 +366,8 @@ static enum power_supply_property ltc294x_properties[] = {
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
-	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
 };
 
 static int ltc294x_i2c_remove(struct i2c_client *client)
@@ -374,10 +384,11 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 {
 	struct power_supply_config psy_cfg = {};
 	struct ltc294x_info *info;
+	struct device_node *np;
 	int ret;
 	u32 prescaler_exp;
 	s32 r_sense;
-	struct device_node *np;
+	u8 status;
 
 	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
 	if (info == NULL)
@@ -420,21 +431,36 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 				(128 / (1 << prescaler_exp));
 	}
 
+	/* Read status register to check for LTC2942 */
+	if (info->num_regs == LTC2941_NUM_REGS) {
+		ret = ltc294x_read_regs(client, LTC294X_REG_STATUS, &status, 1);
+		if (ret < 0) {
+			dev_err(&info->client->dev,
+				"Could not read status register\n");
+			return ret;
+		}
+		if (!(status & LTC2941_REG_STATUS_CHIP_ID))
+			info->num_regs = LTC2942_NUM_REGS;
+	}
+
 	info->client = client;
 	info->supply_desc.type = POWER_SUPPLY_TYPE_BATTERY;
 	info->supply_desc.properties = ltc294x_properties;
-	if (info->num_regs >= LTC2943_REG_TEMPERATURE_LSB)
+	switch (info->num_regs) {
+	case LTC2943_NUM_REGS:
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties);
-	else if (info->num_regs >= LTC2943_REG_CURRENT_LSB)
+		break;
+	case LTC2942_NUM_REGS:
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties) - 1;
-	else if (info->num_regs >= LTC294X_REG_VOLTAGE_LSB)
-		info->supply_desc.num_properties =
-			ARRAY_SIZE(ltc294x_properties) - 2;
-	else
+		break;
+	case LTC2941_NUM_REGS:
+	default:
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties) - 3;
+		break;
+	}
 	info->supply_desc.get_property = ltc294x_get_property;
 	info->supply_desc.set_property = ltc294x_set_property;
 	info->supply_desc.property_is_writeable = ltc294x_property_is_writeable;
-- 
2.11.0

Re: [PATCH 1/2] power: supply: ltc2941-battery-gauge: Define LTC2942 registers

[Mike Looijmans]

See below for my comment...

On 12-6-2017 17:03, Ladislav Michl wrote:
> Add LTC2942 registers definitions. No functional changes.
>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  drivers/power/supply/ltc2941-battery-gauge.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
> index 7efb908f4451..0a1b69bbca7f 100644
> --- a/drivers/power/supply/ltc2941-battery-gauge.c
> +++ b/drivers/power/supply/ltc2941-battery-gauge.c
> @@ -34,16 +34,14 @@ enum ltc294x_reg {
>  	LTC294X_REG_CONTROL		= 0x01,
>  	LTC294X_REG_ACC_CHARGE_MSB	= 0x02,
>  	LTC294X_REG_ACC_CHARGE_LSB	= 0x03,
> -	LTC294X_REG_THRESH_HIGH_MSB	= 0x04,
> -	LTC294X_REG_THRESH_HIGH_LSB	= 0x05,
> -	LTC294X_REG_THRESH_LOW_MSB	= 0x06,
> -	LTC294X_REG_THRESH_LOW_LSB	= 0x07,
> -	LTC294X_REG_VOLTAGE_MSB	= 0x08,
> -	LTC294X_REG_VOLTAGE_LSB	= 0x09,
> -	LTC294X_REG_CURRENT_MSB	= 0x0E,
> -	LTC294X_REG_CURRENT_LSB	= 0x0F,
> -	LTC294X_REG_TEMPERATURE_MSB	= 0x14,
> -	LTC294X_REG_TEMPERATURE_LSB	= 0x15,
> +	LTC294X_REG_VOLTAGE_MSB		= 0x08,
> +	LTC294X_REG_VOLTAGE_LSB		= 0x09,
> +	LTC2942_REG_TEMPERATURE_MSB	= 0x0C,
> +	LTC2942_REG_TEMPERATURE_LSB	= 0x0D,
> +	LTC2943_REG_CURRENT_MSB		= 0x0E,
> +	LTC2943_REG_CURRENT_LSB		= 0x0F,
> +	LTC2943_REG_TEMPERATURE_MSB	= 0x14,
> +	LTC2943_REG_TEMPERATURE_LSB	= 0x15,
>  };

I don't see the advantage of renaming these, I'd rather see the LTC294X 
naming throughout, making it easier to locate them in the enum as they 
all share the exact same prefix.

But if you do want this renaming, I guess you should also rename the 
LTC2941 registers.

I'm neutral to either naming, linux-pm maintainer will have the final 
word I guess.



>
>  #define LTC2943_REG_CONTROL_MODE_MASK (BIT(7) | BIT(6))
> @@ -55,6 +53,7 @@ enum ltc294x_reg {
>  #define LTC294X_REG_CONTROL_ALCC_CONFIG_DISABLED	0
>
>  #define LTC2941_NUM_REGS	0x08
> +#define LTC2942_NUM_REGS	0x10
>  #define LTC2943_NUM_REGS	0x18
>
>  struct ltc294x_info {
> @@ -263,7 +262,7 @@ static int ltc294x_get_current(const struct ltc294x_info *info, int *val)
>  	s32 value;
>
>  	ret = ltc294x_read_regs(info->client,
> -		LTC294X_REG_CURRENT_MSB, &datar[0], 2);
> +		LTC2943_REG_CURRENT_MSB, &datar[0], 2);
>  	value = (datar[0] << 8) | datar[1];
>  	value -= 0x7FFF;
>  	/* Value is in range -32k..+32k, r_sense is usually 10..50 mOhm,
> @@ -280,7 +279,7 @@ static int ltc294x_get_temperature(const struct ltc294x_info *info, int *val)
>  	u32 value;
>
>  	ret = ltc294x_read_regs(info->client,
> -		LTC294X_REG_TEMPERATURE_MSB, &datar[0], 2);
> +		LTC2943_REG_TEMPERATURE_MSB, &datar[0], 2);
>  	value = (datar[0] << 8) | datar[1];
>  	/* Full-scale is 510 Kelvin, convert to centidegrees  */
>  	*val = (((51000 * value) / 0xFFFF) - 27215);
> @@ -424,10 +423,10 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
>  	info->client = client;
>  	info->supply_desc.type = POWER_SUPPLY_TYPE_BATTERY;
>  	info->supply_desc.properties = ltc294x_properties;
> -	if (info->num_regs >= LTC294X_REG_TEMPERATURE_LSB)
> +	if (info->num_regs >= LTC2943_REG_TEMPERATURE_LSB)
>  		info->supply_desc.num_properties =
>  			ARRAY_SIZE(ltc294x_properties);
> -	else if (info->num_regs >= LTC294X_REG_CURRENT_LSB)
> +	else if (info->num_regs >= LTC2943_REG_CURRENT_LSB)
>  		info->supply_desc.num_properties =
>  			ARRAY_SIZE(ltc294x_properties) - 1;
>  	else if (info->num_regs >= LTC294X_REG_VOLTAGE_LSB)
>


-- 
Mike Looijmans


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: [PATCH 2/2] power: supply: ltc2941-battery-gauge: Add support for LTC2942

[Mike Looijmans]

On 12-6-2017 17:04, Ladislav Michl wrote:
> LTC2942 is pin compatible with LTC2941 providing additional
> informations about battery voltage and temperature. It can
> be runtime detected using bit A7 in the Status register.

Runtime detection is always nice. I'd suggest adding "ltc2942" to the 
device name list, since having to specify "ltc2941" when the chip is in 
fact an "ltc2942" looks a bit strange.

Again, no strong preference, whatever the maintainers think is best is 
fine with me also.

>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  drivers/power/supply/ltc2941-battery-gauge.c | 56 ++++++++++++++++++++--------
>  1 file changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
> index 0a1b69bbca7f..07e47478dbc6 100644
> --- a/drivers/power/supply/ltc2941-battery-gauge.c
> +++ b/drivers/power/supply/ltc2941-battery-gauge.c
> @@ -44,6 +44,8 @@ enum ltc294x_reg {
>  	LTC2943_REG_TEMPERATURE_LSB	= 0x15,
>  };
>
> +#define LTC2941_REG_STATUS_CHIP_ID	BIT(7)
> +
>  #define LTC2943_REG_CONTROL_MODE_MASK (BIT(7) | BIT(6))
>  #define LTC2943_REG_CONTROL_MODE_SCAN BIT(7)
>  #define LTC294X_REG_CONTROL_PRESCALER_MASK	(BIT(5) | BIT(4) | BIT(3))
> @@ -250,8 +252,9 @@ static int ltc294x_get_voltage(const struct ltc294x_info *info, int *val)
>
>  	ret = ltc294x_read_regs(info->client,
>  		LTC294X_REG_VOLTAGE_MSB, &datar[0], 2);
> -	value = (datar[0] << 8) | datar[1];
> -	*val = ((value * 23600) / 0xFFFF) * 1000; /* in uV */
> +	value = info->num_regs == LTC2943_NUM_REGS ? 23600 : 6000;
> +	value *= (datar[0] << 8) | datar[1];
> +	*val = 1000 * value / 0xFFFF; /* in uV */
>  	return ret;
>  }
>
> @@ -274,15 +277,22 @@ static int ltc294x_get_current(const struct ltc294x_info *info, int *val)
>
>  static int ltc294x_get_temperature(const struct ltc294x_info *info, int *val)
>  {
> +	enum ltc294x_reg reg;
>  	int ret;
>  	u8 datar[2];
>  	u32 value;
>
> -	ret = ltc294x_read_regs(info->client,
> -		LTC2943_REG_TEMPERATURE_MSB, &datar[0], 2);
> -	value = (datar[0] << 8) | datar[1];
> -	/* Full-scale is 510 Kelvin, convert to centidegrees  */
> -	*val = (((51000 * value) / 0xFFFF) - 27215);
> +	if (info->num_regs == LTC2943_NUM_REGS) {
> +		reg = LTC2943_REG_TEMPERATURE_MSB;
> +		value = 51000;	/* Full-scale is 510 Kelvin */
> +	} else {
> +		reg = LTC2942_REG_TEMPERATURE_MSB;
> +		value = 60000;	/* Full-scale is 600 Kelvin */
> +	}
> +	ret = ltc294x_read_regs(info->client, reg, &datar[0], 2);
> +	value *= (datar[0] << 8) | datar[1];
> +	/* Convert to centidegrees  */
> +	*val = value / 0xFFFF - 27215;
>  	return ret;
>  }
>
> @@ -356,8 +366,8 @@ static enum power_supply_property ltc294x_properties[] = {
>  	POWER_SUPPLY_PROP_CHARGE_COUNTER,
>  	POWER_SUPPLY_PROP_CHARGE_NOW,
>  	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> -	POWER_SUPPLY_PROP_CURRENT_NOW,
>  	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
>  };
>
>  static int ltc294x_i2c_remove(struct i2c_client *client)
> @@ -374,10 +384,11 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
>  {
>  	struct power_supply_config psy_cfg = {};
>  	struct ltc294x_info *info;
> +	struct device_node *np;
>  	int ret;
>  	u32 prescaler_exp;
>  	s32 r_sense;
> -	struct device_node *np;
> +	u8 status;
>
>  	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
>  	if (info == NULL)
> @@ -420,21 +431,36 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
>  				(128 / (1 << prescaler_exp));
>  	}
>
> +	/* Read status register to check for LTC2942 */
> +	if (info->num_regs == LTC2941_NUM_REGS) {
> +		ret = ltc294x_read_regs(client, LTC294X_REG_STATUS, &status, 1);
> +		if (ret < 0) {
> +			dev_err(&info->client->dev,
> +				"Could not read status register\n");
> +			return ret;
> +		}
> +		if (!(status & LTC2941_REG_STATUS_CHIP_ID))
> +			info->num_regs = LTC2942_NUM_REGS;
> +	}
> +
>  	info->client = client;
>  	info->supply_desc.type = POWER_SUPPLY_TYPE_BATTERY;
>  	info->supply_desc.properties = ltc294x_properties;
> -	if (info->num_regs >= LTC2943_REG_TEMPERATURE_LSB)
> +	switch (info->num_regs) {
> +	case LTC2943_NUM_REGS:
>  		info->supply_desc.num_properties =
>  			ARRAY_SIZE(ltc294x_properties);
> -	else if (info->num_regs >= LTC2943_REG_CURRENT_LSB)
> +		break;
> +	case LTC2942_NUM_REGS:
>  		info->supply_desc.num_properties =
>  			ARRAY_SIZE(ltc294x_properties) - 1;
> -	else if (info->num_regs >= LTC294X_REG_VOLTAGE_LSB)
> -		info->supply_desc.num_properties =
> -			ARRAY_SIZE(ltc294x_properties) - 2;
> -	else
> +		break;
> +	case LTC2941_NUM_REGS:
> +	default:
>  		info->supply_desc.num_properties =
>  			ARRAY_SIZE(ltc294x_properties) - 3;
> +		break;
> +	}
>  	info->supply_desc.get_property = ltc294x_get_property;
>  	info->supply_desc.set_property = ltc294x_set_property;
>  	info->supply_desc.property_is_writeable = ltc294x_property_is_writeable;
>


-- 
Mike Looijmans


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: [PATCH 1/2] power: supply: ltc2941-battery-gauge: Define LTC2942 registers

[Ladislav Michl]

On Mon, Jun 12, 2017 at 06:26:17PM +0200, Mike Looijmans wrote:
> On 12-6-2017 17:03, Ladislav Michl wrote:
> > Add LTC2942 registers definitions. No functional changes.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  drivers/power/supply/ltc2941-battery-gauge.c | 27 +++++++++++++--------------
> >  1 file changed, 13 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
> > index 7efb908f4451..0a1b69bbca7f 100644
> > --- a/drivers/power/supply/ltc2941-battery-gauge.c
> > +++ b/drivers/power/supply/ltc2941-battery-gauge.c
> > @@ -34,16 +34,14 @@ enum ltc294x_reg {
> >  	LTC294X_REG_CONTROL		= 0x01,
> >  	LTC294X_REG_ACC_CHARGE_MSB	= 0x02,
> >  	LTC294X_REG_ACC_CHARGE_LSB	= 0x03,
> > -	LTC294X_REG_THRESH_HIGH_MSB	= 0x04,
> > -	LTC294X_REG_THRESH_HIGH_LSB	= 0x05,
> > -	LTC294X_REG_THRESH_LOW_MSB	= 0x06,
> > -	LTC294X_REG_THRESH_LOW_LSB	= 0x07,
> > -	LTC294X_REG_VOLTAGE_MSB	= 0x08,
> > -	LTC294X_REG_VOLTAGE_LSB	= 0x09,
> > -	LTC294X_REG_CURRENT_MSB	= 0x0E,
> > -	LTC294X_REG_CURRENT_LSB	= 0x0F,
> > -	LTC294X_REG_TEMPERATURE_MSB	= 0x14,
> > -	LTC294X_REG_TEMPERATURE_LSB	= 0x15,
> > +	LTC294X_REG_VOLTAGE_MSB		= 0x08,
> > +	LTC294X_REG_VOLTAGE_LSB		= 0x09,
> > +	LTC2942_REG_TEMPERATURE_MSB	= 0x0C,
> > +	LTC2942_REG_TEMPERATURE_LSB	= 0x0D,
> > +	LTC2943_REG_CURRENT_MSB		= 0x0E,
> > +	LTC2943_REG_CURRENT_LSB		= 0x0F,
> > +	LTC2943_REG_TEMPERATURE_MSB	= 0x14,
> > +	LTC2943_REG_TEMPERATURE_LSB	= 0x15,
> >  };
> 
> I don't see the advantage of renaming these, I'd rather see the LTC294X
> naming throughout, making it easier to locate them in the enum as they all
> share the exact same prefix.

LTC2942 and LTC2943 are using different register offsets for temperature
and current register is LTC2943 specific. Hence the rename, otherwise you
won't be able to distinguish between temperature registers in different chips.

> But if you do want this renaming, I guess you should also rename the LTC2941
> registers.

Well, naming convention is to use LTC294X prefix only for registers common
to all chips. Chip specific registers are using chip specific prefixes. Also
I'm not big fan of having register offsets as enums - offset is a number and
should be represented as such.

> I'm neutral to either naming, linux-pm maintainer will have the final word I
> guess.

I have no problem rolling change back with exception of tepmerature registers
(LTC294X_LTC2942_REG_TEMPERATURE_MSB would probably be an option, but such a
long identifier hurts my eyes).

Let's wait for maintainer's comment.

Best regards,
	ladis

Re: [PATCH 2/2] power: supply: ltc2941-battery-gauge: Add support for LTC2942

[Ladislav Michl]

On Mon, Jun 12, 2017 at 06:29:54PM +0200, Mike Looijmans wrote:
> On 12-6-2017 17:04, Ladislav Michl wrote:
> > LTC2942 is pin compatible with LTC2941 providing additional
> > informations about battery voltage and temperature. It can
> > be runtime detected using bit A7 in the Status register.
> 
> Runtime detection is always nice. I'd suggest adding "ltc2942" to the device
> name list, since having to specify "ltc2941" when the chip is in fact an
> "ltc2942" looks a bit strange.

It is not that strange after the one who would otherwise blindly type ltc2942
reads the datasheet ;-)

> Again, no strong preference, whatever the maintainers think is best is fine
> with me also.

What about adding something like this?

diff --git a/Documentation/devicetree/bindings/power/supply/ltc2941.txt b/Documentation/devicetree/bindings/power/supply/ltc2941.txt
index a9d7aa60558b..0d065d54d294 100644
--- a/Documentation/devicetree/bindings/power/supply/ltc2941.txt
+++ b/Documentation/devicetree/bindings/power/supply/ltc2941.txt
@@ -1,9 +1,10 @@
-binding for LTC2941 and LTC2943 battery gauges
+binding for LTC2941, LTC2942 and LTC2943 battery gauges
 
-Both the LTC2941 and LTC2943 measure battery capacity.
-The LTC2943 is compatible with the LTC2941, it adds voltage and
-temperature monitoring, and uses a slightly different conversion
-formula for the charge counter.
+All chips measure battery capacity.
+The LTC2942 is pin compatible with the LTC2941, it adds voltage and
+temperature monitoring, and is runtime detected. LTC2943 is software
+compatible, uses a slightly different conversion formula for the
+charge counter and adds voltage, current and temperature monitoring.
 
 Required properties:
 - compatible: Should contain "lltc,ltc2941" or "lltc,ltc2943" which also

Re: [PATCH 2/2] power: supply: ltc2941-battery-gauge: Add support for LTC2942

[Ladislav Michl]

On Mon, Jun 12, 2017 at 06:29:54PM +0200, Mike Looijmans wrote:
> On 12-6-2017 17:04, Ladislav Michl wrote:
> > LTC2942 is pin compatible with LTC2941 providing additional
> > informations about battery voltage and temperature. It can
> > be runtime detected using bit A7 in the Status register.
> 
> Runtime detection is always nice. I'd suggest adding "ltc2942" to the device
> name list, since having to specify "ltc2941" when the chip is in fact an
> "ltc2942" looks a bit strange.
> 
> Again, no strong preference, whatever the maintainers think is best is fine
> with me also.

Actually, could you or someone else give it a try on ltc2943 to see if I
didn't break anything? Updated version follows...

Subject: [PATCH v2 2/2] power: supply: ltc2941-battery-gauge: Add support for LTC2942

LTC2942 is pin compatible with LTC2941 providing additional
informations about battery voltage and temperature. It can
be runtime detected using bit A7 in the Status register.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes since v1: reworked voltage computing to not overflow and keep presision

 drivers/power/supply/ltc2941-battery-gauge.c | 76 +++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
index 0a1b69bbca7f..65e67cd44c5c 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -44,8 +44,10 @@ enum ltc294x_reg {
 	LTC2943_REG_TEMPERATURE_LSB	= 0x15,
 };
 
-#define LTC2943_REG_CONTROL_MODE_MASK (BIT(7) | BIT(6))
-#define LTC2943_REG_CONTROL_MODE_SCAN BIT(7)
+#define LTC2941_REG_STATUS_CHIP_ID	BIT(7)
+
+#define LTC2942_REG_CONTROL_MODE_SCAN	(BIT(7) | BIT(6))
+#define LTC2943_REG_CONTROL_MODE_SCAN	BIT(7)
 #define LTC294X_REG_CONTROL_PRESCALER_MASK	(BIT(5) | BIT(4) | BIT(3))
 #define LTC294X_REG_CONTROL_SHUTDOWN_MASK	(BIT(0))
 #define LTC294X_REG_CONTROL_PRESCALER_SET(x) \
@@ -144,9 +146,15 @@ static int ltc294x_reset(const struct ltc294x_info *info, int prescaler_exp)
 
 	control = LTC294X_REG_CONTROL_PRESCALER_SET(prescaler_exp) |
 				LTC294X_REG_CONTROL_ALCC_CONFIG_DISABLED;
-	/* Put the 2943 into "monitor" mode, so it measures every 10 sec */
-	if (info->num_regs == LTC2943_NUM_REGS)
+	/* Put device into "monitor" mode */
+	switch (info->num_regs) {
+	case LTC2942_NUM_REGS:	/* 2942 measures every 2 sec */
+		control |= LTC2942_REG_CONTROL_MODE_SCAN;
+		break;
+	case LTC2943_NUM_REGS:	/* 2943 measures every 10 sec */
 		control |= LTC2943_REG_CONTROL_MODE_SCAN;
+		break;
+	}
 
 	if (value != control) {
 		ret = ltc294x_write_regs(info->client,
@@ -251,7 +259,16 @@ static int ltc294x_get_voltage(const struct ltc294x_info *info, int *val)
 	ret = ltc294x_read_regs(info->client,
 		LTC294X_REG_VOLTAGE_MSB, &datar[0], 2);
 	value = (datar[0] << 8) | datar[1];
-	*val = ((value * 23600) / 0xFFFF) * 1000; /* in uV */
+	if (info->num_regs == LTC2943_NUM_REGS) {
+		value *= 23600 * 2;
+		value /= 0xFFFF;
+		value *= 1000 / 2;
+	} else {
+		value *= 6000 * 10;
+		value /= 0xFFFF;
+		value *= 1000 / 10;
+	}
+	*val = value;
 	return ret;
 }
 
@@ -274,15 +291,22 @@ static int ltc294x_get_current(const struct ltc294x_info *info, int *val)
 
 static int ltc294x_get_temperature(const struct ltc294x_info *info, int *val)
 {
+	enum ltc294x_reg reg;
 	int ret;
 	u8 datar[2];
 	u32 value;
 
-	ret = ltc294x_read_regs(info->client,
-		LTC2943_REG_TEMPERATURE_MSB, &datar[0], 2);
-	value = (datar[0] << 8) | datar[1];
-	/* Full-scale is 510 Kelvin, convert to centidegrees  */
-	*val = (((51000 * value) / 0xFFFF) - 27215);
+	if (info->num_regs == LTC2943_NUM_REGS) {
+		reg = LTC2943_REG_TEMPERATURE_MSB;
+		value = 51000;	/* Full-scale is 510 Kelvin */
+	} else {
+		reg = LTC2942_REG_TEMPERATURE_MSB;
+		value = 60000;	/* Full-scale is 600 Kelvin */
+	}
+	ret = ltc294x_read_regs(info->client, reg, &datar[0], 2);
+	value *= (datar[0] << 8) | datar[1];
+	/* Convert to centidegrees  */
+	*val = value / 0xFFFF - 27215;
 	return ret;
 }
 
@@ -356,8 +380,8 @@ static enum power_supply_property ltc294x_properties[] = {
 	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_CHARGE_NOW,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
-	POWER_SUPPLY_PROP_CURRENT_NOW,
 	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
 };
 
 static int ltc294x_i2c_remove(struct i2c_client *client)
@@ -374,10 +398,11 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 {
 	struct power_supply_config psy_cfg = {};
 	struct ltc294x_info *info;
+	struct device_node *np;
 	int ret;
 	u32 prescaler_exp;
 	s32 r_sense;
-	struct device_node *np;
+	u8 status;
 
 	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
 	if (info == NULL)
@@ -420,21 +445,36 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 				(128 / (1 << prescaler_exp));
 	}
 
+	/* Read status register to check for LTC2942 */
+	if (info->num_regs == LTC2941_NUM_REGS) {
+		ret = ltc294x_read_regs(client, LTC294X_REG_STATUS, &status, 1);
+		if (ret < 0) {
+			dev_err(&info->client->dev,
+				"Could not read status register\n");
+			return ret;
+		}
+		if (!(status & LTC2941_REG_STATUS_CHIP_ID))
+			info->num_regs = LTC2942_NUM_REGS;
+	}
+
 	info->client = client;
 	info->supply_desc.type = POWER_SUPPLY_TYPE_BATTERY;
 	info->supply_desc.properties = ltc294x_properties;
-	if (info->num_regs >= LTC2943_REG_TEMPERATURE_LSB)
+	switch (info->num_regs) {
+	case LTC2943_NUM_REGS:
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties);
-	else if (info->num_regs >= LTC2943_REG_CURRENT_LSB)
+		break;
+	case LTC2942_NUM_REGS:
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties) - 1;
-	else if (info->num_regs >= LTC294X_REG_VOLTAGE_LSB)
-		info->supply_desc.num_properties =
-			ARRAY_SIZE(ltc294x_properties) - 2;
-	else
+		break;
+	case LTC2941_NUM_REGS:
+	default:
 		info->supply_desc.num_properties =
 			ARRAY_SIZE(ltc294x_properties) - 3;
+		break;
+	}
 	info->supply_desc.get_property = ltc294x_get_property;
 	info->supply_desc.set_property = ltc294x_set_property;
 	info->supply_desc.property_is_writeable = ltc294x_property_is_writeable;
-- 
2.11.0