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