Re: [PATCH 3/3] power: supply: max17040: Set rcomp value

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

Hi,

On Mon, May 04, 2020 at 03:13:00PM -0700, Jonathan Bakker wrote:
> According to the datasheet (1), the rcomp parameter can
> vary based on the typical operating temperature and the
> battery chemistry.  If provided, make sure we set it after
> we reset the chip on boot.
> 
> 1) https://datasheets.maximintegrated.com/en/ds/MAX17040-MAX17041.pdf
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  drivers/power/supply/max17040_battery.c | 33 +++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
> index 48aa44665e2f..f66e2fdc0a8a 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -10,6 +10,7 @@
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
>  #include <linux/mutex.h>
> +#include <linux/property.h>
>  #include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/delay.h>
> @@ -31,6 +32,8 @@
>  
>  #define MAX17040_ATHD_MASK		0xFFC0
>  #define MAX17040_ATHD_DEFAULT_POWER_UP	4
> +#define MAX17040_RCOMP_MASK		0xFF
> +#define MAX17040_RCOMP_DEFAULT_POWER_UP	0x97

Why is this 8 bits? Quote from the datasheet, that you linked:

»RCOMP is a 16-bit value used to compensate the ModelGauge algorithm«

-- Sebastian

>  struct max17040_chip {
>  	struct i2c_client		*client;
> @@ -48,6 +51,8 @@ struct max17040_chip {
>  	int status;
>  	/* Low alert threshold from 32% to 1% of the State of Charge */
>  	u32 low_soc_alert;
> +	/* Optimization for specific chemistries */
> +	u8 rcomp_value;
>  };
>  
>  static int max17040_get_property(struct power_supply *psy,
> @@ -119,6 +124,20 @@ static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)
>  	return ret;
>  }
>  
> +static int max17040_set_rcomp(struct i2c_client *client, u32 val)
> +{
> +	int ret;
> +	u16 data;
> +
> +	data = max17040_read_reg(client, MAX17040_RCOMP);
> +	/* clear the rcomp val and set MSb 8 bits */
> +	data &= MAX17040_RCOMP_MASK;
> +	data |= val << 8;
> +	ret = max17040_write_reg(client, MAX17040_RCOMP, data);
> +
> +	return ret;
> +}
> +
>  static void max17040_get_vcell(struct i2c_client *client)
>  {
>  	struct max17040_chip *chip = i2c_get_clientdata(client);
> @@ -190,8 +209,14 @@ static int max17040_get_of_data(struct max17040_chip *chip)
>  				 "maxim,alert-low-soc-level",
>  				 &chip->low_soc_alert);
>  
> -	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33)
> +	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33) {
> +		dev_err(&client->dev,
> +			"failed: low SOC alert OF data out of bounds\n");
>  		return -EINVAL;
> +	}
> +
> +	chip->rcomp_value = MAX17040_RCOMP_DEFAULT_POWER_UP;
> +	device_property_read_u8(dev, "maxim,rcomp-value", &chip->rcomp_value);
>  
>  	return 0;
>  }
> @@ -289,11 +314,8 @@ static int max17040_probe(struct i2c_client *client,
>  	chip->client = client;
>  	chip->pdata = client->dev.platform_data;
>  	ret = max17040_get_of_data(chip);
> -	if (ret) {
> -		dev_err(&client->dev,
> -			"failed: low SOC alert OF data out of bounds\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	i2c_set_clientdata(client, chip);
>  	psy_cfg.drv_data = chip;
> @@ -307,6 +329,7 @@ static int max17040_probe(struct i2c_client *client,
>  
>  	max17040_reset(client);
>  	max17040_get_version(client);
> +	max17040_set_rcomp(client, chip->rcomp_value);
>  
>  	/* check interrupt */
>  	if (client->irq && of_device_is_compatible(client->dev.of_node,
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]