| Message ID | cover.1582182989.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| Headers | show |
| Series | Support ROHM BD99954 charger IC | expand |
Hi, Here are some kernel-doc comments for you: On 2/19/20 11:35 PM, Matti Vaittinen wrote: > --- > drivers/base/Kconfig | 3 + > drivers/base/Makefile | 1 + > drivers/base/linear_ranges.c | 246 +++++++++++++++++++++++++++++++++++ > include/linux/linear_range.h | 48 +++++++ > 4 files changed, 298 insertions(+) > create mode 100644 drivers/base/linear_ranges.c > create mode 100644 include/linux/linear_range.h > diff --git a/drivers/base/linear_ranges.c b/drivers/base/linear_ranges.c > new file mode 100644 > index 000000000000..5fa3b96bf2b8 > --- /dev/null > +++ b/drivers/base/linear_ranges.c > @@ -0,0 +1,246 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * linear_ranges.c -- helpers to map values in a linear range to range index > + * > + * Original idea borrowed from regulator framework > + * > + * It might be useful if we could support also inversely proportional ranges? > + * Copyright 2020 ROHM Semiconductors > + */ > + > +#include <linux/errno.h> > +#include <linux/export.h> > +#include <linux/kernel.h> > +#include <linux/linear_range.h> > + > +/** > + * linear_range_values_in_range - return the amount of values in a range > + * > + * @r: pointer to linear range where values are counted > + * > + * Compute the amount of values in range pointed by @r. Note, values can > + * be all equal - range with selectors 0,...,2 with step 0 still contains > + * 3 values even though they are all equal. > + * > + * Returns the amount of values in range pointed by @r * Return: ... > + */ > + > +/** > + * linear_range_values_in_range_array - return the amount of values in ranges > + * > + * @r: pointer to array of linear ranges where values are counted > + * @ranges: amount of ranges we include in computation. > + * > + * Compute the amount of values in ranges pointed by @r. Note, values can > + * be all equal - range with selectors 0,...,2 with step 0 still contains > + * 3 values even though they are all equal. > + * > + * Returns the amount of values in first @ranges ranges pointed by @r * Return: ... > + */ > + > +/** > + * linear_range_get_max_value - return the largest value in a range > + * > + * @r: pointer to linear range where value is looked from > + * > + * Returns the largest value in the given range ditto. > + */ > + > +/** > + * linear_range_get_value - fetch a value from given range > + * > + * @r: pointer to linear range where value is looked from > + * @selector: selector for which the value is searched > + * @val: address where found value is updated > + * > + * Search given ranges for value which matches given selector. > + * > + * Returns 0 on success, -EINVAL given selector is not found from any of the ditto. > + * ranges. > + */ > + > +/** > + * linear_range_get_value_array - fetch a value from array of ranges > + * > + * @r: pointer to array of linear ranges where value is looked from > + * @ranges: amount of ranges in an array > + * @selector: selector for which the value is searched > + * @val: address where found value is updated > + * > + * Search through an array of ranges for value which matches given selector. > + * > + * Returns 0 on success, -EINVAL given selector is not found from any of the same, again. > + * ranges. > + */ > + > +/** > + * linear_range_get_selector_low - return linear range selector for value > + * > + * @r: pointer to linear range where selector is looked from > + * @val: value for which the selcetor is searched selector > + * @selector: address where found selector value is updated > + * @found: flag to indicate that given value was in the range > + * > + * Return selector which which range value is closest match for given * Return: > + * input value. Value is matching if it is equal or smaller than given > + * value. If given value is in the range, then @found is set true. > + * > + * Returns 0 on success, -EINVAL if range is invalid or does not contain > + * value smaller or equal to given value > + */ > +int linear_range_get_selector_low(const struct linear_range *r, > + unsigned int val, unsigned int *selector, > + bool *found) > +{ > + *found = false; > + > + if (r->min > val) > + return -EINVAL; > + > + if (linear_range_get_max_value(r) >= val) > + *found = true; > + > + if (!r->step) > + *selector = r->min_sel; > + else > + *selector = (val - r->min) / r->step + r->min_sel; > + > + return 0; > +} > +EXPORT_SYMBOL(linear_range_get_selector_low); > + > +/** > + * linear_range_get_selector_low_array - return linear range selector for value > + * > + * @r: pointer to array of linear ranges where selector is looked from > + * @ranges: amount of ranges to scan from array > + * @val: value for which the selcetor is searched selector > + * @selector: address where found selector value is updated > + * @found: flag to indicate that given value was in the range > + * > + * Return Scan array of ranges for selector which which range value matches drop "Return" ? > + * given input value. Value is matching if it is equal or smaller than given > + * value. If given value is found to be in a range scannins is stopped and scanning > + * @found is set true. If a range with values smaller than given value is found > + * but the range max is being smaller than given value, then the ranges > + * biggest selector is updated to @selector but scanning ranges is continued > + * and @found is set to false. > + * > + * Returns 0 on success, -EINVAL if range array is invalid or does not contain * Return: > + * range with a value smaller or equal to given value > + */ > + > +/** > + * linear_range_get_selector_high - return linear range selector for value > + * > + * @r: pointer to linear range where selector is looked from > + * @val: value for which the selcetor is searched selector > + * @selector: address where found selector value is updated > + * @found: flag to indicate that given value was in the range > + * > + * Return selector which which range value is closest match for given > + * input value. Value is matching if it is equal or higher than given > + * value. If given value is in the range, then @found is set true. > + * > + * Returns 0 on success, -EINVAL if range is invalid or does not contain * Return: > + * value greater or equal to given value > + */ cheers.
Thanks for taking a look at this Randy :) Highly appreciated. On Wed, 2020-02-19 at 23:47 -0800, Randy Dunlap wrote: > Hi, > Here are some kernel-doc comments for you: I agreed with all the comments - I'll fix them for next version. > On 2/19/20 11:35 PM, Matti Vaittinen wrote: > > --- > > drivers/base/Kconfig | 3 + > > drivers/base/Makefile | 1 + > > drivers/base/linear_ranges.c | 246 > > +++++++++++++++++++++++++++++++++++ > > include/linux/linear_range.h | 48 +++++++ > > 4 files changed, 298 insertions(+) > > create mode 100644 drivers/base/linear_ranges.c > > create mode 100644 include/linux/linear_range.h > > diff --git a/drivers/base/linear_ranges.c > > b/drivers/base/linear_ranges.c > > new file mode 100644 > > index 000000000000..5fa3b96bf2b8 > > --- /dev/null > > +++ b/drivers/base/linear_ranges.c Best Regards, Matti Vaittinen
On Thu, Feb 20, 2020 at 09:36:10AM +0200, Matti Vaittinen wrote: > Rename the "regulator_linear_range" to more generic linear_range > as a first step towards converting the "regulator_linear_range" > to common helpers. Doesn't this introduce a build break when applied by itself? Patches should be bisectable, if you want to split things up you should introduce the new API then use it.
On Thu, Feb 20, 2020 at 09:36:38AM +0200, Matti Vaittinen wrote:
> Change the regulator helpers to use common linear_ranges code.
This needs to be squashed in with the previous commit to avoid build
breaks.
Hello Mark, On Mon, 2020-02-24 at 11:53 +0000, Mark Brown wrote: > On Thu, Feb 20, 2020 at 09:36:10AM +0200, Matti Vaittinen wrote: > > Rename the "regulator_linear_range" to more generic linear_range > > as a first step towards converting the "regulator_linear_range" > > to common helpers. > > Doesn't this introduce a build break when applied by itself? Patches > should be bisectable, if you want to split things up you should > introduce the new API then use it. Uh, I need to double check but this shouldn't cause build break as only the name of the struct is changed - and I intended to change it both in regulator header and in all of the drivers using it at same time. Or did I do some brainfart here? I just wanted to minimize the changes in patch with the widest audience. Oh, after rebasing to linux 5.6-rc2 I see that there are few new users of regulator_linear_range (I should have known that...) - natuarlly all of the users need to be covered before applying this. Br, Matti Vaittinen
Hello Mark, On Mon, 2020-02-24 at 11:57 +0000, Mark Brown wrote: > On Thu, Feb 20, 2020 at 09:36:38AM +0200, Matti Vaittinen wrote: > > Change the regulator helpers to use common linear_ranges code. > > This needs to be squashed in with the previous commit to avoid build > breaks. I don't think so. Only change required on individual regulator drivers should be renaming the struct regulator_linear_range to linear_range. Rest of the changes should be internal to regulator framework, right? Even the naming change of the linear_range struct members should not be visible to these drivers as they use the initializer macro for setting the values. I must admit I didn't compile _all_ the regulator drivers when I tested this though. I will try compiling at least most of the regulator drivers for next version though. And I think the feedback for this series has been mostly positive so I'll also drop the RFC for it. Best Regards Matti Vaittinen
On Tue, Feb 25, 2020 at 06:23:31AM +0000, Vaittinen, Matti wrote: > Only change required on individual regulator drivers should be renaming > the struct regulator_linear_range to linear_range. Rest of the changes > should be internal to regulator framework, right? Right, it's that type replacement that should be done atomically.
Hello Mark, On Tue, 2020-02-25 at 15:33 +0000, Mark Brown wrote: > On Tue, Feb 25, 2020 at 06:23:31AM +0000, Vaittinen, Matti wrote: > > > Only change required on individual regulator drivers should be > > renaming > > the struct regulator_linear_range to linear_range. Rest of the > > changes > > should be internal to regulator framework, right? > > Right, it's that type replacement that should be done atomically. Yes. And the type replacement is done only in this patch where the struct is removed from regulator driver.h header - and the linear_range header providing this new struct is included. Previous patch did not change the type - just renamed the struct. Anyways, I did compile test the patch v4 for these commits and there were no problems in regulator drivers. The BD70528 charger driver had an issue as the linear_range struct was dublicated there - but this should be fixed by the v4 where I added one extra patch doing renaming for this BD70528 charger internal structure. Best Regards Matti Vaittinen
Support ROHM BD99954 Battery Management IC ROHM BD99954 is a Battery Management IC for 1-4 cell Lithium-Ion secondary battery. BD99954 is intended to be used in space-constraint equipment such as Low profile Notebook PC, Tablets and other applications. Series extracts a "linear ranges" helper out of the regulator framework. Linear ranges helper is intended to help converting real-world values to register values when conversion is linear. I suspect this is usefull also for power subsystem and possibly for clk. This version of series introduces new battry DT binding entries and adds the parsing in power_supply_get_battery_info(). These properties can be converted to ROHM specific properties and parsing can be moved to the BD99954 driver if this does not seem like a right thing to do. I just have a feeling the the BD99954 is not the only charger which could utilize these. We also add ROHM specific charger parameters for limiting the input current(s). I think these parameters are pretty common and maybe the "rohm,"-prefix should be dropped and we should try having common input limiting properties for different chips? Series is based on v5.5-rc7 Changelog RFC-v3: DT-bindings: - fix the BD99954 binding (the *-microvolt Vs. *-microvolts issue is still there. Not sure which one is correct) - renabe tricklecharge-* binding to trickle-charge-* as suggested by Rob. - drop the linear-ranges helper which was written for BD70528 and extract the linear-range code from regulator framework instead. - refactor regulator framework to utilize extracted linear-ranges code. - change the struct regulator_linear_range to linear_range from regulator drivers. - refactor BD70528 to use regulator framework originated linear-ranges code. - change BD99954 to use linear-ranges code from regulator framework Changelog RFC-v2: DT-bindings: - Used the battery parameters described in battery.txt - Added few new parameters to battery.txt - Added ASCII art charging profile chart for BD99954 to explain states and limits. Linear ranges: - Fixed division by zero error from linear-ranges code if step 0 is used. Power-supply core: - Added parsing of new battery parameters. BD99954 driver: - converted to use battery parameters from battery node - Added step 0 ranges for reg values which do not change voltage - added dt-node to psy-config Patch 1: DT binding docs for the new battery parameters Patch 2: BD99954 charger DT binding docs Patch 3: Linear ranges helpers Patch 4: Rename struct regulator_linear_range to struct linear_range and convert regulator drivers to use renamed struct Patch 5: Use linear-ranges helpers in regulator framework Patch 6: Use linear-ranges helpers in bd70528 driver Patch 7: Parsing of new battery parameters Patch 8: ROHM BD99954 charger IC driver --- Matti Vaittinen (8): dt-bindings: battry: add new battery parameters dt_bindings: ROHM BD99954 Charger drivers: base: add linear ranges helpers regulator: rename regulator_linear_range to linear_range regulator: use linear_ranges helper power: supply: bd70528: use linear ranges power: supply: add battery parameters power: supply: Support ROHM bd99954 charger .../bindings/power/supply/battery.txt | 6 + .../bindings/power/supply/rohm,bd9995x.yaml | 153 +++ drivers/base/Kconfig | 3 + drivers/base/Makefile | 1 + drivers/base/linear_ranges.c | 246 ++++ drivers/power/supply/Kconfig | 11 + drivers/power/supply/Makefile | 1 + drivers/power/supply/bd70528-charger.c | 142 +- drivers/power/supply/bd99954-charger.c | 1171 +++++++++++++++++ drivers/power/supply/power_supply_core.c | 8 + drivers/regulator/88pg86x.c | 4 +- drivers/regulator/88pm800-regulator.c | 4 +- drivers/regulator/Kconfig | 1 + drivers/regulator/act8865-regulator.c | 4 +- drivers/regulator/act8945a-regulator.c | 2 +- drivers/regulator/arizona-ldo1.c | 2 +- drivers/regulator/arizona-micsupp.c | 4 +- drivers/regulator/as3711-regulator.c | 6 +- drivers/regulator/as3722-regulator.c | 4 +- drivers/regulator/axp20x-regulator.c | 16 +- drivers/regulator/bcm590xx-regulator.c | 8 +- drivers/regulator/bd70528-regulator.c | 8 +- drivers/regulator/bd718x7-regulator.c | 26 +- drivers/regulator/da903x.c | 2 +- drivers/regulator/helpers.c | 130 +- drivers/regulator/hi6421-regulator.c | 4 +- drivers/regulator/lochnagar-regulator.c | 4 +- drivers/regulator/lp873x-regulator.c | 4 +- drivers/regulator/lp87565-regulator.c | 2 +- drivers/regulator/lp8788-buck.c | 2 +- drivers/regulator/max77650-regulator.c | 2 +- drivers/regulator/mcp16502.c | 4 +- drivers/regulator/mt6323-regulator.c | 6 +- drivers/regulator/mt6358-regulator.c | 8 +- drivers/regulator/mt6380-regulator.c | 6 +- drivers/regulator/mt6397-regulator.c | 6 +- drivers/regulator/palmas-regulator.c | 4 +- drivers/regulator/qcom-rpmh-regulator.c | 2 +- drivers/regulator/qcom_rpm-regulator.c | 14 +- drivers/regulator/qcom_smd-regulator.c | 70 +- drivers/regulator/rk808-regulator.c | 10 +- drivers/regulator/s2mps11.c | 14 +- drivers/regulator/sky81452-regulator.c | 2 +- drivers/regulator/stpmic1_regulator.c | 18 +- drivers/regulator/tps65086-regulator.c | 10 +- drivers/regulator/tps65217-regulator.c | 4 +- drivers/regulator/tps65218-regulator.c | 6 +- drivers/regulator/tps65912-regulator.c | 4 +- drivers/regulator/twl-regulator.c | 4 +- drivers/regulator/twl6030-regulator.c | 2 +- drivers/regulator/wm831x-dcdc.c | 2 +- drivers/regulator/wm831x-ldo.c | 4 +- drivers/regulator/wm8350-regulator.c | 2 +- drivers/regulator/wm8400-regulator.c | 2 +- include/linux/linear_range.h | 48 + include/linux/power/bd99954-charger.h | 1075 +++++++++++++++ include/linux/power_supply.h | 4 + include/linux/regulator/driver.h | 27 +- 58 files changed, 3000 insertions(+), 339 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml create mode 100644 drivers/base/linear_ranges.c create mode 100644 drivers/power/supply/bd99954-charger.c create mode 100644 include/linux/linear_range.h create mode 100644 include/linux/power/bd99954-charger.h base-commit def9d2780727 ("Linux 5.5-rc7")