[PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor

Emil Gedenryd posted 2 patches 1 month, 3 weeks ago
[PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Emil Gedenryd 1 month, 3 weeks ago
TI's opt3002 light sensor shares most properties with the opt3001
model, with the exception of supporting a wider spectrum range.

Add support for TI's opt3002 by extending the TI opt3001 driver.

Datasheet: https://www.ti.com/product/OPT3002
Signed-off-by: Emil Gedenryd <emil.gedenryd@axis.com>
---
 drivers/iio/light/Kconfig   |   2 +-
 drivers/iio/light/opt3001.c | 189 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 157 insertions(+), 34 deletions(-)

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index b68dcc1fbaca..c35bf962dae6 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -461,7 +461,7 @@ config OPT3001
 	depends on I2C
 	help
 	  If you say Y or M here, you get support for Texas Instruments
-	  OPT3001 Ambient Light Sensor.
+	  OPT3001 Ambient Light Sensor, OPT3002 Light-to-Digital Sensor.
 
 	  If built as a dynamically linked module, it will be called
 	  opt3001.
diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
index 176e54bb48c3..ff7fc0d4b08f 100644
--- a/drivers/iio/light/opt3001.c
+++ b/drivers/iio/light/opt3001.c
@@ -70,6 +70,35 @@
 #define OPT3001_RESULT_READY_SHORT	150
 #define OPT3001_RESULT_READY_LONG	1000
 
+struct opt3001_scale {
+	int	val;
+	int	val2;
+};
+
+struct opt3001_chip_info {
+	const struct iio_chan_spec (*channels)[2];
+	enum iio_chan_type chan_type;
+	int num_channels;
+
+	const struct opt3001_scale (*scales)[12];
+	/*
+	 * Factor as specified by conversion equation in datasheet.
+	 * eg. 0.01 (scaled to integer 10) for opt3001.
+	 */
+	int factor_whole;
+	/*
+	 * Factor to compensate for potentially scaled factor_whole.
+	 */
+	int factor_integer;
+	/*
+	 * Factor used to align decimal part of proccessed value to six decimal
+	 * places.
+	 */
+	int factor_decimal;
+
+	bool has_id;
+};
+
 struct opt3001 {
 	struct i2c_client	*client;
 	struct device		*dev;
@@ -79,6 +108,7 @@ struct opt3001 {
 	bool			result_ready;
 	wait_queue_head_t	result_ready_queue;
 	u16			result;
+	const struct opt3001_chip_info *chip_info;
 
 	u32			int_time;
 	u32			mode;
@@ -92,11 +122,6 @@ struct opt3001 {
 	bool			use_irq;
 };
 
-struct opt3001_scale {
-	int	val;
-	int	val2;
-};
-
 static const struct opt3001_scale opt3001_scales[] = {
 	{
 		.val = 40,
@@ -148,21 +173,68 @@ static const struct opt3001_scale opt3001_scales[] = {
 	},
 };
 
+static const struct opt3001_scale opt3002_scales[] = {
+	{
+		.val = 4914,
+		.val2 = 0,
+	},
+	{
+		.val = 9828,
+		.val2 = 0,
+	},
+	{
+		.val = 19656,
+		.val2 = 0,
+	},
+	{
+		.val = 39312,
+		.val2 = 0,
+	},
+	{
+		.val = 78624,
+		.val2 = 0,
+	},
+	{
+		.val = 157248,
+		.val2 = 0,
+	},
+	{
+		.val = 314496,
+		.val2 = 0,
+	},
+	{
+		.val = 628992,
+		.val2 = 0,
+	},
+	{
+		.val = 1257984,
+		.val2 = 0,
+	},
+	{
+		.val = 2515968,
+		.val2 = 0,
+	},
+	{
+		.val = 5031936,
+		.val2 = 0,
+	},
+	{
+		.val = 10063872,
+		.val2 = 0,
+	},
+};
+
 static int opt3001_find_scale(const struct opt3001 *opt, int val,
 		int val2, u8 *exponent)
 {
 	int i;
-
-	for (i = 0; i < ARRAY_SIZE(opt3001_scales); i++) {
-		const struct opt3001_scale *scale = &opt3001_scales[i];
-
+	for (i = 0; i < ARRAY_SIZE(*opt->chip_info->scales); i++) {
+		const struct opt3001_scale *scale = &(*opt->chip_info->scales)[i];
 		/*
-		 * Combine the integer and micro parts for comparison
-		 * purposes. Use milli lux precision to avoid 32-bit integer
-		 * overflows.
+		 * Compare the integer and micro parts to determine value scale.
 		 */
-		if ((val * 1000 + val2 / 1000) <=
-				(scale->val * 1000 + scale->val2 / 1000)) {
+		if (val < scale->val ||
+		    (val == scale->val && val2 <= scale->val2)) {
 			*exponent = i;
 			return 0;
 		}
@@ -174,11 +246,14 @@ static int opt3001_find_scale(const struct opt3001 *opt, int val,
 static void opt3001_to_iio_ret(struct opt3001 *opt, u8 exponent,
 		u16 mantissa, int *val, int *val2)
 {
-	int lux;
+	int ret;
+	int whole = opt->chip_info->factor_whole;
+	int integer = opt->chip_info->factor_integer;
+	int decimal = opt->chip_info->factor_decimal;
 
-	lux = 10 * (mantissa << exponent);
-	*val = lux / 1000;
-	*val2 = (lux - (*val * 1000)) * 1000;
+	ret = whole * (mantissa << exponent);
+	*val = ret / integer;
+	*val2 = (ret - (*val * integer)) * decimal;
 }
 
 static void opt3001_set_mode(struct opt3001 *opt, u16 *reg, u16 mode)
@@ -225,7 +300,18 @@ static const struct iio_chan_spec opt3001_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1),
 };
 
-static int opt3001_get_lux(struct opt3001 *opt, int *val, int *val2)
+static const struct iio_chan_spec opt3002_channels[] = {
+	{
+		.type = IIO_INTENSITY,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_INT_TIME),
+		.event_spec = opt3001_event_spec,
+		.num_event_specs = ARRAY_SIZE(opt3001_event_spec),
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static int opt3001_get_processed(struct opt3001 *opt, int *val, int *val2)
 {
 	int ret;
 	u16 mantissa;
@@ -397,14 +483,15 @@ static int opt3001_read_raw(struct iio_dev *iio,
 	if (opt->mode == OPT3001_CONFIGURATION_M_CONTINUOUS)
 		return -EBUSY;
 
-	if (chan->type != IIO_LIGHT)
+	if (chan->type != opt->chip_info->chan_type)
 		return -EINVAL;
 
 	mutex_lock(&opt->lock);
 
 	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
 	case IIO_CHAN_INFO_PROCESSED:
-		ret = opt3001_get_lux(opt, val, val2);
+		ret = opt3001_get_processed(opt, val, val2);
 		break;
 	case IIO_CHAN_INFO_INT_TIME:
 		ret = opt3001_get_int_time(opt, val, val2);
@@ -428,7 +515,7 @@ static int opt3001_write_raw(struct iio_dev *iio,
 	if (opt->mode == OPT3001_CONFIGURATION_M_CONTINUOUS)
 		return -EBUSY;
 
-	if (chan->type != IIO_LIGHT)
+	if (chan->type != opt->chip_info->chan_type)
 		return -EINVAL;
 
 	if (mask != IIO_CHAN_INFO_INT_TIME)
@@ -479,6 +566,9 @@ static int opt3001_write_event_value(struct iio_dev *iio,
 {
 	struct opt3001 *opt = iio_priv(iio);
 	int ret;
+	int whole;
+	int integer;
+	int decimal;
 
 	u16 mantissa;
 	u16 value;
@@ -497,7 +587,12 @@ static int opt3001_write_event_value(struct iio_dev *iio,
 		goto err;
 	}
 
-	mantissa = (((val * 1000) + (val2 / 1000)) / 10) >> exponent;
+	whole = opt->chip_info->factor_whole;
+	integer = opt->chip_info->factor_integer;
+	decimal = opt->chip_info->factor_decimal;
+
+	mantissa = (((val * integer) + (val2 / decimal)) / whole) >> exponent;
+
 	value = (exponent << 12) | mantissa;
 
 	switch (dir) {
@@ -610,7 +705,7 @@ static int opt3001_read_id(struct opt3001 *opt)
 	ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_DEVICE_ID);
 	if (ret < 0) {
 		dev_err(opt->dev, "failed to read register %02x\n",
-				OPT3001_DEVICE_ID);
+			OPT3001_DEVICE_ID);
 		return ret;
 	}
 
@@ -692,6 +787,7 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
 	struct opt3001 *opt = iio_priv(iio);
 	int ret;
 	bool wake_result_ready_queue = false;
+	enum iio_chan_type chan_type = opt->chip_info->chan_type;
 
 	if (!opt->ok_to_ignore_lock)
 		mutex_lock(&opt->lock);
@@ -707,13 +803,13 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
 			OPT3001_CONFIGURATION_M_CONTINUOUS) {
 		if (ret & OPT3001_CONFIGURATION_FH)
 			iio_push_event(iio,
-					IIO_UNMOD_EVENT_CODE(IIO_LIGHT, 0,
+					IIO_UNMOD_EVENT_CODE(chan_type, 0,
 							IIO_EV_TYPE_THRESH,
 							IIO_EV_DIR_RISING),
 					iio_get_time_ns(iio));
 		if (ret & OPT3001_CONFIGURATION_FL)
 			iio_push_event(iio,
-					IIO_UNMOD_EVENT_CODE(IIO_LIGHT, 0,
+					IIO_UNMOD_EVENT_CODE(chan_type, 0,
 							IIO_EV_TYPE_THRESH,
 							IIO_EV_DIR_FALLING),
 					iio_get_time_ns(iio));
@@ -755,22 +851,25 @@ static int opt3001_probe(struct i2c_client *client)
 	opt = iio_priv(iio);
 	opt->client = client;
 	opt->dev = dev;
+	opt->chip_info = i2c_get_match_data(client);
 
 	mutex_init(&opt->lock);
 	init_waitqueue_head(&opt->result_ready_queue);
 	i2c_set_clientdata(client, iio);
 
-	ret = opt3001_read_id(opt);
-	if (ret)
-		return ret;
+	if (opt->chip_info->has_id) {
+		ret = opt3001_read_id(opt);
+		if (ret)
+			return ret;
+	}
 
 	ret = opt3001_configure(opt);
 	if (ret)
 		return ret;
 
 	iio->name = client->name;
-	iio->channels = opt3001_channels;
-	iio->num_channels = ARRAY_SIZE(opt3001_channels);
+	iio->channels = *opt->chip_info->channels;
+	iio->num_channels = opt->chip_info->num_channels;
 	iio->modes = INDIO_DIRECT_MODE;
 	iio->info = &opt3001_info;
 
@@ -825,14 +924,38 @@ static void opt3001_remove(struct i2c_client *client)
 	}
 }
 
+static const struct opt3001_chip_info opt3001_chip_information = {
+	.channels = &opt3001_channels,
+	.chan_type = IIO_LIGHT,
+	.num_channels = ARRAY_SIZE(opt3001_channels),
+	.scales = &opt3001_scales,
+	.factor_whole = 10,
+	.factor_integer = 1000,
+	.factor_decimal = 1000,
+	.has_id = true,
+};
+
+static const struct opt3001_chip_info opt3002_chip_information = {
+	.channels = &opt3002_channels,
+	.chan_type = IIO_INTENSITY,
+	.num_channels = ARRAY_SIZE(opt3002_channels),
+	.scales = &opt3002_scales,
+	.factor_whole = 12,
+	.factor_integer = 10,
+	.factor_decimal = 100000,
+	.has_id = false,
+};
+
 static const struct i2c_device_id opt3001_id[] = {
-	{ "opt3001" },
+	{ "opt3001", (kernel_ulong_t)&opt3001_chip_information },
+	{ "opt3002", (kernel_ulong_t)&opt3002_chip_information },
 	{ } /* Terminating Entry */
 };
 MODULE_DEVICE_TABLE(i2c, opt3001_id);
 
 static const struct of_device_id opt3001_of_match[] = {
-	{ .compatible = "ti,opt3001" },
+	{ .compatible = "ti,opt3001", .data = &opt3001_chip_information },
+	{ .compatible = "ti,opt3002", .data = &opt3002_chip_information },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, opt3001_of_match);

-- 
2.39.2
Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Jonathan Cameron 1 month, 3 weeks ago
On Thu, 3 Oct 2024 14:22:17 +0200
Emil Gedenryd <emil.gedenryd@axis.com> wrote:

> TI's opt3002 light sensor shares most properties with the opt3001
> model, with the exception of supporting a wider spectrum range.
> 
> Add support for TI's opt3002 by extending the TI opt3001 driver.
> 
> Datasheet: https://www.ti.com/product/OPT3002
> Signed-off-by: Emil Gedenryd <emil.gedenryd@axis.com>
> ---
>  drivers/iio/light/Kconfig   |   2 +-
>  drivers/iio/light/opt3001.c | 189 ++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 157 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index b68dcc1fbaca..c35bf962dae6 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -461,7 +461,7 @@ config OPT3001
>  	depends on I2C
>  	help
>  	  If you say Y or M here, you get support for Texas Instruments
> -	  OPT3001 Ambient Light Sensor.
> +	  OPT3001 Ambient Light Sensor, OPT3002 Light-to-Digital Sensor.
>  
>  	  If built as a dynamically linked module, it will be called
>  	  opt3001.
> diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
> index 176e54bb48c3..ff7fc0d4b08f 100644
> --- a/drivers/iio/light/opt3001.c
> +++ b/drivers/iio/light/opt3001.c
> @@ -70,6 +70,35 @@
>  #define OPT3001_RESULT_READY_SHORT	150
>  #define OPT3001_RESULT_READY_LONG	1000
>  
> +struct opt3001_scale {
> +	int	val;
> +	int	val2;
> +};
> +
> +struct opt3001_chip_info {
> +	const struct iio_chan_spec (*channels)[2];
> +	enum iio_chan_type chan_type;
> +	int num_channels;
> +
> +	const struct opt3001_scale (*scales)[12];
This doesn't compile for me as one of the two options only
has 11 entries.  You could either force them to be 12
entries each or use a pointer without the size and
add a num_scales entry in here.

Jonathan
Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Emil Gedenryd 1 month, 3 weeks ago
On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:
> On Thu, 3 Oct 2024 14:22:17 +0200
> Emil Gedenryd <emil.gedenryd@axis.com> wrote:
> > 
> > +struct opt3001_chip_info {
> > +	const struct iio_chan_spec (*channels)[2];
> > +	enum iio_chan_type chan_type;
> > +	int num_channels;
> > +
> > +	const struct opt3001_scale (*scales)[12];
> This doesn't compile for me as one of the two options only
> has 11 entries.  You could either force them to be 12
> entries each or use a pointer without the size and
> add a num_scales entry in here.
> 
> Jonathan

Hi Jonathan,

Are you building on top of the patch that was accepted in earlier versions of this
patch set? That patch adds the twelfth missing scale value for the opt3001.
See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/

Should I have added some tag to highlight the dependency for this version of the
patch set?

Best regards,
Emil 
Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Mon, 7 Oct 2024 07:19:06 +0000
Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:

> On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:
> > On Thu, 3 Oct 2024 14:22:17 +0200
> > Emil Gedenryd <emil.gedenryd@axis.com> wrote:  
> > > 
> > > +struct opt3001_chip_info {
> > > +	const struct iio_chan_spec (*channels)[2];
> > > +	enum iio_chan_type chan_type;
> > > +	int num_channels;
> > > +
> > > +	const struct opt3001_scale (*scales)[12];  
> > This doesn't compile for me as one of the two options only
> > has 11 entries.  You could either force them to be 12
> > entries each or use a pointer without the size and
> > add a num_scales entry in here.
> > 
> > Jonathan  
> 
> Hi Jonathan,
> 
> Are you building on top of the patch that was accepted in earlier versions of this
> patch set? That patch adds the twelfth missing scale value for the opt3001.
> See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/
> 
> Should I have added some tag to highlight the dependency for this version of the
> patch set?
Ah.  Yes, I was half asleep.
They are going via different branches (slow and fast) so I'll have to
sit on this series until after that fix is in the upstream for the togreg
branch of iio.git.

If I seem to have lost it after that is the case feel free to give me a poke.

Jonathan


> 
> Best regards,
> Emil 
Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Emil Gedenryd 1 month, 2 weeks ago
On Thu, 2024-10-10 at 18:47 +0100, Jonathan Cameron wrote:
> On Mon, 7 Oct 2024 07:19:06 +0000
> Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> 
> > On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:
> > > On Thu, 3 Oct 2024 14:22:17 +0200
> > > Emil Gedenryd <emil.gedenryd@axis.com> wrote:  
> > > > 
> > > > +struct opt3001_chip_info {
> > > > +	const struct iio_chan_spec (*channels)[2];
> > > > +	enum iio_chan_type chan_type;
> > > > +	int num_channels;
> > > > +
> > > > +	const struct opt3001_scale (*scales)[12];  
> > > This doesn't compile for me as one of the two options only
> > > has 11 entries.  You could either force them to be 12
> > > entries each or use a pointer without the size and
> > > add a num_scales entry in here.
> > > 
> > > Jonathan  
> > 
> > Hi Jonathan,
> > 
> > Are you building on top of the patch that was accepted in earlier versions of this
> > patch set? That patch adds the twelfth missing scale value for the opt3001.
> > See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/
> > 
> > Should I have added some tag to highlight the dependency for this version of the
> > patch set?
> Ah.  Yes, I was half asleep.
> They are going via different branches (slow and fast) so I'll have to
> sit on this series until after that fix is in the upstream for the togreg
> branch of iio.git.
> 
> If I seem to have lost it after that is the case feel free to give me a poke.
> 
> Jonathan
> 
Hi,

No worries. Just to clarify, do you mean sit on it as that you will continue reviewing
the code after the fix is in upstream, or should I consider this patch to be approved?

Also, do you have an approximation of what time frame we're talking about?

Best Regards,
Emil 

Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Fri, 11 Oct 2024 07:12:05 +0000
Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:

> On Thu, 2024-10-10 at 18:47 +0100, Jonathan Cameron wrote:
> > On Mon, 7 Oct 2024 07:19:06 +0000
> > Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> >   
> > > On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:  
> > > > On Thu, 3 Oct 2024 14:22:17 +0200
> > > > Emil Gedenryd <emil.gedenryd@axis.com> wrote:    
> > > > > 
> > > > > +struct opt3001_chip_info {
> > > > > +	const struct iio_chan_spec (*channels)[2];
> > > > > +	enum iio_chan_type chan_type;
> > > > > +	int num_channels;
> > > > > +
> > > > > +	const struct opt3001_scale (*scales)[12];    
> > > > This doesn't compile for me as one of the two options only
> > > > has 11 entries.  You could either force them to be 12
> > > > entries each or use a pointer without the size and
> > > > add a num_scales entry in here.
> > > > 
> > > > Jonathan    
> > > 
> > > Hi Jonathan,
> > > 
> > > Are you building on top of the patch that was accepted in earlier versions of this
> > > patch set? That patch adds the twelfth missing scale value for the opt3001.
> > > See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/
> > > 
> > > Should I have added some tag to highlight the dependency for this version of the
> > > patch set?  
> > Ah.  Yes, I was half asleep.
> > They are going via different branches (slow and fast) so I'll have to
> > sit on this series until after that fix is in the upstream for the togreg
> > branch of iio.git.
> > 
> > If I seem to have lost it after that is the case feel free to give me a poke.
> > 
> > Jonathan
> >   
> Hi,
> 
> No worries. Just to clarify, do you mean sit on it as that you will continue reviewing
> the code after the fix is in upstream, or should I consider this patch to be approved?
Assuming not other review comes in, I consider this ready to go.
> 
> Also, do you have an approximation of what time frame we're talking about?
2 weeks most likely.

I've just sent Greg KH a pull request with the fix in it. He will hopefully
pick that up and then send a pull request on to Linus.  Then we wait for the
next rc after that at which point Greg will probably pull it into char-misc-next or
I can always merge it into my togreg branch once it is in a release candidate of
Linus' tree.

In parallel with that I'll probably do a pull request for what is already in the
togreg tree to get a lot of stuff in char-misc-next for the next cycle. That makes
the history a little cleaner as I can fast forward my tree and end up with
whatever is in char-misc-next (hopefully including this).

Anyhow, a bit of tree juggling for me, but we have plenty of time as rc3 will probably
be out tomorrow and it normally goes to rc7 at one rc a week

Thanks,

Jonathan
> Best Regards,
> Emil 
> 
Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Emil Gedenryd 1 month, 2 weeks ago
On Sat, 2024-10-12 at 16:10 +0100, Jonathan Cameron wrote:
> On Fri, 11 Oct 2024 07:12:05 +0000
> Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> 
> > On Thu, 2024-10-10 at 18:47 +0100, Jonathan Cameron wrote:
> > > On Mon, 7 Oct 2024 07:19:06 +0000
> > > Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> > >   
> > > > On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:  
> > > > > On Thu, 3 Oct 2024 14:22:17 +0200
> > > > > Emil Gedenryd <emil.gedenryd@axis.com> wrote:    
> > > > > > 
> > > > > > +struct opt3001_chip_info {
> > > > > > +	const struct iio_chan_spec (*channels)[2];
> > > > > > +	enum iio_chan_type chan_type;
> > > > > > +	int num_channels;
> > > > > > +
> > > > > > +	const struct opt3001_scale (*scales)[12];    
> > > > > This doesn't compile for me as one of the two options only
> > > > > has 11 entries.  You could either force them to be 12
> > > > > entries each or use a pointer without the size and
> > > > > add a num_scales entry in here.
> > > > > 
> > > > > Jonathan    
> > > > 
> > > > Hi Jonathan,
> > > > 
> > > > Are you building on top of the patch that was accepted in earlier versions of this
> > > > patch set? That patch adds the twelfth missing scale value for the opt3001.
> > > > See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/
> > > > 
> > > > Should I have added some tag to highlight the dependency for this version of the
> > > > patch set?  
> > > Ah.  Yes, I was half asleep.
> > > They are going via different branches (slow and fast) so I'll have to
> > > sit on this series until after that fix is in the upstream for the togreg
> > > branch of iio.git.
> > > 
> > > If I seem to have lost it after that is the case feel free to give me a poke.
> > > 
> > > Jonathan
> > >   
> > Hi,
> > 
> > No worries. Just to clarify, do you mean sit on it as that you will continue reviewing
> > the code after the fix is in upstream, or should I consider this patch to be approved?
> Assuming not other review comes in, I consider this ready to go.

Great, thank you!

> > 
> > Also, do you have an approximation of what time frame we're talking about?
> 2 weeks most likely.
> 
> I've just sent Greg KH a pull request with the fix in it. He will hopefully
> pick that up and then send a pull request on to Linus.  Then we wait for the
> next rc after that at which point Greg will probably pull it into char-misc-next or
> I can always merge it into my togreg branch once it is in a release candidate of
> Linus' tree.
> 
> In parallel with that I'll probably do a pull request for what is already in the
> togreg tree to get a lot of stuff in char-misc-next for the next cycle. That makes
> the history a little cleaner as I can fast forward my tree and end up with
> whatever is in char-misc-next (hopefully including this).
> 
> Anyhow, a bit of tree juggling for me, but we have plenty of time as rc3 will probably
> be out tomorrow and it normally goes to rc7 at one rc a week

Thank you for the information and for the help during the review process for this patch.
Best regards,
Emil
> 
> Thanks,
> 
> Jonathan
> > Best Regards,
> > Emil 
> > 
> 

Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Jonathan Cameron 1 month, 1 week ago
On Mon, 14 Oct 2024 06:18:29 +0000
Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:

> On Sat, 2024-10-12 at 16:10 +0100, Jonathan Cameron wrote:
> > On Fri, 11 Oct 2024 07:12:05 +0000
> > Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> >   
> > > On Thu, 2024-10-10 at 18:47 +0100, Jonathan Cameron wrote:  
> > > > On Mon, 7 Oct 2024 07:19:06 +0000
> > > > Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> > > >     
> > > > > On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:    
> > > > > > On Thu, 3 Oct 2024 14:22:17 +0200
> > > > > > Emil Gedenryd <emil.gedenryd@axis.com> wrote:      
> > > > > > > 
> > > > > > > +struct opt3001_chip_info {
> > > > > > > +	const struct iio_chan_spec (*channels)[2];
> > > > > > > +	enum iio_chan_type chan_type;
> > > > > > > +	int num_channels;
> > > > > > > +
> > > > > > > +	const struct opt3001_scale (*scales)[12];      
> > > > > > This doesn't compile for me as one of the two options only
> > > > > > has 11 entries.  You could either force them to be 12
> > > > > > entries each or use a pointer without the size and
> > > > > > add a num_scales entry in here.
> > > > > > 
> > > > > > Jonathan      
> > > > > 
> > > > > Hi Jonathan,
> > > > > 
> > > > > Are you building on top of the patch that was accepted in earlier versions of this
> > > > > patch set? That patch adds the twelfth missing scale value for the opt3001.
> > > > > See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/
> > > > > 
> > > > > Should I have added some tag to highlight the dependency for this version of the
> > > > > patch set?    
> > > > Ah.  Yes, I was half asleep.
> > > > They are going via different branches (slow and fast) so I'll have to
> > > > sit on this series until after that fix is in the upstream for the togreg
> > > > branch of iio.git.
> > > > 
> > > > If I seem to have lost it after that is the case feel free to give me a poke.
> > > > 
> > > > Jonathan
> > > >     
> > > Hi,
> > > 
> > > No worries. Just to clarify, do you mean sit on it as that you will continue reviewing
> > > the code after the fix is in upstream, or should I consider this patch to be approved?  
> > Assuming not other review comes in, I consider this ready to go.  
> 
> Great, thank you!
> 
> > > 
> > > Also, do you have an approximation of what time frame we're talking about?  
> > 2 weeks most likely.
> > 
> > I've just sent Greg KH a pull request with the fix in it. He will hopefully
> > pick that up and then send a pull request on to Linus.  Then we wait for the
> > next rc after that at which point Greg will probably pull it into char-misc-next or
> > I can always merge it into my togreg branch once it is in a release candidate of
> > Linus' tree.
> > 
> > In parallel with that I'll probably do a pull request for what is already in the
> > togreg tree to get a lot of stuff in char-misc-next for the next cycle. That makes
> > the history a little cleaner as I can fast forward my tree and end up with
> > whatever is in char-misc-next (hopefully including this).
> > 
> > Anyhow, a bit of tree juggling for me, but we have plenty of time as rc3 will probably
> > be out tomorrow and it normally goes to rc7 at one rc a week  
> 
> Thank you for the information and for the help during the review process for this patch.
> Best regards,
> Emil

Applied to the togreg branch of iio.git and pushed out initially as testing to
let the build bots see if they can find anything we missed.

I'll push it out for linux-next to pick up in a few days.

Jonathan

> > 
> > Thanks,
> > 
> > Jonathan  
> > > Best Regards,
> > > Emil 
> > >   
> >   
> 
Re: [PATCH v4 2/2] iio: light: opt3001: add support for TI's opt3002 light sensor
Posted by Emil Gedenryd 1 month ago
On Mon, 2024-10-21 at 19:28 +0100, Jonathan Cameron wrote:
> On Mon, 14 Oct 2024 06:18:29 +0000
> Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> 
> > On Sat, 2024-10-12 at 16:10 +0100, Jonathan Cameron wrote:
> > > On Fri, 11 Oct 2024 07:12:05 +0000
> > > Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> > >   
> > > > On Thu, 2024-10-10 at 18:47 +0100, Jonathan Cameron wrote:  
> > > > > On Mon, 7 Oct 2024 07:19:06 +0000
> > > > > Emil Gedenryd <Emil.Gedenryd@axis.com> wrote:
> > > > >     
> > > > > > On Sun, 2024-10-06 at 14:16 +0100, Jonathan Cameron wrote:    
> > > > > > > On Thu, 3 Oct 2024 14:22:17 +0200
> > > > > > > Emil Gedenryd <emil.gedenryd@axis.com> wrote:      
> > > > > > > > 
> > > > > > > > +struct opt3001_chip_info {
> > > > > > > > +	const struct iio_chan_spec (*channels)[2];
> > > > > > > > +	enum iio_chan_type chan_type;
> > > > > > > > +	int num_channels;
> > > > > > > > +
> > > > > > > > +	const struct opt3001_scale (*scales)[12];      
> > > > > > > This doesn't compile for me as one of the two options only
> > > > > > > has 11 entries.  You could either force them to be 12
> > > > > > > entries each or use a pointer without the size and
> > > > > > > add a num_scales entry in here.
> > > > > > > 
> > > > > > > Jonathan      
> > > > > > 
> > > > > > Hi Jonathan,
> > > > > > 
> > > > > > Are you building on top of the patch that was accepted in earlier versions of this
> > > > > > patch set? That patch adds the twelfth missing scale value for the opt3001.
> > > > > > See: https://lore.kernel.org/all/20240916-add_opt3002-v3-1-984b190cd68c@axis.com/
> > > > > > 
> > > > > > Should I have added some tag to highlight the dependency for this version of the
> > > > > > patch set?    
> > > > > Ah.  Yes, I was half asleep.
> > > > > They are going via different branches (slow and fast) so I'll have to
> > > > > sit on this series until after that fix is in the upstream for the togreg
> > > > > branch of iio.git.
> > > > > 
> > > > > If I seem to have lost it after that is the case feel free to give me a poke.
> > > > > 
> > > > > Jonathan
> > > > >     
> > > > Hi,
> > > > 
> > > > No worries. Just to clarify, do you mean sit on it as that you will continue reviewing
> > > > the code after the fix is in upstream, or should I consider this patch to be approved?  
> > > Assuming not other review comes in, I consider this ready to go.  
> > 
> > Great, thank you!
> > 
> > > > 
> > > > Also, do you have an approximation of what time frame we're talking about?  
> > > 2 weeks most likely.
> > > 
> > > I've just sent Greg KH a pull request with the fix in it. He will hopefully
> > > pick that up and then send a pull request on to Linus.  Then we wait for the
> > > next rc after that at which point Greg will probably pull it into char-misc-next or
> > > I can always merge it into my togreg branch once it is in a release candidate of
> > > Linus' tree.
> > > 
> > > In parallel with that I'll probably do a pull request for what is already in the
> > > togreg tree to get a lot of stuff in char-misc-next for the next cycle. That makes
> > > the history a little cleaner as I can fast forward my tree and end up with
> > > whatever is in char-misc-next (hopefully including this).
> > > 
> > > Anyhow, a bit of tree juggling for me, but we have plenty of time as rc3 will probably
> > > be out tomorrow and it normally goes to rc7 at one rc a week  
> > 
> > Thank you for the information and for the help during the review process for this patch.
> > Best regards,
> > Emil
> 
> Applied to the togreg branch of iio.git and pushed out initially as testing to
> let the build bots see if they can find anything we missed.
> 
> I'll push it out for linux-next to pick up in a few days.
> 
> Jonathan

Hi Jonathan, thank you for the update!
Best regards,
Emil

> 
> > > 
> > > Thanks,
> > > 
> > > Jonathan  
> > > > Best Regards,
> > > > Emil 
> > > >   
> > >   
> > 
>