* [PATCH 1/3] iio: light: veml6035: fix read_avail in no_irq case for veml6035
2024-10-07 20:36 [PATCH 0/3] iio: light: veml6030: add support for veml7700 Javier Carrasco
@ 2024-10-07 20:36 ` Javier Carrasco
2024-10-07 20:36 ` [PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700 Javier Carrasco
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Javier Carrasco @ 2024-10-07 20:36 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Rishi Gupta
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree,
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] iio: light: veml6030: add support for veml7700
2024-10-07 20:36 [PATCH 0/3] iio: light: veml6030: add support for veml7700 Javier Carrasco
2024-10-07 20:36 ` [PATCH 1/3] iio: light: veml6035: fix read_avail in no_irq case for veml6035 Javier Carrasco
2024-10-07 20:36 ` [PATCH 2/3] dt-bindings: iio: light: veml6030: add veml7700 Javier Carrasco
@ 2024-10-07 20:36 ` Javier Carrasco
2024-10-12 14:11 ` [PATCH 0/3] " Jonathan Cameron
3 siblings, 0 replies; 9+ messages in thread
From: Javier Carrasco @ 2024-10-07 20:36 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Rishi Gupta
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree,
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] iio: light: veml6030: add support for veml7700
2024-10-07 20:36 [PATCH 0/3] iio: light: veml6030: add support for veml7700 Javier Carrasco
` (2 preceding siblings ...)
2024-10-07 20:36 ` [PATCH 3/3] iio: light: veml6030: add support for veml7700 Javier Carrasco
@ 2024-10-12 14:11 ` Jonathan Cameron
3 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2024-10-12 14:11 UTC (permalink / raw)
To: Javier Carrasco
Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Rishi Gupta, Jonathan Cameron, linux-iio,
linux-kernel, devicetree
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,
^ permalink raw reply [flat|nested] 9+ messages in thread