Hi Sebastian, On 2020-05-10 1:08 p.m., Sebastian Reichel wrote: > 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@xxxxxxx> >> --- >> 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« Well, the driver also supports the max17043 (datasheet at https://datasheets.maximintegrated.com/en/ds/MAX17043-MAX17044.pdf here the register is named CONFIG), by the maxim,max77836-battery compatible. The bottom 8 bits are for the alert config, and I'm presuming it's the same on the max17040 (the vendor kernel for the device I'm testing on only sets the top 8 bits and leaves the rest at 0). If there's a better way of doing it (ie maybe explicitly making it a 16 bit value and checking if the bottom 8 bits are set when the compatible is maxim,max77836-battery), then I'm happy to do it that way. Thanks, Jonathan > > -- 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 >>
