[PATCH 0/3] iio: light: veml6030: add support for veml7700

[PATCH 0/3] iio: light: veml6030: add support for veml7700

[Javier Carrasco]

This series adds support for the veml7700 ALS sensor, which is basically
a vem6030 within a different package, with no pins for the interrupt and
the I2C address. The changes introduced are meant to hide the event
functionality in that case, while borrowing the rest from the veml6030.

In theory, the interrupt functionality would still be available as all
the registers are the same, and some polling could be done to read the
threshold indicators to generate events. I did not find examples in iio
where the INIT_DELAYED_WORK() queue_delayed_work() mechanism is used for
that (some drivers do it to read results), so I am not sure if that
would be the desired approach. I am open for discussions about that, but
probably to be applied later on.

While testing this "no_irq" device, I noticed that the veml6035 is still
using dedicated structs for the iio_info, which were there to account
for the device-specific attribute values before read_avail() was
introduced in the driver in later versions of the patch series, and they
managed to survive until v3 was applied.
Once read_avail() was introduced, the device-specific structs were not
required anymore, and they are repetitive. Moreover, the initialization
of the no_irq iio_info for the veml6035 was not updated to account for
the new read_avail(), which is a bug if no irq is provided, as there is
no callback to retrieve the available values.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Javier Carrasco (3):
      iio: light: veml6035: fix read_avail in no_irq case for veml6035
      dt-bindings: iio: light: veml6030: add veml7700
      iio: light: veml6030: add support for veml7700

 .../bindings/iio/light/vishay,veml6030.yaml        |  16 ++-
 drivers/iio/light/veml6030.c                       | 130 ++++++++++++++-------
 2 files changed, 106 insertions(+), 40 deletions(-)
---
base-commit: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a
change-id: 20241007-veml7700-83f54cf94262

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH 1/3] iio: light: veml6035: fix read_avail in no_irq case for veml6035

[Javier Carrasco]

The iio_info is identical for veml6030 and veml6035. Moreover,
veml6035_info_no_irq is missing the initialization of the read_avail
member, which is actually a bug if no irq is provided.

Instead of adding the missing initialization, remove the device-specific
iio_info and use the existing one for the veml6030.

Fixes: ccc26bd7d7d7 ("iio: light: veml6030: add support for veml6035")
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/iio/light/veml6030.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
index a5deae333554..ca0379945b1c 100644
--- a/drivers/iio/light/veml6030.c
+++ b/drivers/iio/light/veml6030.c
@@ -56,8 +56,6 @@ struct veml603x_chip {
 	const char *name;
 	const int(*scale_vals)[][2];
 	const int num_scale_vals;
-	const struct iio_info *info;
-	const struct iio_info *info_no_irq;
 	int (*hw_init)(struct iio_dev *indio_dev, struct device *dev);
 	int (*set_als_gain)(struct iio_dev *indio_dev, int val, int val2);
 	int (*get_als_gain)(struct iio_dev *indio_dev, int *val, int *val2);
@@ -829,28 +827,12 @@ static const struct iio_info veml6030_info = {
 	.event_attrs = &veml6030_event_attr_group,
 };
 
-static const struct iio_info veml6035_info = {
-	.read_raw  = veml6030_read_raw,
-	.read_avail  = veml6030_read_avail,
-	.write_raw = veml6030_write_raw,
-	.read_event_value = veml6030_read_event_val,
-	.write_event_value = veml6030_write_event_val,
-	.read_event_config = veml6030_read_interrupt_config,
-	.write_event_config = veml6030_write_interrupt_config,
-	.event_attrs = &veml6030_event_attr_group,
-};
-
 static const struct iio_info veml6030_info_no_irq = {
 	.read_raw  = veml6030_read_raw,
 	.read_avail  = veml6030_read_avail,
 	.write_raw = veml6030_write_raw,
 };
 
-static const struct iio_info veml6035_info_no_irq = {
-	.read_raw  = veml6030_read_raw,
-	.write_raw = veml6030_write_raw,
-};
-
 static irqreturn_t veml6030_event_handler(int irq, void *private)
 {
 	int ret, reg, evtdir;
@@ -1039,9 +1021,9 @@ static int veml6030_probe(struct i2c_client *client)
 					     "irq %d request failed\n",
 					     client->irq);
 
-		indio_dev->info = data->chip->info;
+		indio_dev->info = &veml6030_info;
 	} else {
-		indio_dev->info = data->chip->info_no_irq;
+		indio_dev->info = &veml6030_info_no_irq;
 	}
 
 	ret = data->chip->hw_init(indio_dev, &client->dev);
