On Fri, 2021-05-28 at 16:12 +0800, Gene Chen wrote:
> From: Gene Chen <gene_chen@xxxxxxxxxxx>
>
> Add linear range get selector within for choose closest selector
> between minimum and maximum selector.
>
> Signed-off-by: Gene Chen <gene_chen@xxxxxxxxxxx>
> ---
> include/linux/linear_range.h | 2 ++
> lib/linear_ranges.c | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/linux/linear_range.h
> b/include/linux/linear_range.h
> index 17b5943727d5..fd3d0b358f22 100644
> --- a/include/linux/linear_range.h
> +++ b/include/linux/linear_range.h
> @@ -41,6 +41,8 @@ int linear_range_get_selector_low(const struct
> linear_range *r,
> int linear_range_get_selector_high(const struct linear_range *r,
> unsigned int val, unsigned int
> *selector,
> bool *found);
> +void linear_range_get_selector_within(const struct linear_range *r,
> + unsigned int val, unsigned int
> *selector);
> int linear_range_get_selector_low_array(const struct linear_range
> *r,
> int ranges, unsigned int val,
> unsigned int *selector, bool
> *found);
> diff --git a/lib/linear_ranges.c b/lib/linear_ranges.c
> index ced5c15d3f04..a1a7dfa881de 100644
> --- a/lib/linear_ranges.c
> +++ b/lib/linear_ranges.c
> @@ -241,5 +241,36 @@ int linear_range_get_selector_high(const struct
> linear_range *r,
> }
> EXPORT_SYMBOL_GPL(linear_range_get_selector_high);
>
> +/**
> + * linear_range_get_selector_within - return linear range selector
> for value
> + * @r: pointer to linear range where selector is
> looked from
> + * @val: value for which the selector is searched
> + * @selector: address where found selector value is updated
> + *
> + * Return selector for which range value is closest match for given
> + * input value. Value is matching if it is equal or lower than given
> + * value. But return maximum selector if given value is higher than
> + * maximum value.
> + */
> +void linear_range_get_selector_within(const struct linear_range *r,
> + unsigned int val, unsigned int
> *selector)
I like the naming! The "within" sounds good to me :)
It slightly bothers my "style eye" to see this not returning a value
(void) - but still returning the value via parameter (selector). It may
be slightly more complicated to read when used.
Please consider for first time reading code like:
unsigned int do_something(const struct linear_range *r, unsigned int
val, int *myvalue)
{
int ret;
...
linear_range_get_selector_within(r, val, myvalue);
...
return ret;
}
to reading code like:
unsigned int do_something(const struct linear_range *r, unsigned int
val, int *myvalue)
{
int ret;
...
*myvalue = linear_range_get_selector_within(r, val);
...
return ret;
}
For me reading the latter shows better where the myvalue is really set.
OTOH, Your suggestion is consistent with the other functions so I am
not asking for a change if this is Ok with others :) Thanks a lot!
Reviewed-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> +{
> + if (r->min > val) {
> + *selector = r->min_sel;
> + return;
> + }
> +
> + if (linear_range_get_max_value(r) < val) {
> + *selector = r->max_sel;
> + return;
> + }
> +
> + if (r->step == 0)
> + *selector = r->min_sel;
> + else
> + *selector = (val - r->min) / r->step + r->min_sel;
> +}
> +EXPORT_SYMBOL_GPL(linear_range_get_selector_within);
> +
> MODULE_DESCRIPTION("linear-ranges helper");
> MODULE_LICENSE("GPL");
