diff mbox series

[v4,4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200

Message ID 20230522142621.1680563-5-astrid.rost@axis.com (mailing list archive)
State Changes Requested
Headers show
Series iio: light: vcnl4000: Add features for vncl4040/4200 | expand

Commit Message

Astrid Rost May 22, 2023, 2:26 p.m. UTC
Add support to configure ambient light sensor interrupts and threshold
limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
will be pushed to the event interface.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 197 ++++++++++++++++++++++++++---------
 1 file changed, 146 insertions(+), 51 deletions(-)

Comments

Andy Shevchenko May 28, 2023, 10:45 p.m. UTC | #1
Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:
> Add support to configure ambient light sensor interrupts and threshold
> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
> will be pushed to the event interface.

...

> +		case IIO_EV_DIR_RISING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_ALS_THDH_LM, val);

Strange indentation.

> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_ALS_THDL_LM, val);

Same.

> +			break;

...

> +	case IIO_PROXIMITY:
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_PS_THDH_LM, val);

Same.

> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_PS_THDL_LM, val);

Same.

> +			break;
Astrid Rost May 29, 2023, 7:41 a.m. UTC | #2
Hello Andy,

Thanks for reviewing.
I can change this. But this is how it gets formatted by .clang-format.

Astrid

On 5/29/23 00:45, andy.shevchenko@gmail.com wrote:
> Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:
>> Add support to configure ambient light sensor interrupts and threshold
>> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
>> will be pushed to the event interface.
> 
> ...
> 
>> +		case IIO_EV_DIR_RISING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_ALS_THDH_LM, val);
> 
> Strange indentation.
> 
>> +			break;
>> +		case IIO_EV_DIR_FALLING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_ALS_THDL_LM, val);
> 
> Same.
> 
>> +			break;
> 
> ...
> 
>> +	case IIO_PROXIMITY:
>> +		switch (dir) {
>> +		case IIO_EV_DIR_RISING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_PS_THDH_LM, val);
> 
> Same.
> 
>> +			break;
>> +		case IIO_EV_DIR_FALLING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_PS_THDL_LM, val);
> 
> Same.
> 
>> +			break;
>
Andy Shevchenko May 29, 2023, 9:02 a.m. UTC | #3
On Mon, May 29, 2023 at 10:41 AM Astrid Rost <astridr@axis.com> wrote:
> Thanks for reviewing.
> I can change this. But this is how it gets formatted by .clang-format.

I would suggest to report the bug (in case it's not configurable) or
configure to avoid such a misindentation.

> On 5/29/23 00:45, andy.shevchenko@gmail.com wrote:
> > Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:
> >> Add support to configure ambient light sensor interrupts and threshold
> >> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
> >> will be pushed to the event interface.
> >
> > ...
> >
> >> +            case IIO_EV_DIR_RISING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_ALS_THDH_LM, val);
> >
> > Strange indentation.
> >
> >> +                    break;
> >> +            case IIO_EV_DIR_FALLING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_ALS_THDL_LM, val);
> >
> > Same.
> >
> >> +                    break;
> >
> > ...
> >
> >> +    case IIO_PROXIMITY:
> >> +            switch (dir) {
> >> +            case IIO_EV_DIR_RISING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_PS_THDH_LM, val);
> >
> > Same.
> >
> >> +                    break;
> >> +            case IIO_EV_DIR_FALLING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_PS_THDL_LM, val);
> >
> > Same.
Jonathan Cameron June 4, 2023, 1:10 p.m. UTC | #4
On Mon, 29 May 2023 12:02:16 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, May 29, 2023 at 10:41 AM Astrid Rost <astridr@axis.com> wrote:
> > Thanks for reviewing.
> > I can change this. But this is how it gets formatted by .clang-format.  
> 
> I would suggest to report the bug (in case it's not configurable) or
> configure to avoid such a misindentation.

I assume it's trading off line length against aligning with opening brackets.
Maybe made the wrong decision, but it is not totally unreasonable to my eyes.
I don't care either way though, so go with whatever you have for next version!

Jonathan

> 
> > On 5/29/23 00:45, andy.shevchenko@gmail.com wrote:  
> > > Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:  
> > >> Add support to configure ambient light sensor interrupts and threshold
> > >> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
> > >> will be pushed to the event interface.  
> > >
> > > ...
> > >  
> > >> +            case IIO_EV_DIR_RISING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_ALS_THDH_LM, val);  
> > >
> > > Strange indentation.
> > >  
> > >> +                    break;
> > >> +            case IIO_EV_DIR_FALLING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_ALS_THDL_LM, val);  
> > >
> > > Same.
> > >  
> > >> +                    break;  
> > >
> > > ...
> > >  
> > >> +    case IIO_PROXIMITY:
> > >> +            switch (dir) {
> > >> +            case IIO_EV_DIR_RISING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_PS_THDH_LM, val);  
> > >
> > > Same.
> > >  
> > >> +                    break;
> > >> +            case IIO_EV_DIR_FALLING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_PS_THDL_LM, val);  
> > >
> > > Same.  
>
diff mbox series

