| Message ID | f2866a9052bbeaa4b3795907f91974e1d18ef68e.1607085199.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| State | New |
| Headers | show |
| Series | power: supply: Add some fuel-gauge logic | expand |
On Fri, Dec 4, 2020 at 1:41 PM Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > The power-supply core supports concept of OCV (Open Circuit Voltage) => > SOC (State Of Charge) conversion tables. Usually these tables are used > to estimate SOC based on OCV. Some systems use so called "Zero Adjust" > where at the near end-of-battery condition the SOC from coulomb counter > is used to retrieve the OCV - and OCV and VSYS difference is used to > re-estimate the battery capacity. > > Add helper to do look-up the other-way around and also get the OCV > based on SOC > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> Overall a good idea! > +/** > + * power_supply_cap2ocv_simple() - find the battery OCV by capacity > + * @table: Pointer to battery OCV/CAP lookup table > + * @table_len: OCV/CAP table length > + * @cap: Current cap value > + * > + * This helper function is used to look up battery OCV according to > + * current capacity value from one OCV table, and the OCV table must be ordered > + * descending. > + * > + * Return: the battery OCV. > + */ Spell out the abbreviations in the kerneldoc and not just the commit. These will be read much more than the commit message so I would move all the excellent info in the commit message into the kerneldoc and diet the commit message instead. > +int power_supply_cap2ocv_simple(struct power_supply_battery_ocv_table *table, > + int table_len, int cap) > +{ > + int i, ocv, tmp; > + > + for (i = 0; i < table_len; i++) > + if (cap > table[i].capacity) > + break; > + > + if (i > 0 && i < table_len) { > + tmp = (table[i - 1].ocv - table[i].ocv) * > + (cap - table[i].capacity); > + > + tmp /= table[i - 1].capacity - table[i].capacity; > + ocv = tmp + table[i].ocv; This is some linear interpolation right? It's not immediately evident so insert some comment about what is going on. > /** > * power_supply_ocv2cap_simple() - find the battery capacity > * @table: Pointer to battery OCV lookup table > @@ -847,6 +884,20 @@ power_supply_find_ocv2cap_table(struct power_supply_battery_info *info, I suspect this kerneldoc could be improved in the process as well. Yours, Linus Walleij
Hi deee Ho Linus, Thanks (again) for taking a look at this! Highly appreciated :] On Tue, 2020-12-08 at 09:54 +0100, Linus Walleij wrote: > On Fri, Dec 4, 2020 at 1:41 PM Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> wrote: > > > The power-supply core supports concept of OCV (Open Circuit > > Voltage) => > > SOC (State Of Charge) conversion tables. Usually these tables are > > used > > to estimate SOC based on OCV. Some systems use so called "Zero > > Adjust" > > where at the near end-of-battery condition the SOC from coulomb > > counter > > is used to retrieve the OCV - and OCV and VSYS difference is used > > to > > re-estimate the battery capacity. > > > > Add helper to do look-up the other-way around and also get the OCV > > based on SOC > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > Overall a good idea! > > > +/** > > + * power_supply_cap2ocv_simple() - find the battery OCV by > > capacity > > + * @table: Pointer to battery OCV/CAP lookup table > > + * @table_len: OCV/CAP table length > > + * @cap: Current cap value > > + * > > + * This helper function is used to look up battery OCV according > > to > > + * current capacity value from one OCV table, and the OCV table > > must be ordered > > + * descending. > > + * > > + * Return: the battery OCV. > > + */ > > Spell out the abbreviations in the kerneldoc and not just the commit. > These will be read much more than the commit message so I would > move all the excellent info in the commit message into the kerneldoc > and > diet the commit message instead. Hm. I think you have a point here. > > > +int power_supply_cap2ocv_simple(struct > > power_supply_battery_ocv_table *table, > > + int table_len, int cap) > > +{ > > + int i, ocv, tmp; > > + > > + for (i = 0; i < table_len; i++) > > + if (cap > table[i].capacity) > > + break; > > + > > + if (i > 0 && i < table_len) { > > + tmp = (table[i - 1].ocv - table[i].ocv) * > > + (cap - table[i].capacity); > > + > > + tmp /= table[i - 1].capacity - table[i].capacity; > > + ocv = tmp + table[i].ocv; > > This is some linear interpolation right? It's not immediately evident > so insert > some comment about what is going on. Yes. Code interpolates the OCV using two closest known values from table. This is pretty much identical loop to the existing ocv2cap calculation - it would have been better to include it in the diff. OTOH - I did not expect seeing any proper careful reviewing - this RFC was sent to collect opinion on whether this would be worth further work. Anyways - If this function is added it should be changed to take more accurate SOC - perhaps 0.1%(?) - I'm afraid rounding the current capacity to nearest 1% will kill the accuracy and render this somewhat useless. This makes me wonder if the SOC/OCV table in DT should also support providing values using 0.1% as unit? (I don't think this is a must but it might be useful). > > > /** > > * power_supply_ocv2cap_simple() - find the battery capacity > > * @table: Pointer to battery OCV lookup table > > @@ -847,6 +884,20 @@ power_supply_find_ocv2cap_table(struct > > power_supply_battery_info *info, > > I suspect this kerneldoc could be improved in the process as well. > I agree. And also for few others. But that could be a separate patch no matter if this RFC proceeds or not :) > Yours, > Linus Walleij
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index 38e3aa642131..67258799ae2e 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -787,6 +787,43 @@ int power_supply_temp2resist_simple(struct power_supply_resistance_temp_table *t } EXPORT_SYMBOL_GPL(power_supply_temp2resist_simple); +/** + * power_supply_cap2ocv_simple() - find the battery OCV by capacity + * @table: Pointer to battery OCV/CAP lookup table + * @table_len: OCV/CAP table length + * @cap: Current cap value + * + * This helper function is used to look up battery OCV according to + * current capacity value from one OCV table, and the OCV table must be ordered + * descending. + * + * Return: the battery OCV. + */ +int power_supply_cap2ocv_simple(struct power_supply_battery_ocv_table *table, + int table_len, int cap) +{ + int i, ocv, tmp; + + for (i = 0; i < table_len; i++) + if (cap > table[i].capacity) + break; + + if (i > 0 && i < table_len) { + tmp = (table[i - 1].ocv - table[i].ocv) * + (cap - table[i].capacity); + + tmp /= table[i - 1].capacity - table[i].capacity; + ocv = tmp + table[i].ocv; + } else if (i == 0) { + ocv = table[0].ocv; + } else { + ocv = table[table_len - 1].ocv; + } + + return ocv; +} +EXPORT_SYMBOL_GPL(power_supply_cap2ocv_simple); + /** * power_supply_ocv2cap_simple() - find the battery capacity * @table: Pointer to battery OCV lookup table @@ -847,6 +884,20 @@ power_supply_find_ocv2cap_table(struct power_supply_battery_info *info, } EXPORT_SYMBOL_GPL(power_supply_find_ocv2cap_table); +int power_supply_batinfo_cap2ocv(struct power_supply_battery_info *info, + int cap, int temp) +{ + struct power_supply_battery_ocv_table *table; + int table_len; + + table = power_supply_find_ocv2cap_table(info, temp, &table_len); + if (!table) + return -EINVAL; + + return power_supply_cap2ocv_simple(table, table_len, cap); +} +EXPORT_SYMBOL_GPL(power_supply_batinfo_cap2ocv); + int power_supply_batinfo_ocv2cap(struct power_supply_battery_info *info, int ocv, int temp) { diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 81a55e974feb..bae98b628f92 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -403,11 +403,16 @@ extern void power_supply_put_battery_info(struct power_supply *psy, struct power_supply_battery_info *info); extern int power_supply_ocv2cap_simple(struct power_supply_battery_ocv_table *table, int table_len, int ocv); +int power_supply_cap2ocv_simple(struct power_supply_battery_ocv_table *table, + int table_len, int cap); + extern struct power_supply_battery_ocv_table * power_supply_find_ocv2cap_table(struct power_supply_battery_info *info, int temp, int *table_len); extern int power_supply_batinfo_ocv2cap(struct power_supply_battery_info *info, int ocv, int temp); +int power_supply_batinfo_cap2ocv(struct power_supply_battery_info *info, + int cap, int temp); extern int power_supply_temp2resist_simple(struct power_supply_resistance_temp_table *table, int table_len, int temp);
The power-supply core supports concept of OCV (Open Circuit Voltage) => SOC (State Of Charge) conversion tables. Usually these tables are used to estimate SOC based on OCV. Some systems use so called "Zero Adjust" where at the near end-of-battery condition the SOC from coulomb counter is used to retrieve the OCV - and OCV and VSYS difference is used to re-estimate the battery capacity. Add helper to do look-up the other-way around and also get the OCV based on SOC Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> --- No changes from RFC v1 - (this should be changed to support at least 0.1% SOC accuracy - will rework for next version if this is continued) drivers/power/supply/power_supply_core.c | 51 ++++++++++++++++++++++++ include/linux/power_supply.h | 5 +++ 2 files changed, 56 insertions(+)