* [PATCH 0/5] MAX17042 add support for maxim POR procedure @ 2012-01-24 17:26 dirk.brandewie 2012-01-24 17:26 ` [PATCH 1/5] max17042: Align register definitions with data sheet and init appnote dirk.brandewie ` (6 more replies) 0 siblings, 7 replies; 17+ messages in thread From: dirk.brandewie @ 2012-01-24 17:26 UTC (permalink / raw) To: linux-kernel Cc: cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Dirk Brandewie From: Dirk Brandewie <dirk.brandewie@gmail.com> This patch set adds support for the power on reset procedure for the max17042 battery fuel gauge and alert interrupts. The accuracy of the fuel gauge is improved by configuring the fuel gauge with the characterization data for the battery present in the platform. Changes since v3: Patch added patch to fix scaling of VCELL register values reported Capacity property now used the reported SOC register to account for empty compensation in reported capacity. Fixed sparse/smatch warnings Changes since v2: Dropped patch moving non-POR init to worker function. Reorganized POR patch to only schedule worker thread if the fuel gauge is in the POR state Changes since v1: Moved power_supply_register() back to the probe max17042_get_property() will return -EAGAIN until the init function completes Added support for interrupts from the alert pin if the platform has it connected to an interrupt source. Added fix for the values returned by POWER_SUPPLY_PROP_VOLTAGE_NOW and POWER_SUPPLY_PROP_VOLTAGE_AVG. Added Ack's from previous patch series Bruce Robertson (1): max17042: Fix value scaling for VCELL and avgVCELL Dirk Brandewie (4): max17042: Align register definitions with data sheet and init appnote max17042: Add POR init procedure from Maxim appnote max17042: Add support for signalling change in SOC max17042: Change capacity property to use reported SOC register drivers/power/max17042_battery.c | 452 +++++++++++++++++++++++++++++++- include/linux/power/max17042_battery.h | 93 ++++++- 2 files changed, 522 insertions(+), 23 deletions(-) -- 1.7.7.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5] max17042: Align register definitions with data sheet and init appnote 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie @ 2012-01-24 17:26 ` dirk.brandewie 2012-01-24 17:26 ` [PATCH 2/5] max17042: Add POR init procedure from Maxim appnote dirk.brandewie ` (5 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: dirk.brandewie @ 2012-01-24 17:26 UTC (permalink / raw) To: linux-kernel Cc: cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Dirk Brandewie From: Dirk Brandewie <dirk.brandewie@gmail.com> align the register names with max17042 data sheet removing registers that are marked reserved that are not used. Add register definitions defined in the maxim initialization appnote Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> --- include/linux/power/max17042_battery.h | 37 ++++++++++++++++++++++++------- 1 files changed, 28 insertions(+), 9 deletions(-) diff --git a/include/linux/power/max17042_battery.h b/include/linux/power/max17042_battery.h index fe99211..67eeada 100644 --- a/include/linux/power/max17042_battery.h +++ b/include/linux/power/max17042_battery.h @@ -40,11 +40,11 @@ enum max17042_register { MAX17042_VCELL = 0x09, MAX17042_Current = 0x0A, MAX17042_AvgCurrent = 0x0B, - MAX17042_Qresidual = 0x0C, + MAX17042_SOC = 0x0D, MAX17042_AvSOC = 0x0E, MAX17042_RemCap = 0x0F, - MAX17402_FullCAP = 0x10, + MAX17042_FullCAP = 0x10, MAX17042_TTE = 0x11, MAX17042_V_empty = 0x12, @@ -62,14 +62,14 @@ enum max17042_register { MAX17042_AvCap = 0x1F, MAX17042_ManName = 0x20, MAX17042_DevName = 0x21, - MAX17042_DevChem = 0x22, + MAX17042_FullCAPNom = 0x23, MAX17042_TempNom = 0x24, - MAX17042_TempCold = 0x25, + MAX17042_TempLim = 0x25, MAX17042_TempHot = 0x26, MAX17042_AIN = 0x27, MAX17042_LearnCFG = 0x28, - MAX17042_SHFTCFG = 0x29, + MAX17042_FilterCFG = 0x29, MAX17042_RelaxCFG = 0x2A, MAX17042_MiscCFG = 0x2B, MAX17042_TGAIN = 0x2C, @@ -77,22 +77,41 @@ enum max17042_register { MAX17042_CGAIN = 0x2E, MAX17042_COFF = 0x2F, - MAX17042_Q_empty = 0x33, + MAX17042_MaskSOC = 0x32, + MAX17042_SOC_empty = 0x33, MAX17042_T_empty = 0x34, + MAX17042_FullCAP0 = 0x35, + MAX17042_LAvg_empty = 0x36, + MAX17042_FCTC = 0x37, MAX17042_RCOMP0 = 0x38, MAX17042_TempCo = 0x39, - MAX17042_Rx = 0x3A, - MAX17042_T_empty0 = 0x3B, + MAX17042_EmptyTempCo = 0x3A, + MAX17042_K_empty0 = 0x3B, MAX17042_TaskPeriod = 0x3C, MAX17042_FSTAT = 0x3D, MAX17042_SHDNTIMER = 0x3F, - MAX17042_VFRemCap = 0x4A, + MAX17042_dQacc = 0x45, + MAX17042_dPacc = 0x46, + + MAX17042_VFSOC0 = 0x48, MAX17042_QH = 0x4D, MAX17042_QL = 0x4E, + + MAX17042_VFSOC0Enable = 0x60, + MAX17042_MLOCKReg1 = 0x62, + MAX17042_MLOCKReg2 = 0x63, + + MAX17042_MODELChrTbl = 0x80, + + MAX17042_OCV = 0xEE, + + MAX17042_OCVInternal = 0xFB, + + MAX17042_VFSOC = 0xFF, }; /* -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] max17042: Add POR init procedure from Maxim appnote 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie 2012-01-24 17:26 ` [PATCH 1/5] max17042: Align register definitions with data sheet and init appnote dirk.brandewie @ 2012-01-24 17:26 ` dirk.brandewie 2012-01-24 17:26 ` [PATCH 3/5] max17042: Add support for signalling change in SOC dirk.brandewie ` (4 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: dirk.brandewie @ 2012-01-24 17:26 UTC (permalink / raw) To: linux-kernel Cc: cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Dirk Brandewie From: Dirk Brandewie <dirk.brandewie@gmail.com> Add power on reset (POR) init procedure defined by the maxim appnote. Using this procedure ensures that the part is configured/initialized correctly at POR and improves early accuracy of the fuel gauge and informs the fuel gauge with the battery characterization parameters. The battery characterization parameters come from the maxim characterization procedure. Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> --- drivers/power/max17042_battery.c | 389 +++++++++++++++++++++++++++++++- include/linux/power/max17042_battery.h | 56 +++++ 2 files changed, 434 insertions(+), 11 deletions(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index 9f0183c..fce502c 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -26,14 +26,40 @@ #include <linux/module.h> #include <linux/slab.h> #include <linux/i2c.h> +#include <linux/delay.h> #include <linux/mod_devicetable.h> #include <linux/power_supply.h> #include <linux/power/max17042_battery.h> +/* Status register bits */ +#define STATUS_POR_BIT (1 << 1) +#define STATUS_BST_BIT (1 << 3) +#define STATUS_VMN_BIT (1 << 8) +#define STATUS_TMN_BIT (1 << 9) +#define STATUS_SMN_BIT (1 << 10) +#define STATUS_BI_BIT (1 << 11) +#define STATUS_VMX_BIT (1 << 12) +#define STATUS_TMX_BIT (1 << 13) +#define STATUS_SMX_BIT (1 << 14) +#define STATUS_BR_BIT (1 << 15) + +#define VFSOC0_LOCK 0x0000 +#define VFSOC0_UNLOCK 0x0080 +#define MODEL_UNLOCK1 0X0059 +#define MODEL_UNLOCK2 0X00C4 +#define MODEL_LOCK1 0X0000 +#define MODEL_LOCK2 0X0000 + +#define dQ_ACC_DIV 0x4 +#define dP_ACC_100 0x1900 +#define dP_ACC_200 0x3200 + struct max17042_chip { struct i2c_client *client; struct power_supply battery; struct max17042_platform_data *pdata; + struct work_struct work; + int init_complete; }; static int max17042_write_reg(struct i2c_client *client, u8 reg, u16 value) @@ -86,6 +112,9 @@ static int max17042_get_property(struct power_supply *psy, struct max17042_chip *chip = container_of(psy, struct max17042_chip, battery); + if (!chip->init_complete) + return -EAGAIN; + switch (psp) { case POWER_SUPPLY_PROP_PRESENT: val->intval = max17042_read_reg(chip->client, @@ -180,12 +209,343 @@ static int max17042_get_property(struct power_supply *psy, return 0; } +static int max17042_write_verify_reg(struct i2c_client *client, + u8 reg, u16 value) +{ + int retries = 8; + int ret; + u16 read_value; + + do { + ret = i2c_smbus_write_word_data(client, reg, value); + read_value = max17042_read_reg(client, reg); + if (read_value != value) { + ret = -EIO; + retries--; + } + } while (retries && read_value != value); + + if (ret < 0) + dev_err(&client->dev, "%s: err %d\n", __func__, ret); + + return ret; +} + +static inline void max17042_override_por( + struct i2c_client *client, u8 reg, u16 value) +{ + if (value) + max17042_write_reg(client, reg, value); +} + +static inline void max10742_unlock_model(struct max17042_chip *chip) +{ + struct i2c_client *client = chip->client; + max17042_write_reg(client, MAX17042_MLOCKReg1, MODEL_UNLOCK1); + max17042_write_reg(client, MAX17042_MLOCKReg2, MODEL_UNLOCK2); +} + +static inline void max10742_lock_model(struct max17042_chip *chip) +{ + struct i2c_client *client = chip->client; + max17042_write_reg(client, MAX17042_MLOCKReg1, MODEL_LOCK1); + max17042_write_reg(client, MAX17042_MLOCKReg2, MODEL_LOCK2); +} + +static inline void max17042_write_model_data(struct max17042_chip *chip, + u8 addr, int size) +{ + struct i2c_client *client = chip->client; + int i; + for (i = 0; i < size; i++) + max17042_write_reg(client, addr + i, + chip->pdata->config_data->cell_char_tbl[i]); +} + +static inline void max17042_read_model_data(struct max17042_chip *chip, + u8 addr, u16 *data, int size) +{ + struct i2c_client *client = chip->client; + int i; + + for (i = 0; i < size; i++) + data[i] = max17042_read_reg(client, addr + i); +} + +static inline int max17042_model_data_compare(struct max17042_chip *chip, + u16 *data1, u16 *data2, int size) +{ + int i; + + if (memcmp(data1, data2, size)) { + dev_err(&chip->client->dev, "%s compare failed\n", __func__); + for (i = 0; i < size; i++) + dev_info(&chip->client->dev, "0x%x, 0x%x", + data1[i], data2[i]); + dev_info(&chip->client->dev, "\n"); + return -EINVAL; + } + return 0; +} + +static int max17042_init_model(struct max17042_chip *chip) +{ + int ret; + int table_size = + sizeof(chip->pdata->config_data->cell_char_tbl)/sizeof(u16); + u16 *temp_data; + + temp_data = kzalloc(table_size, GFP_KERNEL); + if (!temp_data) + return -ENOMEM; + + max10742_unlock_model(chip); + max17042_write_model_data(chip, MAX17042_MODELChrTbl, + table_size); + max17042_read_model_data(chip, MAX17042_MODELChrTbl, temp_data, + table_size); + + ret = max17042_model_data_compare( + chip, + chip->pdata->config_data->cell_char_tbl, + temp_data, + table_size); + + max10742_lock_model(chip); + kfree(temp_data); + + return ret; +} + +static int max17042_verify_model_lock(struct max17042_chip *chip) +{ + int i; + int table_size = + sizeof(chip->pdata->config_data->cell_char_tbl); + u16 *temp_data; + int ret = 0; + + temp_data = kzalloc(table_size, GFP_KERNEL); + if (!temp_data) + return -ENOMEM; + + max17042_read_model_data(chip, MAX17042_MODELChrTbl, temp_data, + table_size); + for (i = 0; i < table_size; i++) + if (temp_data[i]) + ret = -EINVAL; + + kfree(temp_data); + return ret; +} + +static void max17042_write_config_regs(struct max17042_chip *chip) +{ + struct max17042_config_data *config = chip->pdata->config_data; + + max17042_write_reg(chip->client, MAX17042_CONFIG, config->config); + max17042_write_reg(chip->client, MAX17042_LearnCFG, config->learn_cfg); + max17042_write_reg(chip->client, MAX17042_FilterCFG, + config->filter_cfg); + max17042_write_reg(chip->client, MAX17042_RelaxCFG, config->relax_cfg); +} + +static void max17042_write_custom_regs(struct max17042_chip *chip) +{ + struct max17042_config_data *config = chip->pdata->config_data; + + max17042_write_verify_reg(chip->client, MAX17042_RCOMP0, + config->rcomp0); + max17042_write_verify_reg(chip->client, MAX17042_TempCo, + config->tcompc0); + max17042_write_reg(chip->client, MAX17042_EmptyTempCo, + config->empty_tempco); + max17042_write_verify_reg(chip->client, MAX17042_K_empty0, + config->kempty0); + max17042_write_verify_reg(chip->client, MAX17042_ICHGTerm, + config->ichgt_term); +} + +static void max17042_update_capacity_regs(struct max17042_chip *chip) +{ + struct max17042_config_data *config = chip->pdata->config_data; + + max17042_write_verify_reg(chip->client, MAX17042_FullCAP, + config->fullcap); + max17042_write_reg(chip->client, MAX17042_DesignCap, + config->design_cap); + max17042_write_verify_reg(chip->client, MAX17042_FullCAPNom, + config->fullcapnom); +} + +static void max17042_reset_vfsoc0_reg(struct max17042_chip *chip) +{ + u16 vfSoc; + + vfSoc = max17042_read_reg(chip->client, MAX17042_VFSOC); + max17042_write_reg(chip->client, MAX17042_VFSOC0Enable, VFSOC0_UNLOCK); + max17042_write_verify_reg(chip->client, MAX17042_VFSOC0, vfSoc); + max17042_write_reg(chip->client, MAX17042_VFSOC0Enable, VFSOC0_LOCK); +} + +static void max17042_load_new_capacity_params(struct max17042_chip *chip) +{ + u16 full_cap0, rep_cap, dq_acc, vfSoc; + u32 rem_cap; + + struct max17042_config_data *config = chip->pdata->config_data; + + full_cap0 = max17042_read_reg(chip->client, MAX17042_FullCAP0); + vfSoc = max17042_read_reg(chip->client, MAX17042_VFSOC); + + /* fg_vfSoc needs to shifted by 8 bits to get the + * perc in 1% accuracy, to get the right rem_cap multiply + * full_cap0, fg_vfSoc and devide by 100 + */ + rem_cap = ((vfSoc >> 8) * full_cap0) / 100; + max17042_write_verify_reg(chip->client, MAX17042_RemCap, (u16)rem_cap); + + rep_cap = (u16)rem_cap; + max17042_write_verify_reg(chip->client, MAX17042_RepCap, rep_cap); + + /* Write dQ_acc to 200% of Capacity and dP_acc to 200% */ + dq_acc = config->fullcap / dQ_ACC_DIV; + max17042_write_verify_reg(chip->client, MAX17042_dQacc, dq_acc); + max17042_write_verify_reg(chip->client, MAX17042_dPacc, dP_ACC_200); + + max17042_write_verify_reg(chip->client, MAX17042_FullCAP, + config->fullcap); + max17042_write_reg(chip->client, MAX17042_DesignCap, + config->design_cap); + max17042_write_verify_reg(chip->client, MAX17042_FullCAPNom, + config->fullcapnom); +} + +/* + * Block write all the override values coming from platform data. + * This function MUST be called before the POR initialization proceedure + * specified by maxim. + */ +static inline void max17042_override_por_values(struct max17042_chip *chip) +{ + struct i2c_client *client = chip->client; + struct max17042_config_data *config = chip->pdata->config_data; + + max17042_override_por(client, MAX17042_TGAIN, config->tgain); + max17042_override_por(client, MAx17042_TOFF, config->toff); + max17042_override_por(client, MAX17042_CGAIN, config->cgain); + max17042_override_por(client, MAX17042_COFF, config->coff); + + max17042_override_por(client, MAX17042_VALRT_Th, config->valrt_thresh); + max17042_override_por(client, MAX17042_TALRT_Th, config->talrt_thresh); + max17042_override_por(client, MAX17042_SALRT_Th, + config->soc_alrt_thresh); + max17042_override_por(client, MAX17042_CONFIG, config->config); + max17042_override_por(client, MAX17042_SHDNTIMER, config->shdntimer); + + max17042_override_por(client, MAX17042_DesignCap, config->design_cap); + max17042_override_por(client, MAX17042_ICHGTerm, config->ichgt_term); + + max17042_override_por(client, MAX17042_AtRate, config->at_rate); + max17042_override_por(client, MAX17042_LearnCFG, config->learn_cfg); + max17042_override_por(client, MAX17042_FilterCFG, config->filter_cfg); + max17042_override_por(client, MAX17042_RelaxCFG, config->relax_cfg); + max17042_override_por(client, MAX17042_MiscCFG, config->misc_cfg); + max17042_override_por(client, MAX17042_MaskSOC, config->masksoc); + + max17042_override_por(client, MAX17042_FullCAP, config->fullcap); + max17042_override_por(client, MAX17042_FullCAPNom, config->fullcapnom); + max17042_override_por(client, MAX17042_SOC_empty, config->socempty); + max17042_override_por(client, MAX17042_LAvg_empty, config->lavg_empty); + max17042_override_por(client, MAX17042_dQacc, config->dqacc); + max17042_override_por(client, MAX17042_dPacc, config->dpacc); + + max17042_override_por(client, MAX17042_V_empty, config->vempty); + max17042_override_por(client, MAX17042_TempNom, config->temp_nom); + max17042_override_por(client, MAX17042_TempLim, config->temp_lim); + max17042_override_por(client, MAX17042_FCTC, config->fctc); + max17042_override_por(client, MAX17042_RCOMP0, config->rcomp0); + max17042_override_por(client, MAX17042_TempCo, config->tcompc0); + max17042_override_por(client, MAX17042_EmptyTempCo, + config->empty_tempco); + max17042_override_por(client, MAX17042_K_empty0, config->kempty0); +} + +static int max17042_init_chip(struct max17042_chip *chip) +{ + int ret; + int val; + + max17042_override_por_values(chip); + /* After Power up, the MAX17042 requires 500mS in order + * to perform signal debouncing and initial SOC reporting + */ + msleep(500); + + /* Initialize configaration */ + max17042_write_config_regs(chip); + + /* write cell characterization data */ + ret = max17042_init_model(chip); + if (ret) { + dev_err(&chip->client->dev, "%s init failed\n", + __func__); + return -EIO; + } + max17042_verify_model_lock(chip); + if (ret) { + dev_err(&chip->client->dev, "%s lock verify failed\n", + __func__); + return -EIO; + } + /* write custom parameters */ + max17042_write_custom_regs(chip); + + /* update capacity params */ + max17042_update_capacity_regs(chip); + + /* delay must be atleast 350mS to allow VFSOC + * to be calculated from the new configuration + */ + msleep(350); + + /* reset vfsoc0 reg */ + max17042_reset_vfsoc0_reg(chip); + + /* load new capacity params */ + max17042_load_new_capacity_params(chip); + + /* Init complete, Clear the POR bit */ + val = max17042_read_reg(chip->client, MAX17042_STATUS); + max17042_write_reg(chip->client, MAX17042_STATUS, + val & (~STATUS_POR_BIT)); + return 0; +} + + +static void max17042_init_worker(struct work_struct *work) +{ + struct max17042_chip *chip = container_of(work, + struct max17042_chip, work); + int ret; + + /* Initialize registers according to values from the platform data */ + if (chip->pdata->enable_por_init && chip->pdata->config_data) { + ret = max17042_init_chip(chip); + if (ret) + return; + } + + chip->init_complete = 1; +} + static int __devinit max17042_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); struct max17042_chip *chip; int ret; + int reg; if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) return -EIO; @@ -210,17 +570,9 @@ static int __devinit max17042_probe(struct i2c_client *client, if (!chip->pdata->enable_current_sense) chip->battery.num_properties -= 2; - ret = power_supply_register(&client->dev, &chip->battery); - if (ret) { - dev_err(&client->dev, "failed: power supply register\n"); - kfree(chip); - return ret; - } - - /* Initialize registers according to values from the platform data */ if (chip->pdata->init_data) max17042_set_reg(client, chip->pdata->init_data, - chip->pdata->num_init_data); + chip->pdata->num_init_data); if (!chip->pdata->enable_current_sense) { max17042_write_reg(client, MAX17042_CGAIN, 0x0000); @@ -228,10 +580,25 @@ static int __devinit max17042_probe(struct i2c_client *client, max17042_write_reg(client, MAX17042_LearnCFG, 0x0007); } else { if (chip->pdata->r_sns == 0) - chip->pdata->r_sns = MAX17042_DEFAULT_SNS_RESISTOR; + chip->pdata->r_sns = + MAX17042_DEFAULT_SNS_RESISTOR; } - return 0; + reg = max17042_read_reg(chip->client, MAX17042_STATUS); + + if (reg & STATUS_POR_BIT) { + INIT_WORK(&chip->work, max17042_init_worker); + schedule_work(&chip->work); + } else { + chip->init_complete = 1; + } + + ret = power_supply_register(&client->dev, &chip->battery); + if (ret) { + dev_err(&client->dev, "failed: power supply register\n"); + kfree(chip); + } + return ret; } static int __devexit max17042_remove(struct i2c_client *client) diff --git a/include/linux/power/max17042_battery.h b/include/linux/power/max17042_battery.h index 67eeada..e01b167 100644 --- a/include/linux/power/max17042_battery.h +++ b/include/linux/power/max17042_battery.h @@ -27,6 +27,8 @@ #define MAX17042_BATTERY_FULL (100) #define MAX17042_DEFAULT_SNS_RESISTOR (10000) +#define MAX17042_CHARACTERIZATION_DATA_SIZE 48 + enum max17042_register { MAX17042_STATUS = 0x00, MAX17042_VALRT_Th = 0x01, @@ -124,10 +126,64 @@ struct max17042_reg_data { u16 data; }; +struct max17042_config_data { + /* External current sense resistor value in milli-ohms */ + u32 cur_sense_val; + + /* A/D measurement */ + u16 tgain; /* 0x2C */ + u16 toff; /* 0x2D */ + u16 cgain; /* 0x2E */ + u16 coff; /* 0x2F */ + + /* Alert / Status */ + u16 valrt_thresh; /* 0x01 */ + u16 talrt_thresh; /* 0x02 */ + u16 soc_alrt_thresh; /* 0x03 */ + u16 config; /* 0x01D */ + u16 shdntimer; /* 0x03F */ + + /* App data */ + u16 design_cap; /* 0x18 */ + u16 ichgt_term; /* 0x1E */ + + /* MG3 config */ + u16 at_rate; /* 0x04 */ + u16 learn_cfg; /* 0x28 */ + u16 filter_cfg; /* 0x29 */ + u16 relax_cfg; /* 0x2A */ + u16 misc_cfg; /* 0x2B */ + u16 masksoc; /* 0x32 */ + + /* MG3 save and restore */ + u16 fullcap; /* 0x10 */ + u16 fullcapnom; /* 0x23 */ + u16 socempty; /* 0x33 */ + u16 lavg_empty; /* 0x36 */ + u16 dqacc; /* 0x45 */ + u16 dpacc; /* 0x46 */ + + /* Cell technology from power_supply.h */ + u16 cell_technology; + + /* Cell Data */ + u16 vempty; /* 0x12 */ + u16 temp_nom; /* 0x24 */ + u16 temp_lim; /* 0x25 */ + u16 fctc; /* 0x37 */ + u16 rcomp0; /* 0x38 */ + u16 tcompc0; /* 0x39 */ + u16 empty_tempco; /* 0x3A */ + u16 kempty0; /* 0x3B */ + u16 cell_char_tbl[MAX17042_CHARACTERIZATION_DATA_SIZE]; +} __packed; + struct max17042_platform_data { struct max17042_reg_data *init_data; + struct max17042_config_data *config_data; int num_init_data; /* Number of enties in init_data array */ bool enable_current_sense; + bool enable_por_init; /* Use POR init from Maxim appnote */ /* * R_sns in micro-ohms. -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] max17042: Add support for signalling change in SOC 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie 2012-01-24 17:26 ` [PATCH 1/5] max17042: Align register definitions with data sheet and init appnote dirk.brandewie 2012-01-24 17:26 ` [PATCH 2/5] max17042: Add POR init procedure from Maxim appnote dirk.brandewie @ 2012-01-24 17:26 ` dirk.brandewie 2012-01-24 17:26 ` [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL dirk.brandewie ` (3 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: dirk.brandewie @ 2012-01-24 17:26 UTC (permalink / raw) To: linux-kernel Cc: cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Dirk Brandewie From: Dirk Brandewie <dirk.brandewie@gmail.com> If platform has the alert pin attached to an interrupt source have the driver signal a change in the SOC every 1 percent. Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> --- drivers/power/max17042_battery.c | 53 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 53 insertions(+), 0 deletions(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index fce502c..e0a4430 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -27,6 +27,7 @@ #include <linux/slab.h> #include <linux/i2c.h> #include <linux/delay.h> +#include <linux/interrupt.h> #include <linux/mod_devicetable.h> #include <linux/power_supply.h> #include <linux/power/max17042_battery.h> @@ -43,6 +44,11 @@ #define STATUS_SMX_BIT (1 << 14) #define STATUS_BR_BIT (1 << 15) +/* Interrupt mask bits */ +#define CONFIG_ALRT_BIT_ENBL (1 << 2) +#define STATUS_INTR_SOC_BIT (1 << 14) +#define STATUS_INTR_LOW_SOC_BIT (1 << 10) + #define VFSOC0_LOCK 0x0000 #define VFSOC0_UNLOCK 0x0080 #define MODEL_UNLOCK1 0X0059 @@ -522,6 +528,39 @@ static int max17042_init_chip(struct max17042_chip *chip) return 0; } +static void max17042_set_soc_threshold(struct max17042_chip *chip, u16 off) +{ + u16 soc, soc_tr; + + /* program interrupt thesholds such that we should + * get interrupt for every 'off' perc change in the soc + */ + soc = max17042_read_reg(chip->client, MAX17042_RepSOC) >> 8; + soc_tr = (soc + off) << 8; + soc_tr |= (soc - off); + max17042_write_reg(chip->client, MAX17042_SALRT_Th, soc_tr); +} + +static irqreturn_t max17042_intr_handler(int id, void *dev) +{ + return IRQ_WAKE_THREAD; +} + +static irqreturn_t max17042_thread_handler(int id, void *dev) +{ + struct max17042_chip *chip = dev; + u16 val; + + val = max17042_read_reg(chip->client, MAX17042_STATUS); + if ((val & STATUS_INTR_SOC_BIT) || + (val & STATUS_INTR_LOW_SOC_BIT)) { + dev_info(&chip->client->dev, "SOC threshold INTR\n"); + max17042_set_soc_threshold(chip, 1); + } + + power_supply_changed(&chip->battery); + return IRQ_HANDLED; +} static void max17042_init_worker(struct work_struct *work) { @@ -584,6 +623,20 @@ static int __devinit max17042_probe(struct i2c_client *client, MAX17042_DEFAULT_SNS_RESISTOR; } + if (client->irq) { + ret = request_threaded_irq(client->irq, max17042_intr_handler, + max17042_thread_handler, + 0, chip->battery.name, chip); + if (!ret) { + reg = max17042_read_reg(client, MAX17042_CONFIG); + reg |= CONFIG_ALRT_BIT_ENBL; + max17042_write_reg(client, MAX17042_CONFIG, reg); + max17042_set_soc_threshold(chip, 1); + } else + dev_err(&client->dev, "%s(): cannot get IRQ\n", + __func__); + } + reg = max17042_read_reg(chip->client, MAX17042_STATUS); if (reg & STATUS_POR_BIT) { -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie ` (2 preceding siblings ...) 2012-01-24 17:26 ` [PATCH 3/5] max17042: Add support for signalling change in SOC dirk.brandewie @ 2012-01-24 17:26 ` dirk.brandewie 2012-03-13 18:19 ` Anton Vorontsov 2012-01-24 17:26 ` [PATCH 5/5] max17042: Change capacity property to use reported SOC register dirk.brandewie ` (2 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: dirk.brandewie @ 2012-01-24 17:26 UTC (permalink / raw) To: linux-kernel Cc: cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Dirk Brandewie From: Bruce Robertson <bruce.e.robertson@intel.com> The bottom three bits of the register are don't care bits. The LSB value is 625 uV. Adjust the returned values appropriately Signed-off-by: Bruce Robertson <bruce.e.robertson@intel.com> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> --- drivers/power/max17042_battery.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index e0a4430..6e96b58 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -148,11 +148,15 @@ static int max17042_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_VOLTAGE_NOW: val->intval = max17042_read_reg(chip->client, - MAX17042_VCELL) * 83; /* 1000 / 12 = 83 */ + MAX17042_VCELL); + val->intval >>= 3; + val->intval *= 625; /* Units of LSB = 625 uV */ break; case POWER_SUPPLY_PROP_VOLTAGE_AVG: val->intval = max17042_read_reg(chip->client, - MAX17042_AvgVCELL) * 83; + MAX17042_AvgVCELL); + val->intval >>= 3; + val->intval *= 625; /* Units of LSB = 625 uV */ break; case POWER_SUPPLY_PROP_CAPACITY: val->intval = max17042_read_reg(chip->client, -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-01-24 17:26 ` [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL dirk.brandewie @ 2012-03-13 18:19 ` Anton Vorontsov 2012-03-13 21:00 ` bruce robertson 0 siblings, 1 reply; 17+ messages in thread From: Anton Vorontsov @ 2012-03-13 18:19 UTC (permalink / raw) To: dirk.brandewie Cc: linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: > From: Bruce Robertson <bruce.e.robertson@intel.com> > > The bottom three bits of the register are don't care bits. The LSB > value is 625 uV. Adjust the returned values appropriately > > Signed-off-by: Bruce Robertson <bruce.e.robertson@intel.com> > Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> > Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> I guess this was fixed long ago by the following patch: commit cf7a8c03db792894f436db5f3ffc44d947b9b068 Author: MyungJoo Ham <myungjoo.ham@samsung.com> Date: Wed Aug 17 10:18:34 2011 +0900 max17042_battery: Bugfix of incorrect voltage register value interpretation The calculation had error in getting voltage values from MAX17042 registers. The least bit denotes 78.125uV (625/8). Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Signed-off-by: Philip Rakity <prakity@marvell.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> Thanks, -- Anton Vorontsov Email: cbouatmailru@gmail.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-03-13 18:19 ` Anton Vorontsov @ 2012-03-13 21:00 ` bruce robertson 2012-03-13 21:04 ` Jason Wortham 0 siblings, 1 reply; 17+ messages in thread From: bruce robertson @ 2012-03-13 21:00 UTC (permalink / raw) To: Anton Vorontsov Cc: dirk.brandewie, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham Anton Vorontsov <cbouatmailru@gmail.com> writes: > On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: >> From: Bruce Robertson <bruce.e.robertson@intel.com> >> >> The bottom three bits of the register are don't care bits. The LSB >> value is 625 uV. Adjust the returned values appropriately >> >> Signed-off-by: Bruce Robertson <bruce.e.robertson@intel.com> >> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> > > I guess this was fixed long ago by the following patch: > > commit cf7a8c03db792894f436db5f3ffc44d947b9b068 > Author: MyungJoo Ham <myungjoo.ham@samsung.com> > Date: Wed Aug 17 10:18:34 2011 +0900 > > max17042_battery: Bugfix of incorrect voltage register value interpretation > > The calculation had error in getting voltage values from > MAX17042 registers. The least bit denotes 78.125uV (625/8). The multipliers I see in the patch are 83 making the voltages somewhat high and the low 3 bits are not masked off. I'm probably misreading the code. > > Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> > Signed-off-by: Philip Rakity <prakity@marvell.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > Thanks, ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-03-13 21:00 ` bruce robertson @ 2012-03-13 21:04 ` Jason Wortham 2012-03-13 22:59 ` Dirk Brandewie 2012-03-13 23:18 ` bruce robertson 0 siblings, 2 replies; 17+ messages in thread From: Jason Wortham @ 2012-03-13 21:04 UTC (permalink / raw) To: bruce robertson, Anton Vorontsov Cc: dirk.brandewie, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham The bottom 3 bits are still functional bits and aren't required to be masked. Arguably these bits are below the hardware accuracy of the ADC, however, they still provide some averaging information. --Jason -----Original Message----- From: bruce robertson [mailto:bruce.e.robertson@intel.com] Sent: Tuesday, March 13, 2012 2:00 PM To: Anton Vorontsov Cc: dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com; Jason Wortham Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL Anton Vorontsov <cbouatmailru@gmail.com> writes: > On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: >> From: Bruce Robertson <bruce.e.robertson@intel.com> >> >> The bottom three bits of the register are don't care bits. The LSB >> value is 625 uV. Adjust the returned values appropriately >> >> Signed-off-by: Bruce Robertson <bruce.e.robertson@intel.com> >> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> > > I guess this was fixed long ago by the following patch: > > commit cf7a8c03db792894f436db5f3ffc44d947b9b068 > Author: MyungJoo Ham <myungjoo.ham@samsung.com> > Date: Wed Aug 17 10:18:34 2011 +0900 > > max17042_battery: Bugfix of incorrect voltage register value interpretation > > The calculation had error in getting voltage values from > MAX17042 registers. The least bit denotes 78.125uV (625/8). The multipliers I see in the patch are 83 making the voltages somewhat high and the low 3 bits are not masked off. I'm probably misreading the code. > > Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> > Signed-off-by: Philip Rakity <prakity@marvell.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > Thanks, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-03-13 21:04 ` Jason Wortham @ 2012-03-13 22:59 ` Dirk Brandewie 2012-03-14 0:04 ` Jason Wortham 2012-03-13 23:18 ` bruce robertson 1 sibling, 1 reply; 17+ messages in thread From: Dirk Brandewie @ 2012-03-13 22:59 UTC (permalink / raw) To: Jason Wortham Cc: bruce robertson, Anton Vorontsov, dirk.brandewie, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham On 03/13/2012 02:04 PM, Jason Wortham wrote: > The bottom 3 bits are still functional bits and aren't required to be masked. Arguably these bits are below the hardware accuracy of the ADC, however, they still provide some averaging information. > The datasheet for the 17042 shows the bottom three bits as don't care that was the reason for the mask. If Jason says it is still accurate without the mask I am fine with it. --Dirk > --Jason > > -----Original Message----- > From: bruce robertson [mailto:bruce.e.robertson@intel.com] > Sent: Tuesday, March 13, 2012 2:00 PM > To: Anton Vorontsov > Cc: dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com; Jason Wortham > Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL > > Anton Vorontsov<cbouatmailru@gmail.com> writes: > >> On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: >>> From: Bruce Robertson<bruce.e.robertson@intel.com> >>> >>> The bottom three bits of the register are don't care bits. The LSB >>> value is 625 uV. Adjust the returned values appropriately >>> >>> Signed-off-by: Bruce Robertson<bruce.e.robertson@intel.com> >>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com> >>> Acked-by: MyungJoo Ham<myungjoo.ham@samsung.com> >> >> I guess this was fixed long ago by the following patch: >> >> commit cf7a8c03db792894f436db5f3ffc44d947b9b068 >> Author: MyungJoo Ham<myungjoo.ham@samsung.com> >> Date: Wed Aug 17 10:18:34 2011 +0900 >> >> max17042_battery: Bugfix of incorrect voltage register value interpretation >> >> The calculation had error in getting voltage values from >> MAX17042 registers. The least bit denotes 78.125uV (625/8). > > The multipliers I see in the patch are 83 making the voltages somewhat > high and the low 3 bits are not masked off. I'm probably misreading the code. > >> >> Signed-off-by: MyungJoo Ham<myungjoo.ham@samsung.com> >> Signed-off-by: Philip Rakity<prakity@marvell.com> >> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> >> Signed-off-by: Anton Vorontsov<cbouatmailru@gmail.com> >> >> Thanks, ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-03-13 22:59 ` Dirk Brandewie @ 2012-03-14 0:04 ` Jason Wortham 0 siblings, 0 replies; 17+ messages in thread From: Jason Wortham @ 2012-03-14 0:04 UTC (permalink / raw) To: Dirk Brandewie Cc: bruce robertson, Anton Vorontsov, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham Ok, the datasheet is written that way as a commitment about the hardware ADC accuracy/resolution. However, the ADC performs a set of 8x samples, and as a result will have an average which provides some additional "fake" resolution. Even though this extra resolution isn't strong in an analog way, it's still useful information. But I guess I'm really splitting hairs here since it's such a small resolution. --Jason -----Original Message----- From: Dirk Brandewie [mailto:dirk.brandewie@gmail.com] Sent: Tuesday, March 13, 2012 3:59 PM To: Jason Wortham Cc: bruce robertson; Anton Vorontsov; dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL On 03/13/2012 02:04 PM, Jason Wortham wrote: > The bottom 3 bits are still functional bits and aren't required to be masked. Arguably these bits are below the hardware accuracy of the ADC, however, they still provide some averaging information. > The datasheet for the 17042 shows the bottom three bits as don't care that was the reason for the mask. If Jason says it is still accurate without the mask I am fine with it. --Dirk > --Jason > > -----Original Message----- > From: bruce robertson [mailto:bruce.e.robertson@intel.com] > Sent: Tuesday, March 13, 2012 2:00 PM > To: Anton Vorontsov > Cc: dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com; Jason Wortham > Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL > > Anton Vorontsov<cbouatmailru@gmail.com> writes: > >> On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: >>> From: Bruce Robertson<bruce.e.robertson@intel.com> >>> >>> The bottom three bits of the register are don't care bits. The LSB >>> value is 625 uV. Adjust the returned values appropriately >>> >>> Signed-off-by: Bruce Robertson<bruce.e.robertson@intel.com> >>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com> >>> Acked-by: MyungJoo Ham<myungjoo.ham@samsung.com> >> >> I guess this was fixed long ago by the following patch: >> >> commit cf7a8c03db792894f436db5f3ffc44d947b9b068 >> Author: MyungJoo Ham<myungjoo.ham@samsung.com> >> Date: Wed Aug 17 10:18:34 2011 +0900 >> >> max17042_battery: Bugfix of incorrect voltage register value interpretation >> >> The calculation had error in getting voltage values from >> MAX17042 registers. The least bit denotes 78.125uV (625/8). > > The multipliers I see in the patch are 83 making the voltages somewhat > high and the low 3 bits are not masked off. I'm probably misreading the code. > >> >> Signed-off-by: MyungJoo Ham<myungjoo.ham@samsung.com> >> Signed-off-by: Philip Rakity<prakity@marvell.com> >> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> >> Signed-off-by: Anton Vorontsov<cbouatmailru@gmail.com> >> >> Thanks, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-03-13 21:04 ` Jason Wortham 2012-03-13 22:59 ` Dirk Brandewie @ 2012-03-13 23:18 ` bruce robertson 2012-03-14 0:37 ` Jason Wortham 1 sibling, 1 reply; 17+ messages in thread From: bruce robertson @ 2012-03-13 23:18 UTC (permalink / raw) To: Jason Wortham Cc: Anton Vorontsov, dirk.brandewie, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham Jason Wortham <Jason.Wortham@maxim-ic.com> writes: > The bottom 3 bits are still functional bits and aren't required to be masked. Arguably these bits are below the hardware accuracy of the ADC, however, they still provide some averaging information. About the multiplier. why are the multipliers 83? 83*8==664 uV. > > --Jason > > -----Original Message----- > From: bruce robertson [mailto:bruce.e.robertson@intel.com] > Sent: Tuesday, March 13, 2012 2:00 PM > To: Anton Vorontsov > Cc: dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com; Jason Wortham > Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL > > Anton Vorontsov <cbouatmailru@gmail.com> writes: > >> On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: >>> From: Bruce Robertson <bruce.e.robertson@intel.com> >>> >>> The bottom three bits of the register are don't care bits. The LSB >>> value is 625 uV. Adjust the returned values appropriately >>> >>> Signed-off-by: Bruce Robertson <bruce.e.robertson@intel.com> >>> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> >>> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> >> >> I guess this was fixed long ago by the following patch: >> >> commit cf7a8c03db792894f436db5f3ffc44d947b9b068 >> Author: MyungJoo Ham <myungjoo.ham@samsung.com> >> Date: Wed Aug 17 10:18:34 2011 +0900 >> >> max17042_battery: Bugfix of incorrect voltage register value interpretation >> >> The calculation had error in getting voltage values from >> MAX17042 registers. The least bit denotes 78.125uV (625/8). > > The multipliers I see in the patch are 83 making the voltages somewhat > high and the low 3 bits are not masked off. I'm probably misreading the code. > >> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> >> Signed-off-by: Philip Rakity <prakity@marvell.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> >> >> Thanks, ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL 2012-03-13 23:18 ` bruce robertson @ 2012-03-14 0:37 ` Jason Wortham 0 siblings, 0 replies; 17+ messages in thread From: Jason Wortham @ 2012-03-14 0:37 UTC (permalink / raw) To: bruce robertson Cc: Anton Vorontsov, dirk.brandewie, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham I'm not sure where 83 comes from. The LSB is 78.125uV, or bit D3 is 625uV. I think 83uV and 664uV are incorrect. --Jason -----Original Message----- From: bruce robertson [mailto:bruce.e.robertson@intel.com] Sent: Tuesday, March 13, 2012 4:18 PM To: Jason Wortham Cc: Anton Vorontsov; dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL Jason Wortham <Jason.Wortham@maxim-ic.com> writes: > The bottom 3 bits are still functional bits and aren't required to be masked. Arguably these bits are below the hardware accuracy of the ADC, however, they still provide some averaging information. About the multiplier. why are the multipliers 83? 83*8==664 uV. > > --Jason > > -----Original Message----- > From: bruce robertson [mailto:bruce.e.robertson@intel.com] > Sent: Tuesday, March 13, 2012 2:00 PM > To: Anton Vorontsov > Cc: dirk.brandewie@gmail.com; linux-kernel@vger.kernel.org; dg77.kim@samsung.com; kyungmin.park@samsung.com; myungjoo.ham@samsung.com; Jason Wortham > Subject: Re: [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL > > Anton Vorontsov <cbouatmailru@gmail.com> writes: > >> On Tue, Jan 24, 2012 at 09:26:07AM -0800, dirk.brandewie@gmail.com wrote: >>> From: Bruce Robertson <bruce.e.robertson@intel.com> >>> >>> The bottom three bits of the register are don't care bits. The LSB >>> value is 625 uV. Adjust the returned values appropriately >>> >>> Signed-off-by: Bruce Robertson <bruce.e.robertson@intel.com> >>> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> >>> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com> >> >> I guess this was fixed long ago by the following patch: >> >> commit cf7a8c03db792894f436db5f3ffc44d947b9b068 >> Author: MyungJoo Ham <myungjoo.ham@samsung.com> >> Date: Wed Aug 17 10:18:34 2011 +0900 >> >> max17042_battery: Bugfix of incorrect voltage register value interpretation >> >> The calculation had error in getting voltage values from >> MAX17042 registers. The least bit denotes 78.125uV (625/8). > > The multipliers I see in the patch are 83 making the voltages somewhat > high and the low 3 bits are not masked off. I'm probably misreading the code. > >> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> >> Signed-off-by: Philip Rakity <prakity@marvell.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> >> >> Thanks, ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] max17042: Change capacity property to use reported SOC register 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie ` (3 preceding siblings ...) 2012-01-24 17:26 ` [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL dirk.brandewie @ 2012-01-24 17:26 ` dirk.brandewie 2012-03-13 18:34 ` Anton Vorontsov 2012-02-28 16:17 ` [PATCH 0/5] MAX17042 add support for maxim POR procedure Dirk Brandewie 2012-03-13 18:48 ` Anton Vorontsov 6 siblings, 1 reply; 17+ messages in thread From: dirk.brandewie @ 2012-01-24 17:26 UTC (permalink / raw) To: linux-kernel Cc: cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Dirk Brandewie From: Dirk Brandewie <dirk.brandewie@gmail.com> The SOC register (0dh) reports the state of charge before empty compensation adjustments are applied. The max value reported by this register will decrease as the battery ages. Use the RepSOC register (06h) to report the capacity of the battery. RepSOC contains a filtered version of the battery capacity after empty compensation adjustments have been applied. Reported-by: Gary Keyes <gary.e.keyes@intel.com> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> --- drivers/power/max17042_battery.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c index 6e96b58..2194278 100644 --- a/drivers/power/max17042_battery.c +++ b/drivers/power/max17042_battery.c @@ -160,7 +160,7 @@ static int max17042_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_CAPACITY: val->intval = max17042_read_reg(chip->client, - MAX17042_SOC) / 256; + MAX17042_RepSOC) / 256; break; case POWER_SUPPLY_PROP_CHARGE_FULL: val->intval = max17042_read_reg(chip->client, -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] max17042: Change capacity property to use reported SOC register 2012-01-24 17:26 ` [PATCH 5/5] max17042: Change capacity property to use reported SOC register dirk.brandewie @ 2012-03-13 18:34 ` Anton Vorontsov 2012-03-13 22:54 ` Dirk Brandewie 0 siblings, 1 reply; 17+ messages in thread From: Anton Vorontsov @ 2012-03-13 18:34 UTC (permalink / raw) To: dirk.brandewie Cc: linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Karol Lewandowski On Tue, Jan 24, 2012 at 09:26:08AM -0800, dirk.brandewie@gmail.com wrote: > From: Dirk Brandewie <dirk.brandewie@gmail.com> > > The SOC register (0dh) reports the state of charge before empty > compensation adjustments are applied. The max value reported by this > register will decrease as the battery ages. > > Use the RepSOC register (06h) to report the capacity of the > battery. RepSOC contains a filtered version of the battery capacity > after empty compensation adjustments have been applied. > > Reported-by: Gary Keyes <gary.e.keyes@intel.com> > > Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com> > --- > drivers/power/max17042_battery.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c > index 6e96b58..2194278 100644 > --- a/drivers/power/max17042_battery.c > +++ b/drivers/power/max17042_battery.c > @@ -160,7 +160,7 @@ static int max17042_get_property(struct power_supply *psy, > break; > case POWER_SUPPLY_PROP_CAPACITY: > val->intval = max17042_read_reg(chip->client, > - MAX17042_SOC) / 256; > + MAX17042_RepSOC) / 256; Applied. But I don't get it: PROP_CAPACITY should report percents! And it looks that it reports something very different. Also, if you look down the file, you'll see this: case POWER_SUPPLY_PROP_CHARGE_FULL: ret = max17042_read_reg(chip->client, MAX17042_RepSOC); if (ret < 0) return ret; if ((ret >> 8) >= MAX17042_BATTERY_FULL) val->intval = 1; else if (ret >= 0) val->intval = 0; break; Wut? This is also wrong. PROP_CHARGE_FULL reports uAh, not "fully charged" boolean status. Please, read Documentation/power/power_supply_class.txt: ~ ~ ~ ~ ~ ~ ~ Charge/Energy/Capacity - how to not confuse ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Because both "charge" (µAh) and "energy" (µWh) represents "capacity" ~ ~ of battery, this class distinguish these terms. Don't mix them! ~ ~ ~ ~ CHARGE_* attributes represents capacity in µAh only. ~ ~ ENERGY_* attributes represents capacity in µWh only. ~ ~ CAPACITY attribute represents capacity in *percents*, from 0 to 100. ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ The driver is seriously broken, very. I'd even consider adding 'depends on BROKEN', as the driver might be the source of the confusion for drivers based on this one and for userland devs. Thanks, -- Anton Vorontsov Email: cbouatmailru@gmail.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] max17042: Change capacity property to use reported SOC register 2012-03-13 18:34 ` Anton Vorontsov @ 2012-03-13 22:54 ` Dirk Brandewie 0 siblings, 0 replies; 17+ messages in thread From: Dirk Brandewie @ 2012-03-13 22:54 UTC (permalink / raw) To: Anton Vorontsov Cc: dirk.brandewie, linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson, Karol Lewandowski On 03/13/2012 11:34 AM, Anton Vorontsov wrote: > On Tue, Jan 24, 2012 at 09:26:08AM -0800, dirk.brandewie@gmail.com wrote: >> From: Dirk Brandewie<dirk.brandewie@gmail.com> >> >> The SOC register (0dh) reports the state of charge before empty >> compensation adjustments are applied. The max value reported by this >> register will decrease as the battery ages. >> >> Use the RepSOC register (06h) to report the capacity of the >> battery. RepSOC contains a filtered version of the battery capacity >> after empty compensation adjustments have been applied. >> >> Reported-by: Gary Keyes<gary.e.keyes@intel.com> >> >> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com> >> --- >> drivers/power/max17042_battery.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c >> index 6e96b58..2194278 100644 >> --- a/drivers/power/max17042_battery.c >> +++ b/drivers/power/max17042_battery.c >> @@ -160,7 +160,7 @@ static int max17042_get_property(struct power_supply *psy, >> break; >> case POWER_SUPPLY_PROP_CAPACITY: >> val->intval = max17042_read_reg(chip->client, >> - MAX17042_SOC) / 256; >> + MAX17042_RepSOC) / 256; > > Applied. But I don't get it: PROP_CAPACITY should report > percents! And it looks that it reports something very > different. > The resolution of the register is 1/256 of a percent so we are actually returning the value in percent. > Also, if you look down the file, you'll see this: > > case POWER_SUPPLY_PROP_CHARGE_FULL: > ret = max17042_read_reg(chip->client, MAX17042_RepSOC); > if (ret< 0) > return ret; > > if ((ret>> 8)>= MAX17042_BATTERY_FULL) > val->intval = 1; > else if (ret>= 0) > val->intval = 0; > break; > I hadn't looked at this property. Looks like that code came from commit 60a1f6e4 > Wut? This is also wrong. PROP_CHARGE_FULL reports uAh, not "fully > charged" boolean status. > > Please, read Documentation/power/power_supply_class.txt: > > ~ ~ ~ ~ ~ ~ ~ Charge/Energy/Capacity - how to not confuse ~ ~ ~ ~ ~ ~ ~ > ~ ~ > ~ Because both "charge" (µAh) and "energy" (µWh) represents "capacity" ~ > ~ of battery, this class distinguish these terms. Don't mix them! ~ > ~ ~ > ~ CHARGE_* attributes represents capacity in µAh only. ~ > ~ ENERGY_* attributes represents capacity in µWh only. ~ > ~ CAPACITY attribute represents capacity in *percents*, from 0 to 100. ~ > ~ ~ > ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ > > The driver is seriously broken, very. I'd even consider adding > 'depends on BROKEN', as the driver might be the source of the > confusion for drivers based on this one and for userland devs. > > Thanks, > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] MAX17042 add support for maxim POR procedure 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie ` (4 preceding siblings ...) 2012-01-24 17:26 ` [PATCH 5/5] max17042: Change capacity property to use reported SOC register dirk.brandewie @ 2012-02-28 16:17 ` Dirk Brandewie 2012-03-13 18:48 ` Anton Vorontsov 6 siblings, 0 replies; 17+ messages in thread From: Dirk Brandewie @ 2012-02-28 16:17 UTC (permalink / raw) To: dirk.brandewie Cc: linux-kernel, cbouatmailru, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson Ping. Any further comments on this patch set? Can it make the 3.4 merge window? TIA --Dirk On 01/24/2012 09:26 AM, dirk.brandewie@gmail.com wrote: > From: Dirk Brandewie<dirk.brandewie@gmail.com> > > This patch set adds support for the power on reset procedure for the > max17042 battery fuel gauge and alert interrupts. > > The accuracy of the fuel gauge is improved by configuring the fuel > gauge with the characterization data for the battery present in the > platform. > > > Changes since v3: > Patch added patch to fix scaling of VCELL register values reported > > Capacity property now used the reported SOC register to account for > empty compensation in reported capacity. > > Fixed sparse/smatch warnings > > Changes since v2: > > Dropped patch moving non-POR init to worker function. > > Reorganized POR patch to only schedule worker thread if the fuel gauge > is in the POR state > > Changes since v1: > > Moved power_supply_register() back to the probe max17042_get_property() > will return -EAGAIN until the init function completes > > Added support for interrupts from the alert pin if the platform has it > connected to an interrupt source. > > Added fix for the values returned by POWER_SUPPLY_PROP_VOLTAGE_NOW > and POWER_SUPPLY_PROP_VOLTAGE_AVG. > > Added Ack's from previous patch series > > > Bruce Robertson (1): > max17042: Fix value scaling for VCELL and avgVCELL > > Dirk Brandewie (4): > max17042: Align register definitions with data sheet and init appnote > max17042: Add POR init procedure from Maxim appnote > max17042: Add support for signalling change in SOC > max17042: Change capacity property to use reported SOC register > > drivers/power/max17042_battery.c | 452 +++++++++++++++++++++++++++++++- > include/linux/power/max17042_battery.h | 93 ++++++- > 2 files changed, 522 insertions(+), 23 deletions(-) > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] MAX17042 add support for maxim POR procedure 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie ` (5 preceding siblings ...) 2012-02-28 16:17 ` [PATCH 0/5] MAX17042 add support for maxim POR procedure Dirk Brandewie @ 2012-03-13 18:48 ` Anton Vorontsov 6 siblings, 0 replies; 17+ messages in thread From: Anton Vorontsov @ 2012-03-13 18:48 UTC (permalink / raw) To: dirk.brandewie Cc: linux-kernel, dg77.kim, kyungmin.park, myungjoo.ham, Jason.Wortham, bruce.e.robertson On Tue, Jan 24, 2012 at 09:26:03AM -0800, dirk.brandewie@gmail.com wrote: [..] > Bruce Robertson (1): > max17042: Fix value scaling for VCELL and avgVCELL > > Dirk Brandewie (4): > max17042: Align register definitions with data sheet and init appnote > max17042: Add POR init procedure from Maxim appnote > max17042: Add support for signalling change in SOC > max17042: Change capacity property to use reported SOC register > > drivers/power/max17042_battery.c | 452 +++++++++++++++++++++++++++++++- > include/linux/power/max17042_battery.h | 93 ++++++- > 2 files changed, 522 insertions(+), 23 deletions(-) Applied. But the driver is broken anyway, see my other email. Thanks, -- Anton Vorontsov Email: cbouatmailru@gmail.com ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-03-14 0:37 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-01-24 17:26 [PATCH 0/5] MAX17042 add support for maxim POR procedure dirk.brandewie 2012-01-24 17:26 ` [PATCH 1/5] max17042: Align register definitions with data sheet and init appnote dirk.brandewie 2012-01-24 17:26 ` [PATCH 2/5] max17042: Add POR init procedure from Maxim appnote dirk.brandewie 2012-01-24 17:26 ` [PATCH 3/5] max17042: Add support for signalling change in SOC dirk.brandewie 2012-01-24 17:26 ` [PATCH 4/5] max17042: Fix value scaling for VCELL and avgVCELL dirk.brandewie 2012-03-13 18:19 ` Anton Vorontsov 2012-03-13 21:00 ` bruce robertson 2012-03-13 21:04 ` Jason Wortham 2012-03-13 22:59 ` Dirk Brandewie 2012-03-14 0:04 ` Jason Wortham 2012-03-13 23:18 ` bruce robertson 2012-03-14 0:37 ` Jason Wortham 2012-01-24 17:26 ` [PATCH 5/5] max17042: Change capacity property to use reported SOC register dirk.brandewie 2012-03-13 18:34 ` Anton Vorontsov 2012-03-13 22:54 ` Dirk Brandewie 2012-02-28 16:17 ` [PATCH 0/5] MAX17042 add support for maxim POR procedure Dirk Brandewie 2012-03-13 18:48 ` Anton Vorontsov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).