@@ -1084,8 +1066,6 @@ static const struct veml603x_chip veml6030_chip = {
 	.name = "veml6030",
 	.scale_vals = &veml6030_scale_vals,
 	.num_scale_vals = ARRAY_SIZE(veml6030_scale_vals),
-	.info = &veml6030_info,
-	.info_no_irq = &veml6030_info_no_irq,
 	.hw_init = veml6030_hw_init,
 	.set_als_gain = veml6030_set_als_gain,
 	.get_als_gain = veml6030_get_als_gain,
@@ -1095,8 +1075,6 @@ static const struct veml603x_chip veml6035_chip = {
 	.name = "veml6035",
 	.scale_vals = &veml6035_scale_vals,
 	.num_scale_vals = ARRAY_SIZE(veml6035_scale_vals),
-	.info = &veml6035_info,
-	.info_no_irq = &veml6035_info_no_irq,
 	.hw_init = veml6035_hw_init,
 	.set_als_gain = veml6035_set_als_gain,
 	.get_als_gain = veml6035_get_als_gain,

-- 
2.43.0

[PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700

[Javier Carrasco]

The veml7700 contains the same chip as the veml6030 in a different
package with no interrupt line and no pin to select the I2C address,
which makes it suitable to be supported by the same bindings.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 .../devicetree/bindings/iio/light/vishay,veml6030.yaml   | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
index 6218273b0e86..53b55575efd3 100644
--- a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
+++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/light/vishay,veml6030.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: VEML6030 and VEML6035 Ambient Light Sensors (ALS)
+title: VEML6030, VEML6035 and VEML7700 Ambient Light Sensors (ALS)
 
 maintainers:
   - Rishi Gupta <gupt21@gmail.com>
@@ -22,12 +22,14 @@ description: |
   Specifications about the sensors can be found at:
     https://www.vishay.com/docs/84366/veml6030.pdf
     https://www.vishay.com/docs/84889/veml6035.pdf
+    https://www.vishay.com/docs/84286/veml7700.pdf
 
 properties:
   compatible:
     enum:
       - vishay,veml6030
       - vishay,veml6035
+      - vishay,veml7700
 
   reg:
     maxItems: 1
@@ -70,6 +72,18 @@ allOf:
           enum:
             - 0x29
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - vishay,veml7700
+    then:
+      properties:
+        reg:
+          enum:
+            - 0x10
+        interrupts: false
+
 additionalProperties: false
 
 examples:

-- 
2.43.0

[PATCH 3/3] iio: light: veml6030: add support for veml7700

[Javier Carrasco]

The veml7700 contains the same sensor as the veml6030 in a different
package with no interrupt line and no pin to select the I2C address.

To handle the lack of the interrupt line and profit from the existing
support for the veml6030, add a specific iio_chan_spec with no
(num_)event_spec(s), and register the device's info from the
veml6030_info_no_irq struct.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/iio/light/veml6030.c | 108 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 91 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
index ca0379945b1c..173c85a05a8d 100644
--- a/drivers/iio/light/veml6030.c
+++ b/drivers/iio/light/veml6030.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * VEML6030 and VMEL6035 Ambient Light Sensors
+ * VEML6030, VMEL6035 and VEML7700 Ambient Light Sensors
  *
  * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
  *
@@ -11,6 +11,10 @@
  * VEML6035:
  * Datasheet: https://www.vishay.com/docs/84889/veml6035.pdf
  * Appnote-84944: https://www.vishay.com/docs/84944/designingveml6035.pdf
+ *
+ * VEML7700:
+ * Datasheet: https://www.vishay.com/docs/84286/veml7700.pdf
+ * Appnote-84323: https://www.vishay.com/docs/84323/designingveml7700.pdf
  */
 
 #include <linux/bitfield.h>
@@ -56,7 +60,10 @@ struct veml603x_chip {
 	const char *name;
 	const int(*scale_vals)[][2];
 	const int num_scale_vals;
+	const struct iio_chan_spec *channels;
+	const int num_channels;
 	int (*hw_init)(struct iio_dev *indio_dev, struct device *dev);
+	int (*set_info)(struct iio_dev *indio_dev);
 	int (*set_als_gain)(struct iio_dev *indio_dev, int val, int val2);
 	int (*get_als_gain)(struct iio_dev *indio_dev, int *val, int *val2);
 };
@@ -250,6 +257,30 @@ static const struct iio_chan_spec veml6030_channels[] = {
 	},
 };
 
+static const struct iio_chan_spec veml7700_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.channel = CH_ALS,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_PROCESSED) |
+				BIT(IIO_CHAN_INFO_INT_TIME) |
+				BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
+						     BIT(IIO_CHAN_INFO_SCALE),
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel = CH_WHITE,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_BOTH,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_INT_TIME) |
+				BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
+						     BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
 static const struct regmap_config veml6030_regmap_config = {
 	.name = "veml6030_regmap",
 	.reg_bits = 8,
@@ -862,6 +893,37 @@ static irqreturn_t veml6030_event_handler(int irq, void *private)
 	return IRQ_HANDLED;
 }
 
