[PATCH v4 0/5] new macro: find_closest()

[PATCH v4 0/5] new macro: find_closest()

[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.
The second updates Documentation/CodingStyle on the naming
convention for local variables in macros resembling functions.
Other three patches replace duplicated code with calls to one
of these macros in some hwmon drivers.

v4:
- use >= instead of > in find_closest_descending()
- fix kernel docs
- make local variables in added macros less likely to cause namespace
  collisions
- use consistent statement expression syntax

v3:
https://lkml.org/lkml/2015/3/19/369

v2:
https://lkml.org/lkml/2015/3/10/582

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

Bartosz Golaszewski (5):
  util_macros.h: add find_closest() macro
  documentation: update CodingStyle on local variables naming in macros
  hwmon: (ina2xx) replace ina226_avg_bits() with find_closest()
  hwmon: (lm85) use find_closest() in x_TO_REG() functions
  hwmon: (w83795) use find_closest_descending() in pwm_freq_to_reg()

 Documentation/CodingStyle   | 13 +++++++++++++
 drivers/hwmon/ina2xx.c      | 17 +++--------------
 drivers/hwmon/lm85.c        | 26 ++++++++------------------
 drivers/hwmon/w83795.c      |  8 +++-----
 include/linux/util_macros.h | 40 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 67 insertions(+), 37 deletions(-)
 create mode 100644 include/linux/util_macros.h

-- 
2.1.4

[PATCH v4 1/5] util_macros.h: add find_closest() macro

[Bartosz Golaszewski]

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

Add a new include - util_macros.h - and 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/util_macros.h | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 include/linux/util_macros.h

diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
new file mode 100644
index 0000000..d5f4fb6
--- /dev/null
+++ b/include/linux/util_macros.h
@@ -0,0 +1,40 @@
+#ifndef _LINUX_HELPER_MACROS_H_
+#define _LINUX_HELPER_MACROS_H_
+
+#define __find_closest(x, a, as, op)					\
+({									\
+	typeof(as) __fc_i, __fc_as = (as) - 1;				\
+	typeof(x) __fc_x = (x);						\
+	typeof(*a) *__fc_a = (a);					\
+	for (__fc_i = 0; __fc_i < __fc_as; __fc_i++) {			\
+		if (__fc_x op DIV_ROUND_CLOSEST(__fc_a[__fc_i] +	\
+						__fc_a[__fc_i + 1], 2))	\
+			break;						\
+	}								\
+	(__fc_i);							\
+})
+
+/**
+ * find_closest - locate the closest element in a sorted array
+ * @x: The reference value.
+ * @a: The array in which to look for the closest element. Must be sorted
+ *  in ascending order.
+ * @as: Size of 'a'.
+ *
+ * Returns the index of the element closest to 'x'.
+ */
+#define find_closest(x, a, as) __find_closest(x, a, as, <=)
+
+/**
+ * find_closest_descending - locate the closest element in a sorted array
+ * @x: The reference value.
+ * @a: The array in which to look for the closest element. Must be sorted
+ *  in descending order.
+ * @as: Size of 'a'.
+ *
+ * Similar to find_closest() but 'a' is expected to be sorted in descending
+ * order.
+ */
+#define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
+
+#endif
-- 
2.1.4

[PATCH v4 2/5] documentation: update CodingStyle on local variables naming in macros

[Bartosz Golaszewski]

Describe proper naming convention for local variables in macros
resembling functions.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 Documentation/CodingStyle | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 449a8a1..4d4f06d 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -659,6 +659,19 @@ macros using parameters.
 #define CONSTANT 0x4000
 #define CONSTEXP (CONSTANT | 3)
 
+5) namespace collisions when defining local variables in macros resembling
+functions:
+
+#define FOO(x)				\
+({					\
+	typeof(x) ret;			\
+	ret = calc_ret(x);		\
+	(ret);				\
+)}
+
+ret is a common name for a local variable - __foo_ret is less likely
+to collide with an existing variable.
+
 The cpp manual deals with macros exhaustively. The gcc internals manual also
 covers RTL which is used frequently with assembly language in the kernel.
 
-- 
2.1.4

[PATCH v4 3/5] hwmon: (ina2xx) replace ina226_avg_bits() with find_closest()

[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 | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index d1542b7..4d28150 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -36,6 +36,7 @@
 #include <linux/jiffies.h>
 #include <linux/of.h>
 #include <linux/delay.h>
+#include <linux/util_macros.h>
 
 #include <linux/platform_data/ina2xx.h>
 
@@ -141,19 +142,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 +159,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

[PATCH v4 4/5] hwmon: (lm85) use find_closest() in x_TO_REG() functions

[Bartosz Golaszewski]

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

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

diff --git a/drivers/hwmon/lm85.c b/drivers/hwmon/lm85.c
index 2b4b419..6ff773f 100644
--- a/drivers/hwmon/lm85.c
+++ b/drivers/hwmon/lm85.c
@@ -34,6 +34,7 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
+#include <linux/util_macros.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2c, 0x2d, 0x2e, I2C_CLIENT_END };
@@ -190,15 +191,7 @@ static const int lm85_range_map[] = {
 
 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;
+	return find_closest(range, lm85_range_map, ARRAY_SIZE(lm85_range_map));
 }
 #define RANGE_FROM_REG(val)	lm85_range_map[(val) & 0x0f]
 
@@ -209,16 +202,12 @@ 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
 };
+#define FREQ_MAP_LEN	8
 
-static int FREQ_TO_REG(const int *map, unsigned long freq)
+static int FREQ_TO_REG(const int *map,
+		       unsigned int map_size, 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;
+	return find_closest(freq, map, map_size);
 }
 
 static int FREQ_FROM_REG(const int *map, u8 reg)
@@ -828,7 +817,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] = FREQ_TO_REG(data->freq_map,
+						 FREQ_MAP_LEN, val);
 		lm85_write_value(client, LM85_REG_AFAN_RANGE(nr),
 				 (data->zone[nr].range << 4)
 				 | data->pwm_freq[nr]);
-- 
2.1.4

[PATCH v4 5/5] hwmon: (w83795) use find_closest_descending() in pwm_freq_to_reg()

[Bartosz Golaszewski]

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

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

diff --git a/drivers/hwmon/w83795.c b/drivers/hwmon/w83795.c
index 2189413..49276bb 100644
--- a/drivers/hwmon/w83795.c
+++ b/drivers/hwmon/w83795.c
@@ -35,6 +35,7 @@
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/jiffies.h>
+#include <linux/util_macros.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = {
@@ -308,11 +309,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_descending(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

Re: [PATCH v4 2/5] documentation: update CodingStyle on local variables naming in macros

[Joe Perches]

On Thu, 2015-03-19 at 18:27 +0100, Bartosz Golaszewski wrote:
> Describe proper naming convention for local variables in macros
> resembling functions.
[]
> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
[]

> +5) namespace collisions when defining local variables in macros resembling
> +functions:
> +
> +#define FOO(x)				\
> +({					\
> +	typeof(x) ret;			\
> +	ret = calc_ret(x);		\
> +	(ret);				\

trivia: ret wouldn't need parentheses here.

> +)}
> +
> +ret is a common name for a local variable - __foo_ret is less likely
> +to collide with an existing variable.

Is shadowing really much of a problem here?

These are statement expression macros and names
have local scope anyway.

It's similar to function arguments using the same
names as parameters.

Re: [PATCH v4 1/5] util_macros.h: add find_closest() macro

[Andrew Morton]

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

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

Linear search is pretty nasty.  lib/bsearch.c could do this
with a bit of tweaking.  But that's a separate project.