2023-05-09 14:03:22

by Astrid Rost

Subject: [PATCH v2 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200

Add proximity interrupt support for vcnl4200 (similar to vcnl4040).
Add support to configure proximity sensor interrupts and threshold
limits. If an interrupt is detected an event will be pushed to the
event interface.

Signed-off-by: Astrid Rost <[email protected]>
---
drivers/iio/light/vcnl4000.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 56d3963d3d66..13568454baff 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -65,6 +65,7 @@
#define VCNL4200_PS_DATA 0x08 /* Proximity data */
#define VCNL4200_AL_DATA 0x09 /* Ambient light data */
#define VCNL4040_INT_FLAGS 0x0b /* Interrupt register */
+#define VCNL4200_INT_FLAGS 0x0d /* Interrupt register */
#define VCNL4200_DEV_ID 0x0e /* Device ID, slave address and version */

#define VCNL4040_DEV_ID 0x0c /* Device ID and version */
@@ -1004,8 +1005,14 @@ static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
struct iio_dev *indio_dev = p;
struct vcnl4000_data *data = iio_priv(indio_dev);
int ret;
+ int reg;

- ret = i2c_smbus_read_word_data(data->client, VCNL4040_INT_FLAGS);
+ if (data->id == VCNL4200)
+ reg = VCNL4200_INT_FLAGS;
+ else
+ reg = VCNL4040_INT_FLAGS;
+
+ ret = i2c_smbus_read_word_data(data->client, reg);
if (ret < 0)
return IRQ_HANDLED;

@@ -1321,9 +1328,10 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
.measure_light = vcnl4200_measure_light,
.measure_proximity = vcnl4200_measure_proximity,
.set_power_state = vcnl4200_set_power_state,
- .channels = vcnl4000_channels,
+ .channels = vcnl4040_channels,
.num_channels = ARRAY_SIZE(vcnl4000_channels),
- .info = &vcnl4000_info,
+ .info = &vcnl4040_info,
+ .irq_thread = vcnl4040_irq_thread,
},
};

--
2.30.2

2023-05-13 19:16:33

by Jonathan Cameron

Subject: Re: [PATCH v2 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200

On Tue, 9 May 2023 16:01:47 +0200
Astrid Rost <[email protected]> wrote:

> Add proximity interrupt support for vcnl4200 (similar to vcnl4040).
> Add support to configure proximity sensor interrupts and threshold
> limits. If an interrupt is detected an event will be pushed to the
> event interface.
>
> Signed-off-by: Astrid Rost <[email protected]>
Where possible (and I think it is here) please avoid putting data as code.
That is, move the necessary register address into the
*_chip_spec structure. That way, if another version of the chip is supported
in future we can simply add the value to the right instance of that structure.
That tends to end up more flexible than if / else / switch statements.

Thanks,

Jonathan

> ---
> drivers/iio/light/vcnl4000.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 56d3963d3d66..13568454baff 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -65,6 +65,7 @@
> #define VCNL4200_PS_DATA 0x08 /* Proximity data */
> #define VCNL4200_AL_DATA 0x09 /* Ambient light data */
> #define VCNL4040_INT_FLAGS 0x0b /* Interrupt register */
> +#define VCNL4200_INT_FLAGS 0x0d /* Interrupt register */
> #define VCNL4200_DEV_ID 0x0e /* Device ID, slave address and version */
>
> #define VCNL4040_DEV_ID 0x0c /* Device ID and version */
> @@ -1004,8 +1005,14 @@ static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
> struct iio_dev *indio_dev = p;
> struct vcnl4000_data *data = iio_priv(indio_dev);
> int ret;
> + int reg;
>
> - ret = i2c_smbus_read_word_data(data->client, VCNL4040_INT_FLAGS);
> + if (data->id == VCNL4200)
> + reg = VCNL4200_INT_FLAGS;
> + else
> + reg = VCNL4040_INT_FLAGS;

Prefer this as
data->chip_spec->int_reg

> +
> + ret = i2c_smbus_read_word_data(data->client, reg);
> if (ret < 0)
> return IRQ_HANDLED;
>
> @@ -1321,9 +1328,10 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
> .measure_light = vcnl4200_measure_light,
> .measure_proximity = vcnl4200_measure_proximity,
> .set_power_state = vcnl4200_set_power_state,
> - .channels = vcnl4000_channels,
> + .channels = vcnl4040_channels,
> .num_channels = ARRAY_SIZE(vcnl4000_channels),
> - .info = &vcnl4000_info,
> + .info = &vcnl4040_info,
> + .irq_thread = vcnl4040_irq_thread,
> },
> };
>