+static int veml6030_set_info(struct iio_dev *indio_dev)
+{
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+	int ret;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, veml6030_event_handler,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						indio_dev->name, indio_dev);
+		if (ret < 0)
+			return dev_err_probe(&client->dev, ret,
+					     "irq %d request failed\n",
+					     client->irq);
+
+		indio_dev->info = &veml6030_info;
+	} else {
+		indio_dev->info = &veml6030_info_no_irq;
+	}
+
+	return 0;
+}
+
+static int veml7700_set_info(struct iio_dev *indio_dev)
+{
+	indio_dev->info = &veml6030_info_no_irq;
+
+	return 0;
+}
+
 /*
  * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
  * persistence to 1 x integration time and the threshold
@@ -1007,24 +1069,13 @@ static int veml6030_probe(struct i2c_client *client)
 		return -EINVAL;
 
 	indio_dev->name = data->chip->name;
-	indio_dev->channels = veml6030_channels;
-	indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
+	indio_dev->channels = data->chip->channels;
+	indio_dev->num_channels = data->chip->num_channels;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	if (client->irq) {
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
-						NULL, veml6030_event_handler,
-						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-						indio_dev->name, indio_dev);
-		if (ret < 0)
-			return dev_err_probe(&client->dev, ret,
-					     "irq %d request failed\n",
-					     client->irq);
-
-		indio_dev->info = &veml6030_info;
-	} else {
-		indio_dev->info = &veml6030_info_no_irq;
-	}
+	ret = data->chip->set_info(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	ret = data->chip->hw_init(indio_dev, &client->dev);
 	if (ret < 0)
@@ -1066,7 +1117,10 @@ static const struct veml603x_chip veml6030_chip = {
 	.name = "veml6030",
 	.scale_vals = &veml6030_scale_vals,
 	.num_scale_vals = ARRAY_SIZE(veml6030_scale_vals),
+	.channels = veml6030_channels,
+	.num_channels = ARRAY_SIZE(veml6030_channels),
 	.hw_init = veml6030_hw_init,
+	.set_info = veml6030_set_info,
 	.set_als_gain = veml6030_set_als_gain,
 	.get_als_gain = veml6030_get_als_gain,
 };
@@ -1075,11 +1129,26 @@ static const struct veml603x_chip veml6035_chip = {
 	.name = "veml6035",
 	.scale_vals = &veml6035_scale_vals,
 	.num_scale_vals = ARRAY_SIZE(veml6035_scale_vals),
+	.channels = veml6030_channels,
+	.num_channels = ARRAY_SIZE(veml6030_channels),
 	.hw_init = veml6035_hw_init,
+	.set_info = veml6030_set_info,
 	.set_als_gain = veml6035_set_als_gain,
 	.get_als_gain = veml6035_get_als_gain,
 };
 
+static const struct veml603x_chip veml7700_chip = {
+	.name = "veml7700",
+	.scale_vals = &veml6030_scale_vals,
+	.num_scale_vals = ARRAY_SIZE(veml6030_scale_vals),
+	.channels = veml7700_channels,
+	.num_channels = ARRAY_SIZE(veml7700_channels),
+	.hw_init = veml6030_hw_init,
+	.set_info = veml7700_set_info,
+	.set_als_gain = veml6030_set_als_gain,
+	.get_als_gain = veml6030_get_als_gain,
+};
+
 static const struct of_device_id veml6030_of_match[] = {
 	{
 		.compatible = "vishay,veml6030",
@@ -1089,6 +1158,10 @@ static const struct of_device_id veml6030_of_match[] = {
 		.compatible = "vishay,veml6035",
 		.data = &veml6035_chip,
 	},
+	{
+		.compatible = "vishay,veml7700",
+		.data = &veml7700_chip,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, veml6030_of_match);
@@ -1096,6 +1169,7 @@ MODULE_DEVICE_TABLE(of, veml6030_of_match);
 static const struct i2c_device_id veml6030_id[] = {
 	{ "veml6030", (kernel_ulong_t)&veml6030_chip},
 	{ "veml6035", (kernel_ulong_t)&veml6035_chip},
+	{ "veml7700", (kernel_ulong_t)&veml7700_chip},
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, veml6030_id);

-- 
2.43.0

Re: [PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700

[Krzysztof Kozlowski]

On Mon, Oct 07, 2024 at 10:36:37PM +0200, Javier Carrasco wrote:
> The veml7700 contains the same chip as the veml6030 in a different
> package with no interrupt line and no pin to select the I2C address,
> which makes it suitable to be supported by the same bindings.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../devicetree/bindings/iio/light/vishay,veml6030.yaml   | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
> index 6218273b0e86..53b55575efd3 100644
> --- a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml

There is no such file in next.

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700

[Krzysztof Kozlowski]

On 08/10/2024 09:56, Krzysztof Kozlowski wrote:
> On Mon, Oct 07, 2024 at 10:36:37PM +0200, Javier Carrasco wrote:
>> The veml7700 contains the same chip as the veml6030 in a different
>> package with no interrupt line and no pin to select the I2C address,
>> which makes it suitable to be supported by the same bindings.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>> ---
>>  .../devicetree/bindings/iio/light/vishay,veml6030.yaml   | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
>> index 6218273b0e86..53b55575efd3 100644
>> --- a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
> 
> There is no such file in next.

There was no in next from 4th of October, but I found it now. However
the diff chunks still do not fit. It seems you are sending multiple,
related patches. Organize your entire work in one patchset, so big
picture will be visible and any dependencies will be clear.

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700

[Javier Carrasco]

On 08/10/2024 10:01, Krzysztof Kozlowski wrote:
> On 08/10/2024 09:56, Krzysztof Kozlowski wrote:
>> On Mon, Oct 07, 2024 at 10:36:37PM +0200, Javier Carrasco wrote:
>>> The veml7700 contains the same chip as the veml6030 in a different
>>> package with no interrupt line and no pin to select the I2C address,
>>> which makes it suitable to be supported by the same bindings.
>>>
>>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>>> ---
>>>  .../devicetree/bindings/iio/light/vishay,veml6030.yaml   | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
>>> index 6218273b0e86..53b55575efd3 100644
>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
>>
>> There is no such file in next.
> 
> There was no in next from 4th of October, but I found it now. However
> the diff chunks still do not fit. It seems you are sending multiple,
> related patches. Organize your entire work in one patchset, so big
> picture will be visible and any dependencies will be clear.
> 
> Best regards,
> Krzysztof
> 


Hi Krzysztof,

this series uses iio/testing as its base, as it requires patches that
were applied to that branch. Apparently, they are not available in next,
and that is the reason why the diff chunks don't fit.

b4 automatically provides the base-id in the cover later, but I
understand that you are not expecting a commit-id from the iio tree as
the base to apply this patch. Sorry for that, I will rebase onto next as
soon as the required patches are there as well.

Best regards,
Javier Carrasco

Re: [PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700

[Krzysztof Kozlowski]

On Mon, Oct 07, 2024 at 10:36:37PM +0200, Javier Carrasco wrote:
> The veml7700 contains the same chip as the veml6030 in a different
> package with no interrupt line and no pin to select the I2C address,
> which makes it suitable to be supported by the same bindings.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  .../devicetree/bindings/iio/light/vishay,veml6030.yaml   | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 0/3] iio: light: veml6030: add support for veml7700

[Jonathan Cameron]

On Mon, 07 Oct 2024 22:36:35 +0200
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> This series adds support for the veml7700 ALS sensor, which is basically
> a vem6030 within a different package, with no pins for the interrupt and
> the I2C address. The changes introduced are meant to hide the event
> functionality in that case, while borrowing the rest from the veml6030.
> 
> In theory, the interrupt functionality would still be available as all
> the registers are the same, and some polling could be done to read the
> threshold indicators to generate events. I did not find examples in iio
> where the INIT_DELAYED_WORK() queue_delayed_work() mechanism is used for
> that (some drivers do it to read results), so I am not sure if that
> would be the desired approach. I am open for discussions about that, but
> probably to be applied later on.
> 
> While testing this "no_irq" device, I noticed that the veml6035 is still
> using dedicated structs for the iio_info, which were there to account
> for the device-specific attribute values before read_avail() was
> introduced in the driver in later versions of the patch series, and they
> managed to survive until v3 was applied.
> Once read_avail() was introduced, the device-specific structs were not
> required anymore, and they are repetitive. Moreover, the initialization
> of the no_irq iio_info for the veml6035 was not updated to account for
> the new read_avail(), which is a bug if no irq is provided, as there is
> no callback to retrieve the available values.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Series applied to the to togreg branch of iio.git. Pushed out as testing for 
0-day to take a first look at it.

Jonathan

> ---
> Javier Carrasco (3):
>       iio: light: veml6035: fix read_avail in no_irq case for veml6035
>       dt-bindings: iio: light: veml6030: add veml7700
>       iio: light: veml6030: add support for veml7700
> 
>  .../bindings/iio/light/vishay,veml6030.yaml        |  16 ++-
>  drivers/iio/light/veml6030.c                       | 130 ++++++++++++++-------
>  2 files changed, 106 insertions(+), 40 deletions(-)
> ---
> base-commit: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a
> change-id: 20241007-veml7700-83f54cf94262
> 
> Best regards,