diff mbox series

[v2,2/3] iio: light: ltr390: Add ALS channel and support for gain and resolution

Message ID 20240728151957.310237-2-abhashkumarjha123@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series [v2,1/3] iio: light: ltr390: Add configurable gain and resolution | expand

Commit Message

Abhash Jha July 28, 2024, 3:19 p.m. UTC 
Add new ALS channel and allow reading raw and scale values.
    Also provide gain and resolution configuration for ALS channel.
    Add automatic mode switching between the UVS and ALS channel
    based on which channel is being accessed.
    The default mode in which the sensor start is ALS mode.

Signed-off-by: Abhash Jha <abhashkumarjha123@gmail.com>
---
 drivers/iio/light/ltr390.c | 111 ++++++++++++++++++++++++++++++++-----
 1 file changed, 96 insertions(+), 15 deletions(-)

Comments

Jonathan Cameron July 28, 2024, 4:58 p.m. UTC | #1 
On Sun, 28 Jul 2024 20:49:55 +0530
Abhash Jha <abhashkumarjha123@gmail.com> wrote:

>     Add new ALS channel and allow reading raw and scale values.
>     Also provide gain and resolution configuration for ALS channel.
>     Add automatic mode switching between the UVS and ALS channel
>     based on which channel is being accessed.
>     The default mode in which the sensor start is ALS mode.
Same issue with indent as in patch 1.

> 
> Signed-off-by: Abhash Jha <abhashkumarjha123@gmail.com>
Later patches should not be in reply to the first one.
Use a cover letter.

Minor comments inline on using the enum to maintain the type of the mode
setting.


> ---
>  drivers/iio/light/ltr390.c | 111 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 96 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
> index 9f00661c3..d1a259141 100644
> --- a/drivers/iio/light/ltr390.c
> +++ b/drivers/iio/light/ltr390.c
> @@ -62,11 +62,17 @@
>   */
>  #define LTR390_WINDOW_FACTOR 1
>  
> +enum ltr390_mode {
> +	LTR390_SET_ALS_MODE,
> +	LTR390_SET_UVS_MODE,
> +};
> +
>  struct ltr390_data {
>  	struct regmap *regmap;
>  	struct i2c_client *client;
>  	/* Protects device from simulataneous reads */
>  	struct mutex lock;
> +	int mode;
enum ltr380_mode mode;

>  	int gain;
>  	int int_time_us;
>  };
> @@ -94,6 +100,28 @@ static int ltr390_register_read(struct ltr390_data *data, u8 register_address)
>  	return get_unaligned_le24(recieve_buffer);
>  }
>  
> +static int ltr390_set_mode(struct ltr390_data *data, int mode)

pass in the enum lt380_mode.

> +{
> +	if (data->mode == mode)
> +		return 0;
> +
> +	switch (mode) {
> +	case LTR390_SET_ALS_MODE:
> +		regmap_clear_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_UVS_MODE);
> +		break;
> +
> +	case LTR390_SET_UVS_MODE:
> +		regmap_set_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_UVS_MODE);
> +		break;
> +
> +	default:
> +		return -EINVAL;
Should be able to drop this if passing in the enum as the compiler can see you
have cases for every value.

> +	}
> +
> +	data->mode = mode;
> +	return 0;
> +}
> +
diff mbox series

Patch

diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
index 9f00661c3..d1a259141 100644
--- a/drivers/iio/light/ltr390.c
+++ b/drivers/iio/light/ltr390.c
@@ -62,11 +62,17 @@ 
  */
 #define LTR390_WINDOW_FACTOR 1
 
+enum ltr390_mode {
+	LTR390_SET_ALS_MODE,
+	LTR390_SET_UVS_MODE,
+};
+
 struct ltr390_data {
 	struct regmap *regmap;
 	struct i2c_client *client;
 	/* Protects device from simulataneous reads */
 	struct mutex lock;
+	int mode;
 	int gain;
 	int int_time_us;
 };
@@ -94,6 +100,28 @@  static int ltr390_register_read(struct ltr390_data *data, u8 register_address)
 	return get_unaligned_le24(recieve_buffer);
 }
 
+static int ltr390_set_mode(struct ltr390_data *data, int mode)
+{
+	if (data->mode == mode)
+		return 0;
+
+	switch (mode) {
+	case LTR390_SET_ALS_MODE:
+		regmap_clear_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_UVS_MODE);
+		break;
+
+	case LTR390_SET_UVS_MODE:
+		regmap_set_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_UVS_MODE);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	data->mode = mode;
+	return 0;
+}
+
 static int ltr390_read_raw(struct iio_dev *iio_device,
 			   struct iio_chan_spec const *chan, int *val,
 			   int *val2, long mask)