Patch

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 285f1dd314d8..a84fda5e29d0 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -62,6 +62,8 @@ 
 #define VCNL4200_PS_CONF1	0x03 /* Proximity configuration */
 #define VCNL4040_PS_THDL_LM	0x06 /* Proximity threshold low */
 #define VCNL4040_PS_THDH_LM	0x07 /* Proximity threshold high */
+#define VCNL4040_ALS_THDL_LM	0x02 /* Ambient light threshold low */
+#define VCNL4040_ALS_THDH_LM	0x01 /* Ambient light threshold high */
 #define VCNL4200_PS_DATA	0x08 /* Proximity data */
 #define VCNL4200_AL_DATA	0x09 /* Ambient light data */
 #define VCNL4040_INT_FLAGS	0x0b /* Interrupt register */
@@ -81,11 +83,14 @@ 
 
 #define VCNL4040_ALS_CONF_ALS_SHUTDOWN	BIT(0)
 #define VCNL4040_ALS_CONF_IT		GENMASK(7, 6) /* Ambient integration time */
+#define VCNL4040_ALS_CONF_INT_EN	BIT(1) /* Ambient light Interrupt enable */
 #define VCNL4040_PS_CONF1_PS_SHUTDOWN	BIT(0)
 #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
 #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
 #define VCNL4040_PS_IF_AWAY		BIT(8) /* Proximity event cross low threshold */
 #define VCNL4040_PS_IF_CLOSE		BIT(9) /* Proximity event cross high threshold */
+#define VCNL4040_ALS_RISING		BIT(12) /* Ambient Light cross high threshold */
+#define VCNL4040_ALS_FALLING		BIT(13) /* Ambient Light cross low threshold */
 
 /* Bit masks for interrupt registers. */
 #define VCNL4010_INT_THR_SEL	BIT(0) /* Select threshold interrupt source */
@@ -170,6 +175,7 @@  struct vcnl4000_data {
 	int rev;
 	int al_scale;
 	u8 ps_int;		/* proximity interrupt mode */
+	u8 als_int;		/* ambient light interrupt mode*/
 	const struct vcnl4000_chip_spec *chip_spec;
 	struct mutex vcnl4000_lock;
 	struct vcnl4200_channel vcnl4200_al;
@@ -293,7 +299,7 @@  static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
 	int ret;
 
 	/* Do not power down if interrupts are enabled */
-	if (!on && data->ps_int)
+	if (!on && (data->ps_int || data->als_int))
 		return 0;
 
 	ret = vcnl4000_write_als_enable(data, on);
@@ -338,6 +344,7 @@  static int vcnl4200_init(struct vcnl4000_data *data)
 
 	data->rev = (ret >> 8) & 0xf;
 	data->ps_int = 0;
+	data->als_int = 0;
 
 	data->vcnl4200_al.reg = VCNL4200_AL_DATA;
 	data->vcnl4200_ps.reg = VCNL4200_PS_DATA;
@@ -931,27 +938,45 @@  static int vcnl4040_read_event(struct iio_dev *indio_dev,
 			       enum iio_event_info info,
 			       int *val, int *val2)
 {
-	int ret;
+	int ret = -EINVAL;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		ret = i2c_smbus_read_word_data(data->client,
-					       VCNL4040_PS_THDH_LM);
-		if (ret < 0)
-			return ret;
-		*val = ret;
-		return IIO_VAL_INT;
-	case IIO_EV_DIR_FALLING:
-		ret = i2c_smbus_read_word_data(data->client,
-					       VCNL4040_PS_THDL_LM);
-		if (ret < 0)
-			return ret;
-		*val = ret;
-		return IIO_VAL_INT;
+	switch (chan->type) {
+	case IIO_LIGHT:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_ALS_THDH_LM);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_ALS_THDL_LM);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_PROXIMITY:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_PS_THDH_LM);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_PS_THDL_LM);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
+	if (ret < 0)
+		return ret;
+	*val = ret;
+	return IIO_VAL_INT;
 }
 
 static int vcnl4040_write_event(struct iio_dev *indio_dev,
@@ -961,25 +986,43 @@  static int vcnl4040_write_event(struct iio_dev *indio_dev,
 				enum iio_event_info info,
 				int val, int val2)
 {
-	int ret;
+	int ret = -EINVAL;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
-
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		ret = i2c_smbus_write_word_data(data->client,
-						VCNL4040_PS_THDH_LM, val);
-		if (ret < 0)
-			return ret;
-		return IIO_VAL_INT;
-	case IIO_EV_DIR_FALLING:
-		ret = i2c_smbus_write_word_data(data->client,
-						VCNL4040_PS_THDL_LM, val);
-		if (ret < 0)
-			return ret;
-		return IIO_VAL_INT;
+	switch (chan->type) {
+	case IIO_LIGHT:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_ALS_THDH_LM, val);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_ALS_THDL_LM, val);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_PROXIMITY:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_PS_THDH_LM, val);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_PS_THDL_LM, val);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
+	if (ret < 0)
+		return ret;
+	return IIO_VAL_INT;
 }
 
 static bool vcnl4010_is_thr_enabled(struct vcnl4000_data *data)
