Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1677978 > unrolled thread

[PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups Brian Masney <masneyb@onstation.org> - 2017-06-29 19:20 +0200
    [PATCH 2/6] staging: iio: tsl2x7x: remove redundant power_state sysfs attribute Brian Masney <masneyb@onstation.org> - 2017-06-29 19:20 +0200
    [PATCH 6/6] staging: iio: tsl2x7x: use usleep_range() instead of mdelay() Brian Masney <masneyb@onstation.org> - 2017-06-29 19:20 +0200
    [PATCH 1/6] staging: iio: tsl2x7x: add of_match table for device tree support Brian Masney <masneyb@onstation.org> - 2017-06-29 19:20 +0200
    [PATCH 3/6] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read() Brian Masney <masneyb@onstation.org> - 2017-06-29 19:20 +0200
      Re: [PATCH 3/6] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read() Frans Klaver <fransklaver@gmail.com> - 2017-06-29 21:30 +0200
    Re: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups Brian Masney <masneyb@onstation.org> - 2017-06-30 17:30 +0200
      RE: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups Jon Brenner <Jon.Brenner@ams.com> - 2017-06-30 18:20 +0200
      Re: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups Jonathan Cameron <jic23@kernel.org> - 2017-07-01 11:50 +0200

#1677978 — [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

This begins my work to clean this driver up and eventually move it out
of staging. Driver changes were tested using a TSL2771 hooked up to a
Raspberry Pi 2.

Thanks to Jon Brenner at AMS/TAOS for loaning me some hardware samples
to test my driver changes.

Brian Masney (6):
  staging: iio: tsl2x7x: add of_match table for device tree support
  staging: iio: tsl2x7x: remove redundant power_state sysfs attribute
  staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()
  staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()
  staging: iio: tsl2x7x: refactor {read,write}_event_value to allow
    handling multiple iio_event_infos
  staging: iio: tsl2x7x: use usleep_range() instead of mdelay()

 .../devicetree/bindings/trivial-devices.txt        |  10 +
 drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
 2 files changed, 153 insertions(+), 173 deletions(-)

-- 
2.9.4

[toc] | [next] | [standalone]

#1677980 — [PATCH 2/6] staging: iio: tsl2x7x: remove redundant power_state sysfs attribute

The TSL2X7X driver has a custom power_state sysfs attribute. Remove this
attribute since the runtime power management code provides a sysfs
attribute to control the power state of the device.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 3f4f1f3..3ea29e2 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -915,33 +915,6 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 		tsl2x7x_chip_on(indio_dev);
 }
 
-static ssize_t power_state_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", chip->tsl2x7x_chip_status);
-}
-
-static ssize_t power_state_store(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	bool value;
-
-	if (strtobool(buf, &value))
-		return -EINVAL;
-
-	if (value)
-		tsl2x7x_chip_on(indio_dev);
-	else
-		tsl2x7x_chip_off(indio_dev);
-
-	return len;
-}
-
 static ssize_t in_illuminance0_calibscale_available_show(struct device *dev,
 					   struct device_attribute *attr,
 					   char *buf)
@@ -1494,8 +1467,6 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR_RW(power_state);
-
 static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
 
 static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
@@ -1580,7 +1551,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 }
 
 static struct attribute *tsl2x7x_ALS_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_illuminance0_calibscale_available.attr,
 	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
@@ -1591,13 +1561,11 @@ static struct attribute *tsl2x7x_ALS_device_attrs[] = {
 };
 
 static struct attribute *tsl2x7x_PRX_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_proximity0_calibrate.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_illuminance0_calibscale_available.attr,
 	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
@@ -1609,14 +1577,12 @@ static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
 };
 
 static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_proximity0_calibrate.attr,
 	&dev_attr_in_proximity0_calibscale_available.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_illuminance0_calibscale_available.attr,
 	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1677982 — [PATCH 6/6] staging: iio: tsl2x7x: use usleep_range() instead of mdelay()

This driver in some cases can busy wait for upwards of 15ms. Since the
kernel at this point is not running in atomic context, and is running in
process context, we can safely use usleep_range() instead. This patch
changes the two occurrences of mdelay() to usleep_range().

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 6a5d58c..8195faa 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -687,7 +687,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 		}
 	}
 
