* [PATCH 1/2] dt-bindings: iio: proximity: vl53l0x: Add IRQ support
@ 2020-09-10 8:48 Ivan Drobyshevskyi
2020-09-10 8:48 ` [PATCH 2/2] " Ivan Drobyshevskyi
0 siblings, 1 reply; 6+ messages in thread
From: Ivan Drobyshevskyi @ 2020-09-10 8:48 UTC (permalink / raw)
To: linux-iio
Cc: songqiang1304521, jic23, knaack.h, lars, pmeerw, Ivan Drobyshevskyi
Since IRQ support was added to the driver, update bindings accordingly.
Signed-off-by: Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
---
Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
index aac5f621f..dfe00eb96 100644
--- a/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
+++ b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
@@ -4,9 +4,15 @@ Required properties:
- compatible: must be "st,vl53l0x"
- reg: i2c address where to find the device
+Optional properties:
+ - interrupts: Interrupt for notifying that new measurement is ready.
+ If no interrupt is specified, polling is used.
+
Example:
vl53l0x@29 {
compatible = "st,vl53l0x";
reg = <0x29>;
+ interrupt-parent = <&gpio>;
+ interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
};
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iio: proximity: vl53l0x: Add IRQ support
2020-09-10 8:48 [PATCH 1/2] dt-bindings: iio: proximity: vl53l0x: Add IRQ support Ivan Drobyshevskyi
@ 2020-09-10 8:48 ` Ivan Drobyshevskyi
2020-09-13 10:33 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Ivan Drobyshevskyi @ 2020-09-10 8:48 UTC (permalink / raw)
To: linux-iio
Cc: songqiang1304521, jic23, knaack.h, lars, pmeerw, Ivan Drobyshevskyi
VL53L0X can be configured to use interrupt pin (GPIO1)
to notify host about readiness of new measurement.
If interrupt pin is not specified in DT, driver still uses polling.
Signed-off-by: Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
---
drivers/iio/proximity/vl53l0x-i2c.c | 104 ++++++++++++++++++++++++----
1 file changed, 92 insertions(+), 12 deletions(-)
diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
index b48216cc1..b676e3702 100644
--- a/drivers/iio/proximity/vl53l0x-i2c.c
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -4,19 +4,21 @@
*
* Copyright (C) 2016 STMicroelectronics Imaging Division.
* Copyright (C) 2018 Song Qiang <songqiang1304521@gmail.com>
+ * Copyright (C) 2020 Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
*
* Datasheet available at
* <https://www.st.com/resource/en/datasheet/vl53l0x.pdf>
*
* Default 7-bit i2c slave address 0x29.
*
- * TODO: FIFO buffer, continuous mode, interrupts, range selection,
- * sensor ID check.
+ * TODO: FIFO buffer, continuous mode, range selection, sensor ID check.
*/
#include <linux/delay.h>
#include <linux/i2c.h>
+#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/of_irq.h>
#include <linux/iio/iio.h>
@@ -29,14 +31,67 @@
#define VL_REG_SYSRANGE_MODE_TIMED BIT(2)
#define VL_REG_SYSRANGE_MODE_HISTOGRAM BIT(3)
+#define VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO 0x0A
+#define VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY BIT(2)
+
+#define VL_REG_SYSTEM_INTERRUPT_CLEAR 0x0B
+
#define VL_REG_RESULT_INT_STATUS 0x13
#define VL_REG_RESULT_RANGE_STATUS 0x14
#define VL_REG_RESULT_RANGE_STATUS_COMPLETE BIT(0)
struct vl53l0x_data {
- struct i2c_client *client;
+ struct i2c_client *client;
+ struct completion completion;
+ int irq;
};
+static irqreturn_t vl53l0x_handle_irq(int irq, void *dev_id)
+{
+ struct iio_dev *indio_dev = dev_id;
+ struct vl53l0x_data *data = iio_priv(indio_dev);
+
+ complete(&data->completion);
+
+ return IRQ_HANDLED;
+}
+
+static int vl53l0x_configure_irq(struct device *dev, struct iio_dev *indio_dev)
+{
+ struct vl53l0x_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = devm_request_irq(dev, data->irq, vl53l0x_handle_irq,
+ IRQF_TRIGGER_FALLING, indio_dev->name, indio_dev);
+ if (ret) {
+ dev_err(dev, "devm_request_irq error: %d\n", ret);
+ return ret;
+ }
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO,
+ VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY);
+ if (ret)
+ dev_err(dev, "failed to configure IRQ: %d\n", ret);
+
+ return ret;
+}
+
+static void vl53l0x_clear_irq(struct vl53l0x_data *data)
+{
+ u8 status;
+
+ i2c_smbus_write_byte_data(data->client,
+ VL_REG_SYSTEM_INTERRUPT_CLEAR, 1);
+ i2c_smbus_write_byte_data(data->client,
+ VL_REG_SYSTEM_INTERRUPT_CLEAR, 0);
+
+ status = i2c_smbus_read_byte_data(data->client,
+ VL_REG_RESULT_INT_STATUS);
+ if (status & 0x07)
+ dev_err(&data->client->dev, "failed to clear irq\n");
+}
+
static int vl53l0x_read_proximity(struct vl53l0x_data *data,
const struct iio_chan_spec *chan,
int *val)
@@ -50,19 +105,31 @@ static int vl53l0x_read_proximity(struct vl53l0x_data *data,
if (ret < 0)
return ret;
- do {
- ret = i2c_smbus_read_byte_data(client,
- VL_REG_RESULT_RANGE_STATUS);
+ if (data->irq) {
+ reinit_completion(&data->completion);
+
+ ret = wait_for_completion_timeout(&data->completion, HZ/10);
if (ret < 0)
return ret;
+ else if (ret == 0)
+ return -ETIMEDOUT;
- if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
- break;
+ vl53l0x_clear_irq(data);
+ } else {
+ do {
+ ret = i2c_smbus_read_byte_data(client,
+ VL_REG_RESULT_RANGE_STATUS);
+ if (ret < 0)
+ return ret;
+
+ if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
+ break;
- usleep_range(1000, 5000);
- } while (--tries);
- if (!tries)
- return -ETIMEDOUT;
+ usleep_range(1000, 5000);
+ } while (--tries);
+ if (!tries)
+ return -ETIMEDOUT;
+ }
ret = i2c_smbus_read_i2c_block_data(client, VL_REG_RESULT_RANGE_STATUS,
12, buffer);
@@ -120,6 +187,7 @@ static int vl53l0x_probe(struct i2c_client *client)
{
struct vl53l0x_data *data;
struct iio_dev *indio_dev;
+ struct device *dev = &client->dev;
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
if (!indio_dev)
@@ -141,6 +209,18 @@ static int vl53l0x_probe(struct i2c_client *client)
indio_dev->num_channels = ARRAY_SIZE(vl53l0x_channels);
indio_dev->modes = INDIO_DIRECT_MODE;
+ data->irq = irq_of_parse_and_map(dev->of_node, 0);
+ /* usage of interrupt is optional */
+ if (data->irq) {
+ int ret;
+
+ init_completion(&data->completion);
+
+ ret = vl53l0x_configure_irq(dev, indio_dev);
+ if (ret)
+ return ret;
+ }
+
return devm_iio_device_register(&client->dev, indio_dev);
}
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iio: proximity: vl53l0x: Add IRQ support
2020-09-10 8:48 ` [PATCH 2/2] " Ivan Drobyshevskyi
@ 2020-09-13 10:33 ` Jonathan Cameron
2020-09-16 7:44 ` [PATCH v2 1/2] dt-bindings: " Ivan Drobyshevskyi
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2020-09-13 10:33 UTC (permalink / raw)
To: Ivan Drobyshevskyi; +Cc: linux-iio, songqiang1304521, knaack.h, lars, pmeerw
On Thu, 10 Sep 2020 11:48:17 +0300
Ivan Drobyshevskyi <drobyshevskyi@gmail.com> wrote:
> VL53L0X can be configured to use interrupt pin (GPIO1)
> to notify host about readiness of new measurement.
>
> If interrupt pin is not specified in DT, driver still uses polling.
If interrupt pin is not specified, driver still uses polling.
(see below for why I suggest that change!)
Otherwise, a few minor things inline to tidy up.
Thanks,
Jonathan
>
> Signed-off-by: Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
> ---
> drivers/iio/proximity/vl53l0x-i2c.c | 104 ++++++++++++++++++++++++----
> 1 file changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index b48216cc1..b676e3702 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -4,19 +4,21 @@
> *
> * Copyright (C) 2016 STMicroelectronics Imaging Division.
> * Copyright (C) 2018 Song Qiang <songqiang1304521@gmail.com>
> + * Copyright (C) 2020 Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
> *
> * Datasheet available at
> * <https://www.st.com/resource/en/datasheet/vl53l0x.pdf>
> *
> * Default 7-bit i2c slave address 0x29.
> *
> - * TODO: FIFO buffer, continuous mode, interrupts, range selection,
> - * sensor ID check.
> + * TODO: FIFO buffer, continuous mode, range selection, sensor ID check.
> */
>
> #include <linux/delay.h>
> #include <linux/i2c.h>
> +#include <linux/interrupt.h>
> #include <linux/module.h>
> +#include <linux/of_irq.h>
As below, you shouldn't need this.
>
> #include <linux/iio/iio.h>
>
> @@ -29,14 +31,67 @@
> #define VL_REG_SYSRANGE_MODE_TIMED BIT(2)
> #define VL_REG_SYSRANGE_MODE_HISTOGRAM BIT(3)
>
> +#define VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO 0x0A
> +#define VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY BIT(2)
> +
> +#define VL_REG_SYSTEM_INTERRUPT_CLEAR 0x0B
> +
> #define VL_REG_RESULT_INT_STATUS 0x13
> #define VL_REG_RESULT_RANGE_STATUS 0x14
> #define VL_REG_RESULT_RANGE_STATUS_COMPLETE BIT(0)
>
> struct vl53l0x_data {
> - struct i2c_client *client;
Given existing style is perfectly readable, I would follow it and not
add the 'pretty alignment' change you have here to the existing element
or the new ones. It often goes wrong and generates very noisy patches
anyway!
> + struct i2c_client *client;
> + struct completion completion;
> + int irq;
> };
>
> +static irqreturn_t vl53l0x_handle_irq(int irq, void *dev_id)
dev_id is an odd name for that parameter. In what way is it
an identification? Stick to private or similar to avoid
implications you don't intend.
> +{
> + struct iio_dev *indio_dev = dev_id;
> + struct vl53l0x_data *data = iio_priv(indio_dev);
> +
> + complete(&data->completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int vl53l0x_configure_irq(struct device *dev, struct iio_dev *indio_dev)
> +{
> + struct vl53l0x_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = devm_request_irq(dev, data->irq, vl53l0x_handle_irq,
> + IRQF_TRIGGER_FALLING, indio_dev->name, indio_dev);
> + if (ret) {
> + dev_err(dev, "devm_request_irq error: %d\n", ret);
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO,
> + VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY);
> + if (ret)
> + dev_err(dev, "failed to configure IRQ: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void vl53l0x_clear_irq(struct vl53l0x_data *data)
> +{
> + u8 status;
> +
> + i2c_smbus_write_byte_data(data->client,
> + VL_REG_SYSTEM_INTERRUPT_CLEAR, 1);
Even though we can't report the error via return value, it is useful to
check it and print an error if we get one.
> + i2c_smbus_write_byte_data(data->client,
> + VL_REG_SYSTEM_INTERRUPT_CLEAR, 0);
> +
> + status = i2c_smbus_read_byte_data(data->client,
> + VL_REG_RESULT_INT_STATUS);
> + if (status & 0x07)
If we get an error (which IIRC is signified by < 0) then this will be
doing something very strange. As such, we should check that first before
using status.
> + dev_err(&data->client->dev, "failed to clear irq\n");
> +}
> +
> static int vl53l0x_read_proximity(struct vl53l0x_data *data,
> const struct iio_chan_spec *chan,
> int *val)
> @@ -50,19 +105,31 @@ static int vl53l0x_read_proximity(struct vl53l0x_data *data,
> if (ret < 0)
> return ret;
>
> - do {
> - ret = i2c_smbus_read_byte_data(client,
> - VL_REG_RESULT_RANGE_STATUS);
> + if (data->irq) {
> + reinit_completion(&data->completion);
> +
> + ret = wait_for_completion_timeout(&data->completion, HZ/10);
> if (ret < 0)
> return ret;
> + else if (ret == 0)
> + return -ETIMEDOUT;
>
> - if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
> - break;
> + vl53l0x_clear_irq(data);
> + } else {
> + do {
> + ret = i2c_smbus_read_byte_data(client,
> + VL_REG_RESULT_RANGE_STATUS);
> + if (ret < 0)
> + return ret;
> +
> + if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
> + break;
>
> - usleep_range(1000, 5000);
> - } while (--tries);
> - if (!tries)
> - return -ETIMEDOUT;
> + usleep_range(1000, 5000);
> + } while (--tries);
> + if (!tries)
> + return -ETIMEDOUT;
> + }
>
> ret = i2c_smbus_read_i2c_block_data(client, VL_REG_RESULT_RANGE_STATUS,
> 12, buffer);
> @@ -120,6 +187,7 @@ static int vl53l0x_probe(struct i2c_client *client)
> {
> struct vl53l0x_data *data;
> struct iio_dev *indio_dev;
> + struct device *dev = &client->dev;
That's a valid change to make, but if you want to do this it should be
as a precursor patch tidying up all the places client->dev is used
in the probe function.
>
> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> if (!indio_dev)
> @@ -141,6 +209,18 @@ static int vl53l0x_probe(struct i2c_client *client)
> indio_dev->num_channels = ARRAY_SIZE(vl53l0x_channels);
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> + data->irq = irq_of_parse_and_map(dev->of_node, 0);
I would rather we didn't introduce any of specific code into this driver even
if that is the mostly likely route by which it will be instantiated.
Currently the driver can be instantiated from ACPI with PRP0001 based bindings
adding this breaks that (I think).
As it's an i2c device, the i2c core should already have set
client->irq to the appropriate value so use that.
> + /* usage of interrupt is optional */
> + if (data->irq) {
> + int ret;
> +
> + init_completion(&data->completion);
> +
> + ret = vl53l0x_configure_irq(dev, indio_dev);
> + if (ret)
> + return ret;
> + }
> +
> return devm_iio_device_register(&client->dev, indio_dev);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] dt-bindings: iio: proximity: vl53l0x: Add IRQ support
2020-09-13 10:33 ` Jonathan Cameron
@ 2020-09-16 7:44 ` Ivan Drobyshevskyi
2020-09-16 7:44 ` [PATCH v2 2/2] " Ivan Drobyshevskyi
0 siblings, 1 reply; 6+ messages in thread
From: Ivan Drobyshevskyi @ 2020-09-16 7:44 UTC (permalink / raw)
To: linux-iio, jic23
Cc: songqiang1304521, knaack.h, lars, pmeerw, Ivan Drobyshevskyi
Since IRQ support was added to the driver, update bindings accordingly.
Signed-off-by: Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
---
(no changes since v1)
Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
index aac5f621f..dfe00eb96 100644
--- a/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
+++ b/Documentation/devicetree/bindings/iio/proximity/vl53l0x.txt
@@ -4,9 +4,15 @@ Required properties:
- compatible: must be "st,vl53l0x"
- reg: i2c address where to find the device
+Optional properties:
+ - interrupts: Interrupt for notifying that new measurement is ready.
+ If no interrupt is specified, polling is used.
+
Example:
vl53l0x@29 {
compatible = "st,vl53l0x";
reg = <0x29>;
+ interrupt-parent = <&gpio>;
+ interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
};
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] iio: proximity: vl53l0x: Add IRQ support
2020-09-16 7:44 ` [PATCH v2 1/2] dt-bindings: " Ivan Drobyshevskyi
@ 2020-09-16 7:44 ` Ivan Drobyshevskyi
2020-09-17 18:21 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Ivan Drobyshevskyi @ 2020-09-16 7:44 UTC (permalink / raw)
To: linux-iio, jic23
Cc: songqiang1304521, knaack.h, lars, pmeerw, Ivan Drobyshevskyi
VL53L0X can be configured to use interrupt pin (GPIO1)
to notify host about readiness of new measurement.
If interrupt pin is not specified, driver still uses polling.
Signed-off-by: Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
---
changes since v1:
- remove explicit DT parsing for IRQ number, reuse i2c_client's irq
- add checking of i2c_smbus_write_byte_data return value
- fix checking of i2c_smbus_read_byte_data return value
- other styling/cosmetic changes as suggested by Jonathan
drivers/iio/proximity/vl53l0x-i2c.c | 104 +++++++++++++++++++++++++---
1 file changed, 93 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
index b48216cc1..e92a0bf79 100644
--- a/drivers/iio/proximity/vl53l0x-i2c.c
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -4,18 +4,19 @@
*
* Copyright (C) 2016 STMicroelectronics Imaging Division.
* Copyright (C) 2018 Song Qiang <songqiang1304521@gmail.com>
+ * Copyright (C) 2020 Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
*
* Datasheet available at
* <https://www.st.com/resource/en/datasheet/vl53l0x.pdf>
*
* Default 7-bit i2c slave address 0x29.
*
- * TODO: FIFO buffer, continuous mode, interrupts, range selection,
- * sensor ID check.
+ * TODO: FIFO buffer, continuous mode, range selection, sensor ID check.
*/
#include <linux/delay.h>
#include <linux/i2c.h>
+#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/iio/iio.h>
@@ -29,14 +30,72 @@
#define VL_REG_SYSRANGE_MODE_TIMED BIT(2)
#define VL_REG_SYSRANGE_MODE_HISTOGRAM BIT(3)
+#define VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO 0x0A
+#define VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY BIT(2)
+
+#define VL_REG_SYSTEM_INTERRUPT_CLEAR 0x0B
+
#define VL_REG_RESULT_INT_STATUS 0x13
#define VL_REG_RESULT_RANGE_STATUS 0x14
#define VL_REG_RESULT_RANGE_STATUS_COMPLETE BIT(0)
struct vl53l0x_data {
struct i2c_client *client;
+ struct completion completion;
};
+static irqreturn_t vl53l0x_handle_irq(int irq, void *priv)
+{
+ struct iio_dev *indio_dev = priv;
+ struct vl53l0x_data *data = iio_priv(indio_dev);
+
+ complete(&data->completion);
+
+ return IRQ_HANDLED;
+}
+
+static int vl53l0x_configure_irq(struct i2c_client *client,
+ struct iio_dev *indio_dev)
+{
+ struct vl53l0x_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = devm_request_irq(&client->dev, client->irq, vl53l0x_handle_irq,
+ IRQF_TRIGGER_FALLING, indio_dev->name, indio_dev);
+ if (ret) {
+ dev_err(&client->dev, "devm_request_irq error: %d\n", ret);
+ return ret;
+ }
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO,
+ VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY);
+ if (ret < 0)
+ dev_err(&client->dev, "failed to configure IRQ: %d\n", ret);
+
+ return ret;
+}
+
+static void vl53l0x_clear_irq(struct vl53l0x_data *data)
+{
+ struct device *dev = &data->client->dev;
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ VL_REG_SYSTEM_INTERRUPT_CLEAR, 1);
+ if (ret < 0)
+ dev_err(dev, "failed to clear error irq: %d\n", ret);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ VL_REG_SYSTEM_INTERRUPT_CLEAR, 0);
+ if (ret < 0)
+ dev_err(dev, "failed to clear range irq: %d\n", ret);
+
+ ret = i2c_smbus_read_byte_data(data->client, VL_REG_RESULT_INT_STATUS);
+ if (ret < 0 || ret & 0x07)
+ dev_err(dev, "failed to clear irq: %d\n", ret);
+}
+
static int vl53l0x_read_proximity(struct vl53l0x_data *data,
const struct iio_chan_spec *chan,
int *val)
@@ -50,19 +109,31 @@ static int vl53l0x_read_proximity(struct vl53l0x_data *data,
if (ret < 0)
return ret;
- do {
- ret = i2c_smbus_read_byte_data(client,
- VL_REG_RESULT_RANGE_STATUS);
+ if (data->client->irq) {
+ reinit_completion(&data->completion);
+
+ ret = wait_for_completion_timeout(&data->completion, HZ/10);
if (ret < 0)
return ret;
+ else if (ret == 0)
+ return -ETIMEDOUT;
- if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
- break;
+ vl53l0x_clear_irq(data);
+ } else {
+ do {
+ ret = i2c_smbus_read_byte_data(client,
+ VL_REG_RESULT_RANGE_STATUS);
+ if (ret < 0)
+ return ret;
- usleep_range(1000, 5000);
- } while (--tries);
- if (!tries)
- return -ETIMEDOUT;
+ if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
+ break;
+
+ usleep_range(1000, 5000);
+ } while (--tries);
+ if (!tries)
+ return -ETIMEDOUT;
+ }
ret = i2c_smbus_read_i2c_block_data(client, VL_REG_RESULT_RANGE_STATUS,
12, buffer);
@@ -141,6 +212,17 @@ static int vl53l0x_probe(struct i2c_client *client)
indio_dev->num_channels = ARRAY_SIZE(vl53l0x_channels);
indio_dev->modes = INDIO_DIRECT_MODE;
+ /* usage of interrupt is optional */
+ if (client->irq) {
+ int ret;
+
+ init_completion(&data->completion);
+
+ ret = vl53l0x_configure_irq(client, indio_dev);
+ if (ret)
+ return ret;
+ }
+
return devm_iio_device_register(&client->dev, indio_dev);
}
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: proximity: vl53l0x: Add IRQ support
2020-09-16 7:44 ` [PATCH v2 2/2] " Ivan Drobyshevskyi
@ 2020-09-17 18:21 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2020-09-17 18:21 UTC (permalink / raw)
To: Ivan Drobyshevskyi; +Cc: linux-iio, songqiang1304521, knaack.h, lars, pmeerw
On Wed, 16 Sep 2020 10:44:58 +0300
Ivan Drobyshevskyi <drobyshevskyi@gmail.com> wrote:
> VL53L0X can be configured to use interrupt pin (GPIO1)
> to notify host about readiness of new measurement.
>
> If interrupt pin is not specified, driver still uses polling.
>
> Signed-off-by: Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
For future reference, I'd prefer a new version as not threaded with
the previous one. When we get to many versions it makes things
completely unreadable.
Patches look good to me and the dt change is trivial enough I'll
pick it up without wasting DT reviewers time.
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to poke at it and see if we missed anything.
Thanks,
Jonathan
> ---
> changes since v1:
> - remove explicit DT parsing for IRQ number, reuse i2c_client's irq
> - add checking of i2c_smbus_write_byte_data return value
> - fix checking of i2c_smbus_read_byte_data return value
> - other styling/cosmetic changes as suggested by Jonathan
>
> drivers/iio/proximity/vl53l0x-i2c.c | 104 +++++++++++++++++++++++++---
> 1 file changed, 93 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index b48216cc1..e92a0bf79 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -4,18 +4,19 @@
> *
> * Copyright (C) 2016 STMicroelectronics Imaging Division.
> * Copyright (C) 2018 Song Qiang <songqiang1304521@gmail.com>
> + * Copyright (C) 2020 Ivan Drobyshevskyi <drobyshevskyi@gmail.com>
> *
> * Datasheet available at
> * <https://www.st.com/resource/en/datasheet/vl53l0x.pdf>
> *
> * Default 7-bit i2c slave address 0x29.
> *
> - * TODO: FIFO buffer, continuous mode, interrupts, range selection,
> - * sensor ID check.
> + * TODO: FIFO buffer, continuous mode, range selection, sensor ID check.
> */
>
> #include <linux/delay.h>
> #include <linux/i2c.h>
> +#include <linux/interrupt.h>
> #include <linux/module.h>
>
> #include <linux/iio/iio.h>
> @@ -29,14 +30,72 @@
> #define VL_REG_SYSRANGE_MODE_TIMED BIT(2)
> #define VL_REG_SYSRANGE_MODE_HISTOGRAM BIT(3)
>
> +#define VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO 0x0A
> +#define VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY BIT(2)
> +
> +#define VL_REG_SYSTEM_INTERRUPT_CLEAR 0x0B
> +
> #define VL_REG_RESULT_INT_STATUS 0x13
> #define VL_REG_RESULT_RANGE_STATUS 0x14
> #define VL_REG_RESULT_RANGE_STATUS_COMPLETE BIT(0)
>
> struct vl53l0x_data {
> struct i2c_client *client;
> + struct completion completion;
> };
>
> +static irqreturn_t vl53l0x_handle_irq(int irq, void *priv)
> +{
> + struct iio_dev *indio_dev = priv;
> + struct vl53l0x_data *data = iio_priv(indio_dev);
> +
> + complete(&data->completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int vl53l0x_configure_irq(struct i2c_client *client,
> + struct iio_dev *indio_dev)
> +{
> + struct vl53l0x_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = devm_request_irq(&client->dev, client->irq, vl53l0x_handle_irq,
> + IRQF_TRIGGER_FALLING, indio_dev->name, indio_dev);
> + if (ret) {
> + dev_err(&client->dev, "devm_request_irq error: %d\n", ret);
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + VL_REG_SYSTEM_INTERRUPT_CONFIG_GPIO,
> + VL_REG_SYSTEM_INTERRUPT_GPIO_NEW_SAMPLE_READY);
> + if (ret < 0)
> + dev_err(&client->dev, "failed to configure IRQ: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void vl53l0x_clear_irq(struct vl53l0x_data *data)
> +{
> + struct device *dev = &data->client->dev;
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + VL_REG_SYSTEM_INTERRUPT_CLEAR, 1);
> + if (ret < 0)
> + dev_err(dev, "failed to clear error irq: %d\n", ret);
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + VL_REG_SYSTEM_INTERRUPT_CLEAR, 0);
> + if (ret < 0)
> + dev_err(dev, "failed to clear range irq: %d\n", ret);
> +
> + ret = i2c_smbus_read_byte_data(data->client, VL_REG_RESULT_INT_STATUS);
> + if (ret < 0 || ret & 0x07)
> + dev_err(dev, "failed to clear irq: %d\n", ret);
> +}
> +
> static int vl53l0x_read_proximity(struct vl53l0x_data *data,
> const struct iio_chan_spec *chan,
> int *val)
> @@ -50,19 +109,31 @@ static int vl53l0x_read_proximity(struct vl53l0x_data *data,
> if (ret < 0)
> return ret;
>
> - do {
> - ret = i2c_smbus_read_byte_data(client,
> - VL_REG_RESULT_RANGE_STATUS);
> + if (data->client->irq) {
> + reinit_completion(&data->completion);
> +
> + ret = wait_for_completion_timeout(&data->completion, HZ/10);
> if (ret < 0)
> return ret;
> + else if (ret == 0)
> + return -ETIMEDOUT;
>
> - if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
> - break;
> + vl53l0x_clear_irq(data);
> + } else {
> + do {
> + ret = i2c_smbus_read_byte_data(client,
> + VL_REG_RESULT_RANGE_STATUS);
> + if (ret < 0)
> + return ret;
>
> - usleep_range(1000, 5000);
> - } while (--tries);
> - if (!tries)
> - return -ETIMEDOUT;
> + if (ret & VL_REG_RESULT_RANGE_STATUS_COMPLETE)
> + break;
> +
> + usleep_range(1000, 5000);
> + } while (--tries);
> + if (!tries)
> + return -ETIMEDOUT;
> + }
>
> ret = i2c_smbus_read_i2c_block_data(client, VL_REG_RESULT_RANGE_STATUS,
> 12, buffer);
> @@ -141,6 +212,17 @@ static int vl53l0x_probe(struct i2c_client *client)
> indio_dev->num_channels = ARRAY_SIZE(vl53l0x_channels);
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> + /* usage of interrupt is optional */
> + if (client->irq) {
> + int ret;
> +
> + init_completion(&data->completion);
> +
> + ret = vl53l0x_configure_irq(client, indio_dev);
> + if (ret)
> + return ret;
> + }
> +
> return devm_iio_device_register(&client->dev, indio_dev);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-17 18:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-10 8:48 [PATCH 1/2] dt-bindings: iio: proximity: vl53l0x: Add IRQ support Ivan Drobyshevskyi
2020-09-10 8:48 ` [PATCH 2/2] " Ivan Drobyshevskyi
2020-09-13 10:33 ` Jonathan Cameron
2020-09-16 7:44 ` [PATCH v2 1/2] dt-bindings: " Ivan Drobyshevskyi
2020-09-16 7:44 ` [PATCH v2 2/2] " Ivan Drobyshevskyi
2020-09-17 18:21 ` Jonathan Cameron
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).