@@ -1071,16 +1114,28 @@  static int vcnl4040_read_event_config(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
+	switch (chan->type) {
+	case IIO_LIGHT:
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+		if (ret < 0)
+			return ret;
 
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
-	if (ret < 0)
-		return ret;
+		data->als_int = FIELD_GET(VCNL4040_ALS_CONF_INT_EN, ret);
 
-	data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, ret);
+		return data->als_int;
+	case IIO_PROXIMITY:
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+		if (ret < 0)
+			return ret;
+
+		data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, ret);
 
-	return (dir == IIO_EV_DIR_RISING) ?
-		FIELD_GET(VCNL4040_PS_IF_AWAY, ret) :
-		FIELD_GET(VCNL4040_PS_IF_CLOSE, ret);
+		return (dir == IIO_EV_DIR_RISING) ?
+			FIELD_GET(VCNL4040_PS_IF_AWAY, ret) :
+			FIELD_GET(VCNL4040_PS_IF_CLOSE, ret);
+	default:
+		return -EINVAL;
+	}
 }
 
 static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
@@ -1088,29 +1143,51 @@  static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
 				       enum iio_event_type type,
 				       enum iio_event_direction dir, int state)
 {
-	int ret;
+	int ret = -EINVAL;
 	u16 val, mask;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 
 	mutex_lock(&data->vcnl4000_lock);
 
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
-	if (ret < 0)
-		goto out;
+	switch (chan->type) {
+	case IIO_LIGHT:
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+		if (ret < 0)
+			goto out;
 
-	if (dir == IIO_EV_DIR_RISING)
-		mask = VCNL4040_PS_IF_AWAY;
-	else
-		mask = VCNL4040_PS_IF_CLOSE;
+		mask = VCNL4040_ALS_CONF_INT_EN;
 
-	val = state ? (ret | mask) : (ret & ~mask);
+		val = state ? (ret | mask) : (ret & ~mask);
 
-	data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, val);
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val);
+		data->als_int = FIELD_GET(VCNL4040_ALS_CONF_INT_EN, val);
+		ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
+						val);
+		break;
+	case IIO_PROXIMITY:
+
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+		if (ret < 0)
+			goto out;
+
+		if (dir == IIO_EV_DIR_RISING)
+			mask = VCNL4040_PS_IF_AWAY;
+		else
+			mask = VCNL4040_PS_IF_CLOSE;
+
+		val = state ? (ret | mask) : (ret & ~mask);
+
+		data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, val);
+		ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
+						val);
+		break;
+	default:
+		break;
+	}
 
 out:
 	mutex_unlock(&data->vcnl4000_lock);
-	data->chip_spec->set_power_state(data, data->ps_int != 0);
+	data->chip_spec->set_power_state(data, data->ps_int ||
+						data->als_int);
 
 	return ret;
 }
@@ -1141,6 +1218,22 @@  static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
 			       iio_get_time_ns(indio_dev));
 	}
 
+	if (ret & VCNL4040_ALS_FALLING) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_LIGHT, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_FALLING),
+			       iio_get_time_ns(indio_dev));
+	}
+
+	if (ret & VCNL4040_ALS_RISING) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_LIGHT, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+			       iio_get_time_ns(indio_dev));
+	}
+
 	return IRQ_HANDLED;
 }
 
@@ -1363,6 +1456,8 @@  static const struct iio_chan_spec vcnl4040_channels[] = {
 			BIT(IIO_CHAN_INFO_SCALE) |
 			BIT(IIO_CHAN_INFO_INT_TIME),
 		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
+		.event_spec = vcnl4000_event_spec,
+		.num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
 	}, {
 		.type = IIO_PROXIMITY,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |