LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2 0/4] kernel.h: find_closest() macro
@ 2015-03-10 17:27 Bartosz Golaszewski
  2015-03-10 17:27 ` [PATCH v2 1/4] kernel.h: add " Bartosz Golaszewski
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2015-03-10 17:27 UTC (permalink / raw)
  To: LKML
  Cc: Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt,
	Bartosz Golaszewski

This series proposes to unduplicate the code used to find the
member in an array closest to 'x'.

The first patch adds a macro implementing the algorithm in two
flavors - for arrays sorted in ascending and descending order.
Other three patches replace duplicated code with calls to one
of these macros in some hwmon drivers.

v2:
- fixed an off-by-one bug in patch [3/4]

v1:
https://lkml.org/lkml/2015/2/24/509

Bartosz Golaszewski (4):
  kernel.h: add find_closest() macro
  hwmon: (ina2xx) replace ina226_avg_bits() with find_closest()
  hwmon: (lm85) replace x_TO_REG() functions with find_closest()
  hwmon: (w83795) use find_closest_desc() in pwm_freq_to_reg()

 drivers/hwmon/ina2xx.c | 16 ++--------------
 drivers/hwmon/lm85.c   | 43 ++++++++++++-------------------------------
 drivers/hwmon/w83795.c |  7 ++-----
 include/linux/kernel.h | 23 +++++++++++++++++++++++
 4 files changed, 39 insertions(+), 50 deletions(-)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] kernel.h: add find_closest() macro
  2015-03-10 17:27 [PATCH v2 0/4] kernel.h: find_closest() macro Bartosz Golaszewski
@ 2015-03-10 17:27 ` Bartosz Golaszewski
  2015-03-10 18:27   ` Joe Perches
  2015-03-12 23:07   ` Andrew Morton
  2015-03-10 17:27 ` [PATCH v2 2/4] hwmon: (ina2xx) replace ina226_avg_bits() with find_closest() Bartosz Golaszewski
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2015-03-10 17:27 UTC (permalink / raw)
  To: LKML
  Cc: Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt,
	Bartosz Golaszewski

Searching for the member of an array closest to 'x' is
duplicated in several places.

Add two macros that implement this algorithm for arrays
sorted both in ascending and descending order.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 include/linux/kernel.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d6d630d..f4294da 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -116,6 +116,29 @@
 }							\
 )
 
+#define __find_closest(x, a, as, op)(					\
+{									\
+	typeof(as) _i, _as = (as) - 1;					\
+	typeof(x) _x = (x);						\
+	typeof(*a) *_a = (a);						\
+	for (_i = 0; _i < _as; _i++) {					\
+		if (_x op DIV_ROUND_CLOSEST(_a[_i] + _a[_i + 1], 2))	\
+			break;						\
+	}								\
+	(_i);								\
+}									\
+)
+
+/*
+ * Given an array 'a' (sorted in ascending order) of size 'as' return
+ * the index of the element in that array closest to 'x'.
+ */
+#define find_closest(x, a, as) __find_closest(x, a, as, <=)
+/*
+ * Similar to find_closest(), but 'a' is expected to be sorted
+ * in descending order.
+ */
+#define find_closest_desc(x, a, as) __find_closest(x, a, as, >)
 
 #define _RET_IP_		(unsigned long)__builtin_return_address(0)
 #define _THIS_IP_  ({ __label__ __here; __here: (unsigned long)&&__here; })
-- 
2.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] hwmon: (ina2xx) replace ina226_avg_bits() with find_closest()
  2015-03-10 17:27 [PATCH v2 0/4] kernel.h: find_closest() macro Bartosz Golaszewski
  2015-03-10 17:27 ` [PATCH v2 1/4] kernel.h: add " Bartosz Golaszewski
