[PATCH v1 01/11] iio: light: tsl2563: Do not hardcode interrupt trigger type

[PATCH v1 01/11] iio: light: tsl2563: Do not hardcode interrupt trigger type

[Andy Shevchenko]

From: Ferry Toth <ftoth@exalondelft.nl>

Instead of hardcoding IRQ trigger type to IRQF_TRIGGER_RAISING,
let's respect the settings specified in the firmware description.
To be compatible with the older firmware descriptions, if trigger
type is not set up there, we'll set it to default (raising edge).

Fixes: 388be4883952 ("staging:iio: tsl2563 abi fixes and interrupt handling")
Fixes: bdab1001738f ("staging:iio:light:tsl2563 remove old style event registration.")
Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/light/tsl2563.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index d0e42b73203a..71302ae864d9 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -704,6 +704,7 @@ static int tsl2563_probe(struct i2c_client *client)
 	struct iio_dev *indio_dev;
 	struct tsl2563_chip *chip;
 	struct tsl2563_platform_data *pdata = client->dev.platform_data;
+	unsigned long irq_flags;
 	int err = 0;
 	u8 id = 0;
 
@@ -759,10 +760,15 @@ static int tsl2563_probe(struct i2c_client *client)
 		indio_dev->info = &tsl2563_info_no_irq;
 
 	if (client->irq) {
+		irq_flags = irq_get_trigger_type(client->irq);
+		if (irq_flags == IRQF_TRIGGER_NONE)
+			irq_flags = IRQF_TRIGGER_RISING;
+		irq_flags |= IRQF_ONESHOT;
+
 		err = devm_request_threaded_irq(&client->dev, client->irq,
 					   NULL,
 					   &tsl2563_event_handler,
-					   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					   irq_flags,
 					   "tsl2563_event",
 					   indio_dev);
 		if (err) {
-- 
2.35.1

[PATCH v1 02/11] iio: light: tsl2563: Use i2c_smbus_write_word_data() in tsl2563_configure()

[Andy Shevchenko]

Driver already uses the word accessors when it makes sense, but
in the tsl2563_configure(). Switch the latter to use word accessor.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 52 ++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index 71302ae864d9..d836c15ba777 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -49,16 +49,12 @@
 
 #define TSL2563_REG_CTRL	0x00
 #define TSL2563_REG_TIMING	0x01
-#define TSL2563_REG_LOWLOW	0x02 /* data0 low threshold, 2 bytes */
-#define TSL2563_REG_LOWHIGH	0x03
-#define TSL2563_REG_HIGHLOW	0x04 /* data0 high threshold, 2 bytes */
-#define TSL2563_REG_HIGHHIGH	0x05
+#define TSL2563_REG_LOW		0x02 /* data0 low threshold, 2 bytes */
+#define TSL2563_REG_HIGH	0x04 /* data0 high threshold, 2 bytes */
 #define TSL2563_REG_INT		0x06
 #define TSL2563_REG_ID		0x0a
-#define TSL2563_REG_DATA0LOW	0x0c /* broadband sensor value, 2 bytes */
-#define TSL2563_REG_DATA0HIGH	0x0d
-#define TSL2563_REG_DATA1LOW	0x0e /* infrared sensor value, 2 bytes */
-#define TSL2563_REG_DATA1HIGH	0x0f
+#define TSL2563_REG_DATA0	0x0c /* broadband sensor value, 2 bytes */
+#define TSL2563_REG_DATA1	0x0e /* infrared sensor value, 2 bytes */
 
 #define TSL2563_CMD_POWER_ON	0x03
 #define TSL2563_CMD_POWER_OFF	0x00
@@ -161,24 +157,16 @@ static int tsl2563_configure(struct tsl2563_chip *chip)
 			chip->gainlevel->gaintime);
 	if (ret)
 		goto error_ret;
-	ret = i2c_smbus_write_byte_data(chip->client,
-			TSL2563_CMD | TSL2563_REG_HIGHLOW,
-			chip->high_thres & 0xFF);
-	if (ret)
-		goto error_ret;
-	ret = i2c_smbus_write_byte_data(chip->client,
-			TSL2563_CMD | TSL2563_REG_HIGHHIGH,
-			(chip->high_thres >> 8) & 0xFF);
+	ret = i2c_smbus_write_word_data(chip->client,
+			TSL2563_CMD | TSL2563_REG_HIGH,
+			chip->high_thres);
 	if (ret)
 		goto error_ret;
-	ret = i2c_smbus_write_byte_data(chip->client,
-			TSL2563_CMD | TSL2563_REG_LOWLOW,
-			chip->low_thres & 0xFF);
+	ret = i2c_smbus_write_word_data(chip->client,
+			TSL2563_CMD | TSL2563_REG_LOW,
+			chip->low_thres);
 	if (ret)
 		goto error_ret;
-	ret = i2c_smbus_write_byte_data(chip->client,
-			TSL2563_CMD | TSL2563_REG_LOWHIGH,
-			(chip->low_thres >> 8) & 0xFF);
 /*
  * Interrupt register is automatically written anyway if it is relevant
  * so is not here.
@@ -325,13 +313,13 @@ static int tsl2563_get_adc(struct tsl2563_chip *chip)
 
 	while (retry) {
 		ret = i2c_smbus_read_word_data(client,
-				TSL2563_CMD | TSL2563_REG_DATA0LOW);
+				TSL2563_CMD | TSL2563_REG_DATA0);
 		if (ret < 0)
 			goto out;
 		adc0 = ret;
 
 		ret = i2c_smbus_read_word_data(client,
-				TSL2563_CMD | TSL2563_REG_DATA1LOW);
+				TSL2563_CMD | TSL2563_REG_DATA1);
 		if (ret < 0)
 			goto out;
 		adc1 = ret;
@@ -584,20 +572,18 @@ static int tsl2563_write_thresh(struct iio_dev *indio_dev,
 {
 	struct tsl2563_chip *chip = iio_priv(indio_dev);
 	int ret;
-	u8 address;
+
+	mutex_lock(&chip->lock);
 
 	if (dir == IIO_EV_DIR_RISING)
-		address = TSL2563_REG_HIGHLOW;
+		ret = i2c_smbus_write_word_data(chip->client,
+						TSL2563_CMD | TSL2563_REG_HIGH, val);
 	else
-		address = TSL2563_REG_LOWLOW;
-	mutex_lock(&chip->lock);
-	ret = i2c_smbus_write_byte_data(chip->client, TSL2563_CMD | address,
-					val & 0xFF);
+		ret = i2c_smbus_write_word_data(chip->client,
+						TSL2563_CMD | TSL2563_REG_LOW, val);
 	if (ret)
 		goto error_ret;
-	ret = i2c_smbus_write_byte_data(chip->client,
-					TSL2563_CMD | (address + 1),
-					(val >> 8) & 0xFF);
+
 	if (dir == IIO_EV_DIR_RISING)
 		chip->high_thres = val;
 	else
-- 
2.35.1

[PATCH v1 03/11] iio: light: tsl2563: Configure INT in one place

[Andy Shevchenko]

Introduce tsl2563_configure_irq() to configure INT in one place.

While at it, make use of TSL2563_INT_LEVEL and newly introduced
TSL2563_INT_MASK.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 42 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index d836c15ba777..d071805239ef 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -69,6 +69,7 @@
 
 #define TSL2563_INT_DISABLED	0x00
 #define TSL2563_INT_LEVEL	0x10
+#define TSL2563_INT_MASK	0x30
 #define TSL2563_INT_PERSIST(n)	((n) & 0x0F)
 
 struct tsl2563_gainlevel_coeff {
@@ -211,6 +212,24 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
 	return 0;
 }
 
+static int tsl2563_configure_irq(struct tsl2563_chip *chip, bool enable)
+{
+	int ret;
+
+	chip->intr &= ~TSL2563_INT_MASK;
+	if (enable)
+		chip->intr |= TSL2563_INT_LEVEL;
+
+	ret = i2c_smbus_write_byte_data(chip->client,
+					TSL2563_CMD | TSL2563_REG_INT,
+					chip->intr);
+	if (ret < 0)
+		return ret;
+
+	chip->int_enabled = enable;
+	return 0;
+}
+
 /*
  * "Normalized" ADC value is one obtained with 400ms of integration time and
  * 16x gain. This function returns the number of bits of shift needed to
@@ -620,9 +639,7 @@ static int tsl2563_write_interrupt_config(struct iio_dev *indio_dev,
 	int ret = 0;
 
 	mutex_lock(&chip->lock);
-	if (state && !(chip->intr & 0x30)) {
-		chip->intr &= ~0x30;
-		chip->intr |= 0x10;
+	if (state && !(chip->intr & TSL2563_INT_MASK)) {
 		/* ensure the chip is actually on */
 		cancel_delayed_work_sync(&chip->poweroff_work);
 		if (!tsl2563_get_power(chip)) {
@@ -633,18 +650,11 @@ static int tsl2563_write_interrupt_config(struct iio_dev *indio_dev,
 			if (ret)
 				goto out;
 		}
-		ret = i2c_smbus_write_byte_data(chip->client,
-						TSL2563_CMD | TSL2563_REG_INT,
-						chip->intr);
-		chip->int_enabled = true;
+		ret = tsl2563_configure_irq(chip, true);
 	}
 
-	if (!state && (chip->intr & 0x30)) {
-		chip->intr &= ~0x30;
-		ret = i2c_smbus_write_byte_data(chip->client,
-						TSL2563_CMD | TSL2563_REG_INT,
-						chip->intr);
-		chip->int_enabled = false;
+	if (!state && (chip->intr & TSL2563_INT_MASK)) {
+		ret = tsl2563_configure_irq(chip, false);
 		/* now the interrupt is not enabled, we can go to sleep */
 		schedule_delayed_work(&chip->poweroff_work, 5 * HZ);
 	}
@@ -668,7 +678,7 @@ static int tsl2563_read_interrupt_config(struct iio_dev *indio_dev,
 	if (ret < 0)
 		return ret;
 
-	return !!(ret & 0x30);
+	return !!(ret & TSL2563_INT_MASK);
 }
 
 static const struct iio_info tsl2563_info_no_irq = {
@@ -796,9 +806,7 @@ static void tsl2563_remove(struct i2c_client *client)
 	if (!chip->int_enabled)
 		cancel_delayed_work_sync(&chip->poweroff_work);
 	/* Ensure that interrupts are disabled - then flush any bottom halves */
-	chip->intr &= ~0x30;
-	i2c_smbus_write_byte_data(chip->client, TSL2563_CMD | TSL2563_REG_INT,
-				  chip->intr);
+	tsl2563_configure_irq(chip, false);
 	tsl2563_set_power(chip, 0);
 }
 
-- 
2.35.1

[PATCH v1 04/11] iio: light: tsl2563: Make use of the macros from bits.h

[Andy Shevchenko]

Make use of BIT() and GENMASK() where it makes sense.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index d071805239ef..3b60d8000351 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -11,6 +11,7 @@
  * Amit Kucheria <amit.kucheria@verdurent.com>
  */
 
+#include <linux/bits.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/property.h>
@@ -33,19 +34,19 @@
 #define ADC_FRAC_BITS		14
 
 /* Given number of 1/10000's in ADC_FRAC_BITS precision. */
-#define FRAC10K(f)		(((f) * (1L << (ADC_FRAC_BITS))) / (10000))
+#define FRAC10K(f)		(((f) * BIT(ADC_FRAC_BITS)) / (10000))
 
 /* Bits used for fraction in calibration coefficients.*/
 #define CALIB_FRAC_BITS		10
 /* 0.5 in CALIB_FRAC_BITS precision */
-#define CALIB_FRAC_HALF		(1 << (CALIB_FRAC_BITS - 1))
+#define CALIB_FRAC_HALF		BIT(CALIB_FRAC_BITS - 1)
 /* Make a fraction from a number n that was multiplied with b. */
 #define CALIB_FRAC(n, b)	(((n) << CALIB_FRAC_BITS) / (b))
 /* Decimal 10^(digits in sysfs presentation) */
 #define CALIB_BASE_SYSFS	1000
 
-#define TSL2563_CMD		0x80
-#define TSL2563_CLEARINT	0x40
+#define TSL2563_CMD		BIT(7)
+#define TSL2563_CLEARINT	BIT(6)
 
 #define TSL2563_REG_CTRL	0x00
 #define TSL2563_REG_TIMING	0x01
@@ -58,19 +59,19 @@
 
 #define TSL2563_CMD_POWER_ON	0x03
 #define TSL2563_CMD_POWER_OFF	0x00
-#define TSL2563_CTRL_POWER_MASK	0x03
+#define TSL2563_CTRL_POWER_MASK	GENMASK(1, 0)
 
 #define TSL2563_TIMING_13MS	0x00
 #define TSL2563_TIMING_100MS	0x01
 #define TSL2563_TIMING_400MS	0x02
-#define TSL2563_TIMING_MASK	0x03
+#define TSL2563_TIMING_MASK	GENMASK(1, 0)
 #define TSL2563_TIMING_GAIN16	0x10
 #define TSL2563_TIMING_GAIN1	0x00
 
 #define TSL2563_INT_DISABLED	0x00
 #define TSL2563_INT_LEVEL	0x10
-#define TSL2563_INT_MASK	0x30
-#define TSL2563_INT_PERSIST(n)	((n) & 0x0F)
+#define TSL2563_INT_MASK	GENMASK(5, 4)
+#define TSL2563_INT_PERSIST(n)	((n) & GENMASK(3, 0))
 
 struct tsl2563_gainlevel_coeff {
 	u8 gaintime;
-- 
2.35.1

[PATCH v1 05/11] iio: light: tsl2563: Drop unused defintion(s)

[Andy Shevchenko]

The CALIB_FRAC() is defined and might had been used in
tsl2563_calib_from_sysfs(). But let's just move a comment
from it to the latter function.

CLAIB_FRAC_HALF is used in a single place, i.e. in
tsl2563_calib_to_sysfs(). So, let's just inline it there.

While at it, switch to use DIV_ROUND_CLOSEST().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index 3b60d8000351..bdd40a5df53d 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/sched.h>
+#include <linux/math.h>
 #include <linux/mutex.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
@@ -38,10 +39,6 @@
 
 /* Bits used for fraction in calibration coefficients.*/
 #define CALIB_FRAC_BITS		10
-/* 0.5 in CALIB_FRAC_BITS precision */
-#define CALIB_FRAC_HALF		BIT(CALIB_FRAC_BITS - 1)
-/* Make a fraction from a number n that was multiplied with b. */
-#define CALIB_FRAC(n, b)	(((n) << CALIB_FRAC_BITS) / (b))
 /* Decimal 10^(digits in sysfs presentation) */
 #define CALIB_BASE_SYSFS	1000
 
@@ -360,12 +357,12 @@ static int tsl2563_get_adc(struct tsl2563_chip *chip)
 
 static inline int tsl2563_calib_to_sysfs(u32 calib)
 {
-	return (int) (((calib * CALIB_BASE_SYSFS) +
-		       CALIB_FRAC_HALF) >> CALIB_FRAC_BITS);
+	return (int)DIV_ROUND_CLOSEST(calib * CALIB_BASE_SYSFS, BIT(CALIB_FRAC_BITS));
 }
 
 static inline u32 tsl2563_calib_from_sysfs(int value)
 {
+	/* Make a fraction from a number n that was multiplied with b. */
 	return (((u32) value) << CALIB_FRAC_BITS) / CALIB_BASE_SYSFS;
 }
 
-- 
2.35.1

[PATCH v1 06/11] iio: light: tsl2563: Simplify with dev_err_probe

[Andy Shevchenko]

Code can be a bit simpler with dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index bdd40a5df53d..cce044556293 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -695,12 +695,13 @@ static const struct iio_info tsl2563_info = {
 
 static int tsl2563_probe(struct i2c_client *client)
 {
+	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	struct tsl2563_chip *chip;
 	struct tsl2563_platform_data *pdata = client->dev.platform_data;
 	unsigned long irq_flags;
-	int err = 0;
 	u8 id = 0;
+	int err;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
@@ -712,16 +713,12 @@ static int tsl2563_probe(struct i2c_client *client)
 	chip->client = client;
 
 	err = tsl2563_detect(chip);
-	if (err) {
-		dev_err(&client->dev, "detect error %d\n", -err);
-		return err;
-	}
+	if (err)
+		return dev_err_probe(dev, err, "detect error\n");
 
 	err = tsl2563_read_id(chip, &id);
-	if (err) {
-		dev_err(&client->dev, "read id error %d\n", -err);
-		return err;
-	}
+	if (err)
+		return dev_err_probe(dev, err, "read id error\n");
 
 	mutex_init(&chip->lock);
 
@@ -765,17 +762,13 @@ static int tsl2563_probe(struct i2c_client *client)
 					   irq_flags,
 					   "tsl2563_event",
 					   indio_dev);
-		if (err) {
-			dev_err(&client->dev, "irq request error %d\n", -err);
-			return err;
-		}
+		if (err)
+			return dev_err_probe(dev, err, "irq request error\n");
 	}
 
 	err = tsl2563_configure(chip);
-	if (err) {
-		dev_err(&client->dev, "configure error %d\n", -err);
-		return err;
-	}
+	if (err)
+		return dev_err_probe(dev, err, "configure error\n");
 
 	INIT_DELAYED_WORK(&chip->poweroff_work, tsl2563_poweroff_work);
 
@@ -784,7 +777,7 @@ static int tsl2563_probe(struct i2c_client *client)
 
 	err = iio_device_register(indio_dev);
 	if (err) {
-		dev_err(&client->dev, "iio registration error %d\n", -err);
+		dev_err_probe(dev, err, "iio registration error\n");
 		goto fail;
 	}
 
-- 
2.35.1

[PATCH v1 07/11] iio: light: tsl2563: Drop legacy platform data code

[Andy Shevchenko]

There is no in-kernel user for legacy platform data.
Otherwise, a new one can use software nodes instead.
Hence, drop legacy platform data code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c           | 12 ++----------
 include/linux/platform_data/tsl2563.h |  9 ---------
 2 files changed, 2 insertions(+), 19 deletions(-)
 delete mode 100644 include/linux/platform_data/tsl2563.h

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index cce044556293..ed193a3da91e 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -29,7 +29,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
-#include <linux/platform_data/tsl2563.h>
 
 /* Use this many bits for fraction part. */
 #define ADC_FRAC_BITS		14
@@ -698,7 +697,6 @@ static int tsl2563_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	struct tsl2563_chip *chip;
-	struct tsl2563_platform_data *pdata = client->dev.platform_data;
 	unsigned long irq_flags;
 	u8 id = 0;
 	int err;
@@ -730,14 +728,8 @@ static int tsl2563_probe(struct i2c_client *client)
 	chip->calib0 = tsl2563_calib_from_sysfs(CALIB_BASE_SYSFS);
 	chip->calib1 = tsl2563_calib_from_sysfs(CALIB_BASE_SYSFS);
 
-	if (pdata) {
-		chip->cover_comp_gain = pdata->cover_comp_gain;
-	} else {
-		err = device_property_read_u32(&client->dev, "amstaos,cover-comp-gain",
-					       &chip->cover_comp_gain);
-		if (err)
-			chip->cover_comp_gain = 1;
-	}
+	chip->cover_comp_gain = 1;
+	device_property_read_u32(dev, "amstaos,cover-comp-gain", &chip->cover_comp_gain);
 
 	dev_info(&client->dev, "model %d, rev. %d\n", id >> 4, id & 0x0f);
 	indio_dev->name = client->name;
diff --git a/include/linux/platform_data/tsl2563.h b/include/linux/platform_data/tsl2563.h
deleted file mode 100644
index 9cf9309c3f24..000000000000
--- a/include/linux/platform_data/tsl2563.h
+++ /dev/null
@@ -1,9 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __LINUX_TSL2563_H
-#define __LINUX_TSL2563_H
-
-struct tsl2563_platform_data {
-	int cover_comp_gain;
-};
-
-#endif /* __LINUX_TSL2563_H */
-- 
2.35.1

[PATCH v1 08/11] iio: light: tsl2563: Utilise temporary variable for struct device

[Andy Shevchenko]

We have a temporary variable to keep pointer to struct device.
Utilise it inside the ->probe() implementation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index ed193a3da91e..c5814545fd19 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -701,7 +701,7 @@ static int tsl2563_probe(struct i2c_client *client)
 	u8 id = 0;
 	int err;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -731,7 +731,7 @@ static int tsl2563_probe(struct i2c_client *client)
 	chip->cover_comp_gain = 1;
 	device_property_read_u32(dev, "amstaos,cover-comp-gain", &chip->cover_comp_gain);
 
-	dev_info(&client->dev, "model %d, rev. %d\n", id >> 4, id & 0x0f);
+	dev_info(dev, "model %d, rev. %d\n", id >> 4, id & 0x0f);
 	indio_dev->name = client->name;
 	indio_dev->channels = tsl2563_channels;
 	indio_dev->num_channels = ARRAY_SIZE(tsl2563_channels);
@@ -748,7 +748,7 @@ static int tsl2563_probe(struct i2c_client *client)
 			irq_flags = IRQF_TRIGGER_RISING;
 		irq_flags |= IRQF_ONESHOT;
 
-		err = devm_request_threaded_irq(&client->dev, client->irq,
+		err = devm_request_threaded_irq(dev, client->irq,
 					   NULL,
 					   &tsl2563_event_handler,
 					   irq_flags,
-- 
2.35.1

[PATCH v1 09/11] iio: light: tsl2563: Use dev_get_drvdata() directly in PM callbacks

[Andy Shevchenko]

PM callbacks take struct device pointer as a parameter, use
dev_get_drvdata() to retrieve it instead of unneeded double
loop of referencing via i2c_get_clientdata(to_i2c_client(dev)).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index c5814545fd19..f92190fadffd 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -795,7 +795,7 @@ static void tsl2563_remove(struct i2c_client *client)
 
 static int tsl2563_suspend(struct device *dev)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct tsl2563_chip *chip = iio_priv(indio_dev);
 	int ret;
 
@@ -814,7 +814,7 @@ static int tsl2563_suspend(struct device *dev)
 
 static int tsl2563_resume(struct device *dev)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct tsl2563_chip *chip = iio_priv(indio_dev);
 	int ret;
 
-- 
2.35.1

[PATCH v1 10/11] iio: light: tsl2563: sort header inclusion alphabetically

[Andy Shevchenko]

Sort header inclusion alphabetically.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/tsl2563.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
index f92190fadffd..f2f55239a072 100644
--- a/drivers/iio/light/tsl2563.c
+++ b/drivers/iio/light/tsl2563.c
@@ -12,23 +12,23 @@
  */
 
 #include <linux/bits.h>
-#include <linux/module.h>
-#include <linux/mod_devicetable.h>
-#include <linux/property.h>
+#include <linux/delay.h>
+#include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/sched.h>
 #include <linux/math.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/delay.h>
 #include <linux/pm.h>
-#include <linux/err.h>
+#include <linux/property.h>
+#include <linux/sched.h>
 #include <linux/slab.h>
 
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-#include <linux/iio/events.h>
 
 /* Use this many bits for fraction part. */
 #define ADC_FRAC_BITS		14
-- 
2.35.1

[PATCH v1 11/11] iio: light: tsl2563: Keep Makefile sorted by module name

[Andy Shevchenko]

The Makefile is sorted by a module name, keep it that way.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Ferry Toth <ftoth@exalondelft.nl>
---
 drivers/iio/light/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 6f23817fae6f..d74d2b5ff14c 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -39,7 +39,6 @@ obj-$(CONFIG_NOA1305)		+= noa1305.o
 obj-$(CONFIG_OPT3001)		+= opt3001.o
 obj-$(CONFIG_PA12203001)	+= pa12203001.o
 obj-$(CONFIG_RPR0521)		+= rpr0521.o
-obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
 obj-$(CONFIG_SI1133)		+= si1133.o
 obj-$(CONFIG_SI1145)		+= si1145.o
 obj-$(CONFIG_STK3310)          += stk3310.o
@@ -48,6 +47,7 @@ obj-$(CONFIG_ST_UVIS25_I2C)	+= st_uvis25_i2c.o
 obj-$(CONFIG_ST_UVIS25_SPI)	+= st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)		+= tcs3414.o
 obj-$(CONFIG_TCS3472)		+= tcs3472.o
+obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
 obj-$(CONFIG_TSL2583)		+= tsl2583.o
 obj-$(CONFIG_TSL2591)		+= tsl2591.o
 obj-$(CONFIG_TSL2772)		+= tsl2772.o
-- 
2.35.1

Re: [PATCH v1 03/11] iio: light: tsl2563: Configure INT in one place

[Jonathan Cameron]

On Wed,  7 Dec 2022 21:03:40 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Introduce tsl2563_configure_irq() to configure INT in one place.
> 
> While at it, make use of TSL2563_INT_LEVEL and newly introduced
> TSL2563_INT_MASK.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Ferry Toth <ftoth@exalondelft.nl>
Trivial note on a further improvement inline.

LGTM
> ---
>  drivers/iio/light/tsl2563.c | 42 ++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
> index d836c15ba777..d071805239ef 100644
> --- a/drivers/iio/light/tsl2563.c
> +++ b/drivers/iio/light/tsl2563.c
> @@ -69,6 +69,7 @@
>  
>  #define TSL2563_INT_DISABLED	0x00
>  #define TSL2563_INT_LEVEL	0x10
> +#define TSL2563_INT_MASK	0x30

Better to use GENMASK etc, but given age of driver I'm not surprised no one
has made the conversion to that and FIELD_GET/ FIELD_PREP.

Nice follow up if anyone is bored enough ;)


>  #define TSL2563_INT_PERSIST(n)	((n) & 0x0F)
>  

Re: [PATCH v1 04/11] iio: light: tsl2563: Make use of the macros from bits.h

[Jonathan Cameron]

On Wed,  7 Dec 2022 21:03:41 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Make use of BIT() and GENMASK() where it makes sense.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Ferry Toth <ftoth@exalondelft.nl>
Hohum.  I did look to see if there was a GENMASK() patch before
the comment on previous one, but apparently my eyes skipped
this one! Ignore me as this is exactly what I was suggesting.

J

Re: [PATCH v1 01/11] iio: light: tsl2563: Do not hardcode interrupt trigger type

[Jonathan Cameron]

On Wed,  7 Dec 2022 21:03:38 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> From: Ferry Toth <ftoth@exalondelft.nl>
> 
> Instead of hardcoding IRQ trigger type to IRQF_TRIGGER_RAISING,
> let's respect the settings specified in the firmware description.
> To be compatible with the older firmware descriptions, if trigger
> type is not set up there, we'll set it to default (raising edge).
> 
> Fixes: 388be4883952 ("staging:iio: tsl2563 abi fixes and interrupt handling")
> Fixes: bdab1001738f ("staging:iio:light:tsl2563 remove old style event registration.")
> Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Andy, would have preferred a cover letter, so I had an obvious place
to reply to the whole series...

Mostly I'm amazed anyone still has one of these devices (I have one but
it's on a break out board for the stargate2/imote2 pxa27x platform that we
dropped support for last year - I hadn't booted it for a few years)
- I can probably bodge it onto something else but I can't say it was
high on my todo list ;)  So nice to know that someone still cares about
this.

So I'm curious Ferry, what device has one of these?

Whole series applied to the togreg branch of iio.git though note I'll only
push this out as testing for now because I'll want to rebase that tree
after rc1 is available.

Thanks,

Jonathan

> ---
>  drivers/iio/light/tsl2563.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
> index d0e42b73203a..71302ae864d9 100644
> --- a/drivers/iio/light/tsl2563.c
> +++ b/drivers/iio/light/tsl2563.c
> @@ -704,6 +704,7 @@ static int tsl2563_probe(struct i2c_client *client)
>  	struct iio_dev *indio_dev;
>  	struct tsl2563_chip *chip;
>  	struct tsl2563_platform_data *pdata = client->dev.platform_data;
> +	unsigned long irq_flags;
>  	int err = 0;
>  	u8 id = 0;
>  
> @@ -759,10 +760,15 @@ static int tsl2563_probe(struct i2c_client *client)
>  		indio_dev->info = &tsl2563_info_no_irq;
>  
>  	if (client->irq) {
> +		irq_flags = irq_get_trigger_type(client->irq);
> +		if (irq_flags == IRQF_TRIGGER_NONE)
> +			irq_flags = IRQF_TRIGGER_RISING;
> +		irq_flags |= IRQF_ONESHOT;
> +
>  		err = devm_request_threaded_irq(&client->dev, client->irq,
>  					   NULL,
>  					   &tsl2563_event_handler,
> -					   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					   irq_flags,
>  					   "tsl2563_event",
>  					   indio_dev);
>  		if (err) {

Re: [PATCH v1 01/11] iio: light: tsl2563: Do not hardcode interrupt trigger type

[Ferry Toth]

Hi,

Op 11-12-2022 om 14:26 schreef Jonathan Cameron:
> On Wed,  7 Dec 2022 21:03:38 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> From: Ferry Toth <ftoth@exalondelft.nl>
>>
>> Instead of hardcoding IRQ trigger type to IRQF_TRIGGER_RAISING,
>> let's respect the settings specified in the firmware description.
>> To be compatible with the older firmware descriptions, if trigger
>> type is not set up there, we'll set it to default (raising edge).
>>
>> Fixes: 388be4883952 ("staging:iio: tsl2563 abi fixes and interrupt handling")
>> Fixes: bdab1001738f ("staging:iio:light:tsl2563 remove old style event registration.")
>> Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Andy, would have preferred a cover letter, so I had an obvious place
> to reply to the whole series...
> 
> Mostly I'm amazed anyone still has one of these devices (I have one but
> it's on a break out board for the stargate2/imote2 pxa27x platform that we
> dropped support for last year - I hadn't booted it for a few years)
> - I can probably bodge it onto something else but I can't say it was
> high on my todo list ;)  So nice to know that someone still cares about
> this.
> 
> So I'm curious Ferry, what device has one of these?

It's a breakout board too. I think it's something like GY-2561.

I wanted to write up an example how to get connect iio sensors to work 
with linux. So I asked my colleague who is a great fan of aliexpress if 
he had any sensor on a breakout board with I2C. In the past I had it 
working with MRAA and UPM but that seems to be a dead end now.

We have ACPI working on Intel Edison-Arduino with quite a few examples 
from Andy. And the "Arduino" header makes it very easy to wire up these 
kind of breakout boards, fantastic platform this type of developments.

Just wiring up the I2C and get it to work was easy enough. And then the 
interrupt pin makes an interesting example (even though likely useless 
for most applications of the light sensor).

Write-up here if you are interested:
https://htot.github.io/meta-intel-edison/4.6-libiio.html

> Whole series applied to the togreg branch of iio.git though note I'll only
> push this out as testing for now because I'll want to rebase that tree
> after rc1 is available.
> 
> Thanks,
> 
> Jonathan
> 
>> ---
>>   drivers/iio/light/tsl2563.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
>> index d0e42b73203a..71302ae864d9 100644
>> --- a/drivers/iio/light/tsl2563.c
>> +++ b/drivers/iio/light/tsl2563.c
>> @@ -704,6 +704,7 @@ static int tsl2563_probe(struct i2c_client *client)
>>   	struct iio_dev *indio_dev;
>>   	struct tsl2563_chip *chip;
>>   	struct tsl2563_platform_data *pdata = client->dev.platform_data;
>> +	unsigned long irq_flags;
>>   	int err = 0;
>>   	u8 id = 0;
>>   
>> @@ -759,10 +760,15 @@ static int tsl2563_probe(struct i2c_client *client)
>>   		indio_dev->info = &tsl2563_info_no_irq;
>>   
>>   	if (client->irq) {
>> +		irq_flags = irq_get_trigger_type(client->irq);
>> +		if (irq_flags == IRQF_TRIGGER_NONE)
>> +			irq_flags = IRQF_TRIGGER_RISING;
>> +		irq_flags |= IRQF_ONESHOT;
>> +
>>   		err = devm_request_threaded_irq(&client->dev, client->irq,
>>   					   NULL,
>>   					   &tsl2563_event_handler,
>> -					   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> +					   irq_flags,
>>   					   "tsl2563_event",
>>   					   indio_dev);
>>   		if (err) {
> 

Re: [PATCH v1 01/11] iio: light: tsl2563: Do not hardcode interrupt trigger type

[Jonathan Cameron]

On Sun, 11 Dec 2022 18:14:01 +0100
Ferry Toth <fntoth@gmail.com> wrote:

> Hi,
> 
> Op 11-12-2022 om 14:26 schreef Jonathan Cameron:
> > On Wed,  7 Dec 2022 21:03:38 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> >> From: Ferry Toth <ftoth@exalondelft.nl>
> >>
> >> Instead of hardcoding IRQ trigger type to IRQF_TRIGGER_RAISING,
> >> let's respect the settings specified in the firmware description.
> >> To be compatible with the older firmware descriptions, if trigger
> >> type is not set up there, we'll set it to default (raising edge).
> >>
> >> Fixes: 388be4883952 ("staging:iio: tsl2563 abi fixes and interrupt handling")
> >> Fixes: bdab1001738f ("staging:iio:light:tsl2563 remove old style event registration.")
> >> Signed-off-by: Ferry Toth <ftoth@exalondelft.nl>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> > 
> > Andy, would have preferred a cover letter, so I had an obvious place
> > to reply to the whole series...
> > 
> > Mostly I'm amazed anyone still has one of these devices (I have one but
> > it's on a break out board for the stargate2/imote2 pxa27x platform that we
> > dropped support for last year - I hadn't booted it for a few years)
> > - I can probably bodge it onto something else but I can't say it was
> > high on my todo list ;)  So nice to know that someone still cares about
> > this.
> > 
> > So I'm curious Ferry, what device has one of these?  
> 
> It's a breakout board too. I think it's something like GY-2561.
> 
> I wanted to write up an example how to get connect iio sensors to work 
> with linux. So I asked my colleague who is a great fan of aliexpress if 
> he had any sensor on a breakout board with I2C. In the past I had it 
> working with MRAA and UPM but that seems to be a dead end now.
> 
> We have ACPI working on Intel Edison-Arduino with quite a few examples 
> from Andy. And the "Arduino" header makes it very easy to wire up these 
> kind of breakout boards, fantastic platform this type of developments.
> 
> Just wiring up the I2C and get it to work was easy enough. And then the 
> interrupt pin makes an interesting example (even though likely useless 
> for most applications of the light sensor).
> 
> Write-up here if you are interested:
> https://htot.github.io/meta-intel-edison/4.6-libiio.html

Thanks!

Jonathan

> 
> > Whole series applied to the togreg branch of iio.git though note I'll only
> > push this out as testing for now because I'll want to rebase that tree
> > after rc1 is available.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> >> ---
> >>   drivers/iio/light/tsl2563.c | 8 +++++++-
> >>   1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c
> >> index d0e42b73203a..71302ae864d9 100644
> >> --- a/drivers/iio/light/tsl2563.c
> >> +++ b/drivers/iio/light/tsl2563.c
> >> @@ -704,6 +704,7 @@ static int tsl2563_probe(struct i2c_client *client)
> >>   	struct iio_dev *indio_dev;
> >>   	struct tsl2563_chip *chip;
> >>   	struct tsl2563_platform_data *pdata = client->dev.platform_data;
> >> +	unsigned long irq_flags;
> >>   	int err = 0;
> >>   	u8 id = 0;
> >>   
> >> @@ -759,10 +760,15 @@ static int tsl2563_probe(struct i2c_client *client)
> >>   		indio_dev->info = &tsl2563_info_no_irq;
> >>   
> >>   	if (client->irq) {
> >> +		irq_flags = irq_get_trigger_type(client->irq);
> >> +		if (irq_flags == IRQF_TRIGGER_NONE)
> >> +			irq_flags = IRQF_TRIGGER_RISING;
> >> +		irq_flags |= IRQF_ONESHOT;
> >> +
> >>   		err = devm_request_threaded_irq(&client->dev, client->irq,
> >>   					   NULL,
> >>   					   &tsl2563_event_handler,
> >> -					   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> >> +					   irq_flags,
> >>   					   "tsl2563_event",
> >>   					   indio_dev);
> >>   		if (err) {  
> >   
>