@@ -104,17 +132,57 @@  static int ltr390_read_raw(struct iio_dev *iio_device,
 	guard(mutex)(&data->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = ltr390_register_read(data, LTR390_UVS_DATA);
-		if (ret < 0)
-			return ret;
+		switch (chan->type) {
+		case IIO_UVINDEX:
+			ret = ltr390_set_mode(data, LTR390_SET_UVS_MODE);
+			if (ret < 0)
+				return ret;
+
+			ret = ltr390_register_read(data, LTR390_UVS_DATA);
+			if (ret < 0)
+				return ret;
+			break;
+
+		case IIO_INTENSITY:
+			ret = ltr390_set_mode(data, LTR390_SET_ALS_MODE);
+			if (ret < 0)
+				return ret;
+
+			ret = ltr390_register_read(data, LTR390_ALS_DATA);
+			if (ret < 0)
+				return ret;
+			break;
+
+		default:
+			return -EINVAL;
+		}
 
 		*val = ret;
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = LTR390_WINDOW_FACTOR;
-		*val2 = LTR390_COUNTS_PER_UVI;
-		return IIO_VAL_FRACTIONAL;
+		switch (chan->type) {
+		case IIO_UVINDEX:
+			ret = ltr390_set_mode(data, LTR390_SET_UVS_MODE);
+			if (ret < 0)
+				return ret;
+
+			*val = LTR390_WINDOW_FACTOR;
+			*val2 = LTR390_COUNTS_PER_UVI;
+			return IIO_VAL_FRACTIONAL;
+
+		case IIO_INTENSITY:
+			ret = ltr390_set_mode(data, LTR390_SET_ALS_MODE);
+			if (ret < 0)
+				return ret;
+
+			*val = LTR390_WINDOW_FACTOR;
+			*val2 = data->gain;
+			return IIO_VAL_FRACTIONAL;
+
+		default:
+			return -EINVAL;
+		}
 
 	case IIO_CHAN_INFO_INT_TIME:
 		*val = data->int_time_us;
@@ -129,11 +197,23 @@  static int ltr390_read_raw(struct iio_dev *iio_device,
 static const int ltr390_int_time_map_us[] = {400000, 200000, 100000, 50000, 25000, 12500};
 static const int ltr390_gain_map[] = {1, 3, 6, 9, 18};
 
-static const struct iio_chan_spec ltr390_channel = {
-	.type = IIO_UVINDEX,
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
-	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
-	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) | BIT(IIO_CHAN_INFO_SCALE)
+static const struct iio_chan_spec ltr390_channels[] = {
+	/* UV sensor */
+	{
+		.type = IIO_UVINDEX,
+		.scan_index = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) | BIT(IIO_CHAN_INFO_SCALE)
+	},
+	/* ALS sensor */
+	{
+		.type = IIO_INTENSITY,
+		.scan_index = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) | BIT(IIO_CHAN_INFO_SCALE)
+	},
 };
 
 static int ltr390_set_gain(struct ltr390_data *data, int val)
@@ -253,12 +333,14 @@  static int ltr390_probe(struct i2c_client *client)
 	data->int_time_us = 100000;
 	/* default value of gain from pg: 16 of the datasheet */
 	data->gain = 3;
+	/* default mode for ltr390 is ALS mode */
+	data->mode = LTR390_SET_ALS_MODE;
 
 	mutex_init(&data->lock);
 
 	indio_dev->info = &ltr390_info;
-	indio_dev->channels = &ltr390_channel;
-	indio_dev->num_channels = 1;
+	indio_dev->channels = ltr390_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ltr390_channels);
 	indio_dev->name = "ltr390";
 
 	ret = regmap_read(data->regmap, LTR390_PART_ID, &part_number);
@@ -276,8 +358,7 @@  static int ltr390_probe(struct i2c_client *client)
 	/* Wait for the registers to reset before proceeding */
 	usleep_range(1000, 2000);
 
-	ret = regmap_set_bits(data->regmap, LTR390_MAIN_CTRL,
-				LTR390_SENSOR_ENABLE | LTR390_UVS_MODE);
+	ret = regmap_set_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_SENSOR_ENABLE);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to enable the sensor\n");