@ 2015-03-10 17:27 ` Bartosz Golaszewski
  2015-03-10 17:27 ` [PATCH v2 3/4] hwmon: (lm85) replace x_TO_REG() functions " Bartosz Golaszewski
  2015-03-10 17:27 ` [PATCH v2 4/4] hwmon: (w83795) use find_closest_desc() in pwm_freq_to_reg() Bartosz Golaszewski
  3 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2015-03-10 17:27 UTC (permalink / raw)
  To: LKML
  Cc: Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt,
	Bartosz Golaszewski

Use find_closest() to locate the closest average in ina226_avg_tab.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/hwmon/ina2xx.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index d1542b7..fc7f023 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -141,19 +141,6 @@ static const struct ina2xx_config ina2xx_config[] = {
  */
 static const int ina226_avg_tab[] = { 1, 4, 16, 64, 128, 256, 512, 1024 };
 
-static int ina226_avg_bits(int avg)
-{
-	int i;
-
-	/* Get the closest average from the tab. */
-	for (i = 0; i < ARRAY_SIZE(ina226_avg_tab) - 1; i++) {
-		if (avg <= (ina226_avg_tab[i] + ina226_avg_tab[i + 1]) / 2)
-			break;
-	}
-
-	return i; /* Return 0b0111 for values greater than 1024. */
-}
-
 static int ina226_reg_to_interval(u16 config)
 {
 	int avg = ina226_avg_tab[INA226_READ_AVG(config)];
@@ -171,7 +158,8 @@ static u16 ina226_interval_to_reg(int interval, u16 config)
 
 	avg = DIV_ROUND_CLOSEST(interval * 1000,
 				INA226_TOTAL_CONV_TIME_DEFAULT);
-	avg_bits = ina226_avg_bits(avg);
+	avg_bits = find_closest(avg, ina226_avg_tab,
+				ARRAY_SIZE(ina226_avg_tab));
 
 	return (config & ~INA226_AVG_RD_MASK) | INA226_SHIFT_AVG(avg_bits);
 }
-- 
2.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] hwmon: (lm85) replace x_TO_REG() functions with find_closest()
  2015-03-10 17:27 [PATCH v2 0/4] kernel.h: find_closest() macro Bartosz Golaszewski
  2015-03-10 17:27 ` [PATCH v2 1/4] kernel.h: add " Bartosz Golaszewski
  2015-03-10 17:27 ` [PATCH v2 2/4] hwmon: (ina2xx) replace ina226_avg_bits() with find_closest() Bartosz Golaszewski
@ 2015-03-10 17:27 ` Bartosz Golaszewski
  2015-03-13  8:53   ` Rasmus Villemoes
  2015-03-10 17:27 ` [PATCH v2 4/4] hwmon: (w83795) use find_closest_desc() in pwm_freq_to_reg() Bartosz Golaszewski
  3 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2015-03-10 17:27 UTC (permalink / raw)
  To: LKML
  Cc: Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt,
	Bartosz Golaszewski

Replace RANGE_TO_REG() and FREQ_TO_REG() functions with calls
to find_closest().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/hwmon/lm85.c | 43 ++++++++++++-------------------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/drivers/hwmon/lm85.c b/drivers/hwmon/lm85.c
index 2b4b419..a0c0050 100644
--- a/drivers/hwmon/lm85.c
+++ b/drivers/hwmon/lm85.c
@@ -188,18 +188,6 @@ static const int lm85_range_map[] = {
 	13300, 16000, 20000, 26600, 32000, 40000, 53300, 80000
 };
 
-static int RANGE_TO_REG(long range)
-{
-	int i;
-
-	/* Find the closest match */
-	for (i = 0; i < 15; ++i) {
-		if (range <= (lm85_range_map[i] + lm85_range_map[i + 1]) / 2)
-			break;
-	}
-
-	return i;
-}
 #define RANGE_FROM_REG(val)	lm85_range_map[(val) & 0x0f]
 
 /* These are the PWM frequency encodings */