-	mdelay(3);	/* Power-on settling time */
+	/* Power-on settling time */
+	usleep_range(3000, 3500);
 
 	/*
 	 * NOW enable the ADC
@@ -854,7 +855,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 
 	/*gather the samples*/
 	for (i = 0; i < chip->tsl2x7x_settings.prox_max_samples_cal; i++) {
-		mdelay(15);
+		usleep_range(15000, 17500);
 		tsl2x7x_get_prox(indio_dev);
 		prox_history[i] = chip->prox_data;
 		dev_info(&chip->client->dev, "2 i=%d prox data= %d\n",
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1677987 — [PATCH 1/6] staging: iio: tsl2x7x: add of_match table for device tree support

Add device tree support for the tsl2x7x IIO driver with no custom
properties.

Signed-off-by: Brian Masney <masneyb@onstation.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/trivial-devices.txt | 10 ++++++++++
 drivers/staging/iio/light/tsl2x7x.c                   | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt
index 35f406d..0e6e953 100644
--- a/Documentation/devicetree/bindings/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/trivial-devices.txt
@@ -21,6 +21,16 @@ adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
 adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 ams,iaq-core		AMS iAQ-Core VOC Sensor
+amstaos,tsl2571		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2671		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2671		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2771		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2771		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2572		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2672		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2672		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2772		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2772		AMS/TAOS ALS and proximity sensor
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,at97sc3204t	i2c trusted platform module (TPM)
 capella,cm32181		CM32181: Ambient Light Sensor
diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 1467199..3f4f1f3 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -2026,6 +2026,21 @@ static struct i2c_device_id tsl2x7x_idtable[] = {
 
 MODULE_DEVICE_TABLE(i2c, tsl2x7x_idtable);
 
+static const struct of_device_id tsl2x7x_of_match[] = {
+        { .compatible = "amstaos,tsl2571" },
+        { .compatible = "amstaos,tsl2671" },
+        { .compatible = "amstaos,tmd2671" },
+        { .compatible = "amstaos,tsl2771" },
+        { .compatible = "amstaos,tmd2771" },
+        { .compatible = "amstaos,tsl2572" },
+        { .compatible = "amstaos,tsl2672" },
+        { .compatible = "amstaos,tmd2672" },
+        { .compatible = "amstaos,tsl2772" },
+        { .compatible = "amstaos,tmd2772" },
+        {}
+};
+MODULE_DEVICE_TABLE(of, tsl2x7x_of_match);
+
 static const struct dev_pm_ops tsl2x7x_pm_ops = {
 	.suspend = tsl2x7x_suspend,
 	.resume  = tsl2x7x_resume,
@@ -2035,6 +2050,7 @@ static const struct dev_pm_ops tsl2x7x_pm_ops = {
 static struct i2c_driver tsl2x7x_driver = {
 	.driver = {
 		.name = "tsl2x7x",
+		.of_match_table = tsl2x7x_of_match,
 		.pm = &tsl2x7x_pm_ops,
 	},
 	.id_table = tsl2x7x_idtable,
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1677988 — [PATCH 3/6] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()

tsl2x7x_i2c_read() would call i2c_smbus_write_byte() and
i2c_smbus_read_byte(). These two i2c functions can be replaced with a
single call to i2c_smbus_read_byte_data(). This patch removes the
tsl2x7x_i2c_read() function and replaces all occurances with a call to
i2c_smbus_read_byte_data().

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 71 ++++++++++++-------------------------
 1 file changed, 22 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 3ea29e2..d917b9d 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -285,35 +285,6 @@ static const u8 device_channel_config[] = {
 };
 
 /**
- * tsl2x7x_i2c_read() - Read a byte from a register.
- * @client:	i2c client
- * @reg:	device register to read from
- * @*val:	pointer to location to store register contents.
- *
- */
-static int
-tsl2x7x_i2c_read(struct i2c_client *client, u8 reg, u8 *val)
-{
-	int ret;
-
-	/* select register to write */
-	ret = i2c_smbus_write_byte(client, (TSL2X7X_CMD_REG | reg));
-	if (ret < 0) {
-		dev_err(&client->dev, "failed to write register %x\n", reg);
-		return ret;
-	}
-
-	/* read the data */
-	ret = i2c_smbus_read_byte(client);
-	if (ret >= 0)
-		*val = (u8)ret;
-	else
-		dev_err(&client->dev, "failed to read register %x\n", reg);
-
-	return ret;
-}
-
-/**
  * tsl2x7x_get_lux() - Reads and calculates current lux value.
  * @indio_dev:	pointer to IIO device
  *
@@ -352,15 +323,15 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 		goto out_unlock;
 	}
 
-	ret = tsl2x7x_i2c_read(chip->client,
-			       (TSL2X7X_CMD_REG | TSL2X7X_STATUS), &buf[0]);
+	ret = i2c_smbus_read_byte_data(chip->client,
+				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
 	if (ret < 0) {
 		dev_err(&chip->client->dev,
 			"%s: Failed to read STATUS Reg\n", __func__);
 		goto out_unlock;
 	}
 	/* is data new & valid */
-	if (!(buf[0] & TSL2X7X_STA_ADC_VALID)) {
+	if (!(ret & TSL2X7X_STA_ADC_VALID)) {
 		dev_err(&chip->client->dev,
 			"%s: data not valid yet\n", __func__);
 		ret = chip->als_cur_info.lux; /* return LAST VALUE */
@@ -368,14 +339,16 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	}
 
 	for (i = 0; i < 4; i++) {
-		ret = tsl2x7x_i2c_read(chip->client,
-				       (TSL2X7X_CMD_REG |
-				       (TSL2X7X_ALS_CHAN0LO + i)), &buf[i]);
+		int reg = TSL2X7X_CMD_REG | (TSL2X7X_ALS_CHAN0LO + i);
+
+		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		if (ret < 0) {
 			dev_err(&chip->client->dev,
 				"failed to read. err=%x\n", ret);
 			goto out_unlock;
 		}
+
+		buf[i] = ret;
 	}
 
 	/* clear any existing interrupt status */
@@ -475,7 +448,6 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 {
 	int i;
 	int ret;
-	u8 status;
 	u8 chdata[2];
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 
@@ -485,8 +457,8 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 		return -EBUSY;
 	}
 
-	ret = tsl2x7x_i2c_read(chip->client,
-			       (TSL2X7X_CMD_REG | TSL2X7X_STATUS), &status);
+	ret = i2c_smbus_read_byte_data(chip->client,
+			               TSL2X7X_CMD_REG | TSL2X7X_STATUS);
 	if (ret < 0) {
 		dev_err(&chip->client->dev, "i2c err=%d\n", ret);
 		goto prox_poll_err;
@@ -498,7 +470,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 	case tmd2671:
 	case tsl2771:
 	case tmd2771:
-		if (!(status & TSL2X7X_STA_ADC_VALID))
+		if (!(ret & TSL2X7X_STA_ADC_VALID))
 			goto prox_poll_err;
 	break;
 	case tsl2572:
@@ -506,17 +478,19 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 	case tmd2672:
 	case tsl2772:
 	case tmd2772:
-		if (!(status & TSL2X7X_STA_PRX_VALID))
+		if (!(ret & TSL2X7X_STA_PRX_VALID))
 			goto prox_poll_err;
 	break;
 	}
 
 	for (i = 0; i < 2; i++) {
-		ret = tsl2x7x_i2c_read(chip->client,
-				       (TSL2X7X_CMD_REG |
-				       (TSL2X7X_PRX_LO + i)), &chdata[i]);
+		int reg = TSL2X7X_CMD_REG | (TSL2X7X_PRX_LO + i);
+
+		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		if (ret < 0)
 			goto prox_poll_err;
+
+		chdata[i] = ret;
 	}
 
 	chip->prox_data =
@@ -1486,7 +1460,7 @@ static DEVICE_ATTR_RW(in_intensity0_thresh_period);
 static DEVICE_ATTR_RW(in_proximity0_thresh_period);
 
 /* Use the default register values to identify the Taos device */
-static int tsl2x7x_device_id(unsigned char *id, int target)
+static int tsl2x7x_device_id(int *id, int target)
 {
 	switch (target) {
 	case tsl2571:
@@ -1843,7 +1817,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 			 const struct i2c_device_id *id)
 {
 	int ret;
-	unsigned char device_id;
 	struct iio_dev *indio_dev;
 	struct tsl2X7X_chip *chip;
 
@@ -1855,13 +1828,13 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 	chip->client = clientp;
 	i2c_set_clientdata(clientp, indio_dev);
 
-	ret = tsl2x7x_i2c_read(chip->client,
-			       TSL2X7X_CHIPID, &device_id);
+	ret = i2c_smbus_read_byte_data(chip->client,
+				       TSL2X7X_CMD_REG | TSL2X7X_CHIPID);
 	if (ret < 0)
 		return ret;
 
-	if ((!tsl2x7x_device_id(&device_id, id->driver_data)) ||
-	    (tsl2x7x_device_id(&device_id, id->driver_data) == -EINVAL)) {
+	if ((!tsl2x7x_device_id(&ret, id->driver_data)) ||
+	    (tsl2x7x_device_id(&ret, id->driver_data) == -EINVAL)) {
 		dev_info(&chip->client->dev,
 			 "%s: i2c device found does not match expected id\n",
 				__func__);
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1678122 — Re: [PATCH 3/6] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()

On Thu, Jun 29, 2017 at 7:03 PM, Brian Masney <masneyb@onstation.org> wrote:
> tsl2x7x_i2c_read() would call i2c_smbus_write_byte() and
> i2c_smbus_read_byte(). These two i2c functions can be replaced with a
> single call to i2c_smbus_read_byte_data(). This patch removes the
> tsl2x7x_i2c_read() function and replaces all occurances with a call to
> i2c_smbus_read_byte_data().

s,occurances,occurrences,

Frans

[toc] | [prev] | [next] | [standalone]

#1678904

On Thu, Jun 29, 2017 at 01:03:46PM -0400, Brian Masney wrote:
> This begins my work to clean this driver up and eventually move it out
> of staging. Driver changes were tested using a TSL2771 hooked up to a
> Raspberry Pi 2.
> 
> Thanks to Jon Brenner at AMS/TAOS for loaning me some hardware samples
> to test my driver changes.
> 
> Brian Masney (6):
>   staging: iio: tsl2x7x: add of_match table for device tree support
>   staging: iio: tsl2x7x: remove redundant power_state sysfs attribute
>   staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()
>   staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()
>   staging: iio: tsl2x7x: refactor {read,write}_event_value to allow
>     handling multiple iio_event_infos
>   staging: iio: tsl2x7x: use usleep_range() instead of mdelay()
> 
>  .../devicetree/bindings/trivial-devices.txt        |  10 +
>  drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
>  2 files changed, 153 insertions(+), 173 deletions(-)

Hi Jonathon,

Hold off on applying this series. There are several other formatting
warnings from checkpatch with this series that I need to fix. Sorry
about the noise. I'd appreciate it if you could at least look at the
functionality of my changes and I'll resubmit next week.

I held back several other changes related to the event subsystem until
I'm able to properly test my changes. I'm having trouble getting the
interrupts to work. I wired the interrupt pin on the sensor to GPIO
pin 17 on my Raspberry Pi 2 and added the following section to
arch/arm/boot/dts/bcm2836-rpi-2-b.dts for my sensor:

&i2c1 {
        tsl2771@29 {
                compatible = "amstaos,tsl2771";
                reg = <0x39>;
                interrupt-parent = <&gpio>;
                interrupts = <17 2>;
        };
};

I start up iio_event_monitor, and run these commands:

echo 1 > events/in_intensity0_thresh_rising_en
echo 256 > events/in_intensity0_thresh_rising_value

When I shine a light on the ALS sensor, and the reading goes above
256, I do not get any events back from iio_event_monitor.

I'm honestly not sure about the 2 in the 'interrupts = <17 2>;' line.
I looked at how interrupts were setup in device tree overlays in the
official Raspberry Pi kernel and tried several variations from there.
I have more reading to do. :)

Thanks,

Brian

[toc] | [prev] | [next] | [standalone]

#1678969

Hi Brian,
Not sure about the DTS interrupt setting you mentioned but would like to know how you resolve - as I am planning on getting a Pi0w for some 'home personal projects'.

Anyway (and I am sure you probably already know this - but just in case) WRT the device registers (on the TSL2771 for example);
If you enable interrupts (ie. Reg#0x00= 0x37) and set the persistence register to 0x00 (is. Reg# 0x0C = 0x00), you should see interrupts occurring every integration period.
That way you can see if you're at least getting them.

You probably know this too - on earlier parts such as these, interrupts were cleared via use of the "command register" special function bits.  (see data sheets for details).
Now a day - interrupts are cleared in a more conventional manner 

Jon

-----Original Message-----
From: Brian Masney [mailto:masneyb@onstation.org] 
Sent: Friday, June 30, 2017 10:21 AM
To: jic23@kernel.org; linux-iio@vger.kernel.org
Cc: devel@driverdev.osuosl.org; lars@metafoo.de; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; Jon Brenner <Jon.Brenner@ams.com>; pmeerw@pmeerw.net; knaack.h@gmx.de
Subject: Re: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

On Thu, Jun 29, 2017 at 01:03:46PM -0400, Brian Masney wrote:
> This begins my work to clean this driver up and eventually move it out 
> of staging. Driver changes were tested using a TSL2771 hooked up to a 
> Raspberry Pi 2.
> 
> Thanks to Jon Brenner at AMS/TAOS for loaning me some hardware samples 
> to test my driver changes.
> 
> Brian Masney (6):
>   staging: iio: tsl2x7x: add of_match table for device tree support
>   staging: iio: tsl2x7x: remove redundant power_state sysfs attribute
>   staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()
>   staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()
>   staging: iio: tsl2x7x: refactor {read,write}_event_value to allow
>     handling multiple iio_event_infos
>   staging: iio: tsl2x7x: use usleep_range() instead of mdelay()
> 
>  .../devicetree/bindings/trivial-devices.txt        |  10 +
>  drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
>  2 files changed, 153 insertions(+), 173 deletions(-)

Hi Jonathon,

Hold off on applying this series. There are several other formatting warnings from checkpatch with this series that I need to fix. Sorry about the noise. I'd appreciate it if you could at least look at the functionality of my changes and I'll resubmit next week.

I held back several other changes related to the event subsystem until I'm able to properly test my changes. I'm having trouble getting the interrupts to work. I wired the interrupt pin on the sensor to GPIO pin 17 on my Raspberry Pi 2 and added the following section to arch/arm/boot/dts/bcm2836-rpi-2-b.dts for my sensor:

&i2c1 {
        tsl2771@29 {
                compatible = "amstaos,tsl2771";
                reg = <0x39>;
                interrupt-parent = <&gpio>;
                interrupts = <17 2>;
        };
};

I start up iio_event_monitor, and run these commands:

echo 1 > events/in_intensity0_thresh_rising_en
echo 256 > events/in_intensity0_thresh_rising_value

When I shine a light on the ALS sensor, and the reading goes above 256, I do not get any events back from iio_event_monitor.

I'm honestly not sure about the 2 in the 'interrupts = <17 2>;' line.
I looked at how interrupts were setup in device tree overlays in the official Raspberry Pi kernel and tried several variations from there.
I have more reading to do. :)

Thanks,

Brian

[toc] | [prev] | [next] | [standalone]

#1679320

On Fri, 30 Jun 2017 11:21:14 -0400
Brian Masney <masneyb@onstation.org> wrote:

> On Thu, Jun 29, 2017 at 01:03:46PM -0400, Brian Masney wrote:
> > This begins my work to clean this driver up and eventually move it out
> > of staging. Driver changes were tested using a TSL2771 hooked up to a
> > Raspberry Pi 2.
> > 
> > Thanks to Jon Brenner at AMS/TAOS for loaning me some hardware samples
> > to test my driver changes.
> > 
> > Brian Masney (6):
> >   staging: iio: tsl2x7x: add of_match table for device tree support
> >   staging: iio: tsl2x7x: remove redundant power_state sysfs attribute
> >   staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()
> >   staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()
> >   staging: iio: tsl2x7x: refactor {read,write}_event_value to allow
> >     handling multiple iio_event_infos
> >   staging: iio: tsl2x7x: use usleep_range() instead of mdelay()
> > 
> >  .../devicetree/bindings/trivial-devices.txt        |  10 +
> >  drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
> >  2 files changed, 153 insertions(+), 173 deletions(-)  
> 
> Hi Jonathon,
> 
> Hold off on applying this series.
Will do.
> There are several other formatting
> warnings from checkpatch with this series that I need to fix. Sorry
> about the noise. I'd appreciate it if you could at least look at the
> functionality of my changes and I'll resubmit next week.
All fine except possibly needing a stronger justification for patch 5.
I'm not against the patch - and it may be fine on the basis that
it lines up better with the standard form in other drivers, but
right now there isn't anything actually 'wrong' with the existing code.

J
> 
> I held back several other changes related to the event subsystem until
> I'm able to properly test my changes. I'm having trouble getting the
> interrupts to work. I wired the interrupt pin on the sensor to GPIO
> pin 17 on my Raspberry Pi 2 and added the following section to
> arch/arm/boot/dts/bcm2836-rpi-2-b.dts for my sensor:
> 
> &i2c1 {
>         tsl2771@29 {
>                 compatible = "amstaos,tsl2771";
>                 reg = <0x39>;
>                 interrupt-parent = <&gpio>;
>                 interrupts = <17 2>;
>         };
> };
> 
> I start up iio_event_monitor, and run these commands:
> 
> echo 1 > events/in_intensity0_thresh_rising_en
> echo 256 > events/in_intensity0_thresh_rising_value
> 
> When I shine a light on the ALS sensor, and the reading goes above
> 256, I do not get any events back from iio_event_monitor.
> 
> I'm honestly not sure about the 2 in the 'interrupts = <17 2>;' line.
> I looked at how interrupts were setup in device tree overlays in the
> official Raspberry Pi kernel and tried several variations from there.
> I have more reading to do. :)
> 
> Thanks,
> 
> Brian

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web