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

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

[Emil Gedenryd]

TI's opt3002 light-to-digital sensor provides the functionality
of an optical power meter within a single device. It shares a lot of
similarities with their opt3001 model but has a wide spectral bandwidth,
ranging from 300 nm to 1000 nm.

This patch set adds support for the TI opt3002 by extending the opt3001
driver. In addition, a missing full-scale range value for the opt3001 is
added, resulting in higher precision when setting event trigger values.

Datasheet: http://www.ti.com/product/OPT3002

Signed-off-by: Emil Gedenryd <emil.gedenryd@axis.com>
---
Changes in v4:
- Drop applied patch: "iio: light: opt3001: add missing full-scale range value"
- Add num_channels field to opt3001_chip_info
- Use i2c_get_match_data() instead of device_get_match_data()
- Link to v3: https://lore.kernel.org/r/20240916-add_opt3002-v3-0-984b190cd68c@axis.com

Changes in v3:
- Include difference between opt3001 and opt3002 in commit message for
  dt-binding patch.
- Remove whitespace between tags in commit message for opt3002 patch.
- Rename opt300x_chip_info to opt3001_chip_info.
- Add documentation for mathematical constants in opt3001_chip_info.
- Only check return value after opt3001_read_id if chip has id register
  in opt3001_probe.
- Change opt3002 channel mask to IIO_CHAN_INFO_RAW.
- Link to v2: https://lore.kernel.org/r/20240913-add_opt3002-v2-0-69e04f840360@axis.com

Changes in v2:
- Move dt-binding patch to before implementation.
- Fix dt-binding compatible definition.
- Clarify bug description for missing full-scale range value.
- Remove model enum from chip info and all in-function switch-case
  statements.
- Move model-specific channels and mathematic constants to chip info.
- Add valid match data to opt3001_id array
- Skip call to function opt3001_read_id() if model doesn't have a device
  id register.
- Link to v1: https://lore.kernel.org/r/20240905-add_opt3002-v1-0-a5ae21b924fb@axis.com

---
Emil Gedenryd (2):
      dt-bindings: iio: light: opt3001: add compatible for opt3002
      iio: light: opt3001: add support for TI's opt3002 light sensor

 .../devicetree/bindings/iio/light/ti,opt3001.yaml  |   4 +-
 drivers/iio/light/Kconfig                          |   2 +-
 drivers/iio/light/opt3001.c                        | 189 +++++++++++++++++----
 3 files changed, 160 insertions(+), 35 deletions(-)
---
base-commit: e2df62bb0fadc8ae16aa3bb88ee05ee7e037046b
change-id: 20240828-add_opt3002-40552c1a2f77

Best regards,
-- 
Emil Gedenryd <emil.gedenryd@axis.com>

[PATCH v4 1/2] dt-bindings: iio: light: opt3001: add compatible for opt3002

[Emil Gedenryd]

The opt3002 is a Light-to-Digital Sensor by TI with support for wide-range
spectrum light. It shares most properties with their opt3001 model with
the exception of having a wide spectral bandwidth, ranging from 300 nm
to 1000 nm.

Add the compatible string of opt3002.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Emil Gedenryd <emil.gedenryd@axis.com>
---
 Documentation/devicetree/bindings/iio/light/ti,opt3001.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/light/ti,opt3001.yaml b/Documentation/devicetree/bindings/iio/light/ti,opt3001.yaml
index 441e9343fc97..67ca8d08256a 100644
--- a/Documentation/devicetree/bindings/iio/light/ti,opt3001.yaml
+++ b/Documentation/devicetree/bindings/iio/light/ti,opt3001.yaml
@@ -15,7 +15,9 @@ description: |
 
 properties:
   compatible:
-    const: ti,opt3001
+    enum:
+      - ti,opt3001
+      - ti,opt3002
 
   reg:
     maxItems: 1

-- 
2.39.2

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

[Emil Gedenryd]

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

[Jonathan Cameron]

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

[Emil Gedenryd]

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

[Jonathan Cameron]

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

[Emil Gedenryd]

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

[Jonathan Cameron]

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

[Emil Gedenryd]

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

[Jonathan Cameron]

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

[Emil Gedenryd]

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 
> > > >   
> > >   
> > 
>