@@ -209,17 +197,7 @@ static const int lm85_freq_map[8] = { /* 1 Hz */
 static const int adm1027_freq_map[8] = { /* 1 Hz */
 	11, 15, 22, 29, 35, 44, 59, 88
 };
-
-static int FREQ_TO_REG(const int *map, unsigned long freq)
-{
-	int i;
-
-	/* Find the closest match */
-	for (i = 0; i < 7; ++i)
-		if (freq <= (map[i] + map[i + 1]) / 2)
-			break;
-	return i;
-}
+#define FREQ_MAP_SIZE		8
 
 static int FREQ_FROM_REG(const int *map, u8 reg)
 {
@@ -828,7 +806,8 @@ static ssize_t set_pwm_freq(struct device *dev,
 		data->cfg5 &= ~ADT7468_HFPWM;
 		lm85_write_value(client, ADT7468_REG_CFG5, data->cfg5);
 	} else {					/* Low freq. mode */
-		data->pwm_freq[nr] = FREQ_TO_REG(data->freq_map, val);
+		data->pwm_freq[nr] = find_closest(val, data->freq_map,
+						  FREQ_MAP_SIZE);
 		lm85_write_value(client, LM85_REG_AFAN_RANGE(nr),
 				 (data->zone[nr].range << 4)
 				 | data->pwm_freq[nr]);
@@ -1186,7 +1165,7 @@ static ssize_t set_temp_auto_temp_min(struct device *dev,
 	int nr = to_sensor_dev_attr(attr)->index;
 	struct lm85_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
-	long val;
+	long val, range;
 	int err;
 
 	err = kstrtol(buf, 10, &val);
@@ -1198,10 +1177,12 @@ static ssize_t set_temp_auto_temp_min(struct device *dev,
 	lm85_write_value(client, LM85_REG_AFAN_LIMIT(nr),
 		data->zone[nr].limit);
 
-/* Update temp_auto_max and temp_auto_range */
-	data->zone[nr].range = RANGE_TO_REG(
-		TEMP_FROM_REG(data->zone[nr].max_desired) -
-		TEMP_FROM_REG(data->zone[nr].limit));
+	/* Update temp_auto_max and temp_auto_range */
+	range = TEMP_FROM_REG(data->zone[nr].max_desired) -
+		TEMP_FROM_REG(data->zone[nr].limit);
+	data->zone[nr].range = find_closest(range, lm85_range_map,
+					    ARRAY_SIZE(lm85_range_map));
+
 	lm85_write_value(client, LM85_REG_AFAN_RANGE(nr),
 		((data->zone[nr].range & 0x0f) << 4)
 		| (data->pwm_freq[nr] & 0x07));
@@ -1236,8 +1217,8 @@ static ssize_t set_temp_auto_temp_max(struct device *dev,
 	mutex_lock(&data->update_lock);
 	min = TEMP_FROM_REG(data->zone[nr].limit);
 	data->zone[nr].max_desired = TEMP_TO_REG(val);
-	data->zone[nr].range = RANGE_TO_REG(
-		val - min);
+	data->zone[nr].range = find_closest(val - min, lm85_range_map,
+					    ARRAY_SIZE(lm85_range_map));
 	lm85_write_value(client, LM85_REG_AFAN_RANGE(nr),
 		((data->zone[nr].range & 0x0f) << 4)
 		| (data->pwm_freq[nr] & 0x07));
-- 
2.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] hwmon: (w83795) use find_closest_desc() in pwm_freq_to_reg()
  2015-03-10 17:27 [PATCH v2 0/4] kernel.h: find_closest() macro Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2015-03-10 17:27 ` [PATCH v2 3/4] hwmon: (lm85) replace x_TO_REG() functions " Bartosz Golaszewski
@ 2015-03-10 17:27 ` Bartosz Golaszewski
  3 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2015-03-10 17:27 UTC (permalink / raw)
  To: LKML
  Cc: Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt,
	Bartosz Golaszewski

Replace the loop iterating over pwm_freq_cksel0 with a call to
find_closest_desc().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/hwmon/w83795.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/w83795.c b/drivers/hwmon/w83795.c
index 2189413..1f0b301 100644
--- a/drivers/hwmon/w83795.c
+++ b/drivers/hwmon/w83795.c
@@ -308,11 +308,8 @@ static u8 pwm_freq_to_reg(unsigned long val, u16 clkin)
 	unsigned long best0, best1;
 
 	/* Best fit for cksel = 0 */
-	for (reg0 = 0; reg0 < ARRAY_SIZE(pwm_freq_cksel0) - 1; reg0++) {
-		if (val > (pwm_freq_cksel0[reg0] +
-			   pwm_freq_cksel0[reg0 + 1]) / 2)
-			break;
-	}
+	reg0 = find_closest_desc(val, pwm_freq_cksel0,
+				 ARRAY_SIZE(pwm_freq_cksel0));
 	if (val < 375)	/* cksel = 1 can't beat this */
 		return reg0;
 	best0 = pwm_freq_cksel0[reg0];
-- 
2.1.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/4] kernel.h: add find_closest() macro
  2015-03-10 17:27 ` [PATCH v2 1/4] kernel.h: add " Bartosz Golaszewski
@ 2015-03-10 18:27   ` Joe Perches
  2015-03-12 23:07   ` Andrew Morton
  1 sibling, 0 replies; 8+ messages in thread
From: Joe Perches @ 2015-03-10 18:27 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: LKML, Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt

On Tue, 2015-03-10 at 18:27 +0100, Bartosz Golaszewski wrote:
> Searching for the member of an array closest to 'x' is
> duplicated in several places.
[]
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
[]
> @@ -116,6 +116,29 @@
>  }							\
>  )
>  
> +#define __find_closest(x, a, as, op)(					\
> +{									\
> +	typeof(as) _i, _as = (as) - 1;						\
> +	typeof(x) _x = (x);						\
> +	typeof(*a) *_a = (a);						\
> +	for (_i = 0; _i < _as; _i++) {					\
> +		if (_x op DIV_ROUND_CLOSEST(_a[_i] + _a[_i + 1], 2))	\
> +			break;						\
> +	}								\
> +	(_i);								\
> +}									\
> +)

Please use more descriptive variable names.

Most kernel statement expression macros consolidate
the "({" and "})" uses on single lines

#define sem(args) {(		\
	etc...			\
)}

> +
> +/*
> + * Given an array 'a' (sorted in ascending order) of size 'as' return
> + * the index of the element in that array closest to 'x'.
> + */

It'd be nice to use kernel-doc comments here.

> +#define find_closest(x, a, as) __find_closest(x, a, as, <=)
> +/*
> + * Similar to find_closest(), but 'a' is expected to be sorted
> + * in descending order.
> + */

And here.

> +#define find_closest_desc(x, a, as) __find_closest(x, a, as, >)

Shouldn't find_closest and find_closest_dest use
equivalent comparison?

>= ?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/4] kernel.h: add find_closest() macro
  2015-03-10 17:27 ` [PATCH v2 1/4] kernel.h: add " Bartosz Golaszewski
  2015-03-10 18:27   ` Joe Perches
@ 2015-03-12 23:07   ` Andrew Morton
  1 sibling, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2015-03-12 23:07 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: LKML, Guenter Roeck, lm-sensors, Steven Rostedt

On Tue, 10 Mar 2015 18:27:19 +0100 Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote:

> +/*
> + * Similar to find_closest(), but 'a' is expected to be sorted
> + * in descending order.
> + */
> +#define find_closest_desc(x, a, as) __find_closest(x, a, as, >)

description?  descriptor? descendent?  oh, it's "descending".  So
let's use "descending"!

I think it's good to put possibly-useful stuff like this into
includes/linux, particularly when we already have several usage sites. 
But we don't _have_ to keep putting things into kernel.h.  Maybe a new
include/linux/handy-macros.h or include/linux/find_closest.h?


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/4] hwmon: (lm85) replace x_TO_REG() functions with find_closest()
  2015-03-10 17:27 ` [PATCH v2 3/4] hwmon: (lm85) replace x_TO_REG() functions " Bartosz Golaszewski
@ 2015-03-13  8:53   ` Rasmus Villemoes
  0 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2015-03-13  8:53 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: LKML, Guenter Roeck, lm-sensors, Andrew Morton, Steven Rostedt

On Tue, Mar 10 2015, Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote:

> Replace RANGE_TO_REG() and FREQ_TO_REG() functions with calls
> to find_closest().
>

I think the other functions you've replaced only had a single caller,
but RANGE_TO_REG is called in two places. It's not a huge function and 2
is not a big number, but maybe it's better to let gcc decide whether to
inline the code. So how about leaving the *_TO_REG functions in place
and just replace their bodies with the appropriate macro invocation?

Rasmus

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-03-13  8:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-10 17:27 [PATCH v2 0/4] kernel.h: find_closest() macro Bartosz Golaszewski
2015-03-10 17:27 ` [PATCH v2 1/4] kernel.h: add " Bartosz Golaszewski
2015-03-10 18:27   ` Joe Perches
2015-03-12 23:07   ` Andrew Morton
2015-03-10 17:27 ` [PATCH v2 2/4] hwmon: (ina2xx) replace ina226_avg_bits() with find_closest() Bartosz Golaszewski
2015-03-10 17:27 ` [PATCH v2 3/4] hwmon: (lm85) replace x_TO_REG() functions " Bartosz Golaszewski
2015-03-13  8:53   ` Rasmus Villemoes
2015-03-10 17:27 ` [PATCH v2 4/4] hwmon: (w83795) use find_closest_desc() in pwm_freq_to_reg() Bartosz Golaszewski

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).