drivers/iio/light/Kconfig | 2 ++ drivers/iio/light/veml6030.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+)
All devices supported by this driver (currently veml6030, veml6035
and veml7700) have two 16-bit channels, and can profit for the same
configuration to support data access via triggered buffers.
The measurements are stored in two 16-bit consecutive registers
(addresses 0x04 and 0x05) as little endian, unsigned data.
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
drivers/iio/light/Kconfig | 2 ++
drivers/iio/light/veml6030.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+)
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 29ffa8491927..0e2566ff5f7b 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -683,6 +683,8 @@ config VEML3235
config VEML6030
tristate "VEML6030 and VEML6035 ambient light sensors"
select REGMAP_I2C
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
depends on I2C
help
Say Y here if you want to build a driver for the Vishay VEML6030
diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
index ccb43dfd5cf7..d57ae0c4cae3 100644
--- a/drivers/iio/light/veml6030.c
+++ b/drivers/iio/light/veml6030.c
@@ -28,6 +28,8 @@
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
#include <linux/iio/events.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
/* Device registers */
#define VEML6030_REG_ALS_CONF 0x00
@@ -37,6 +39,7 @@
#define VEML6030_REG_ALS_DATA 0x04
#define VEML6030_REG_WH_DATA 0x05
#define VEML6030_REG_ALS_INT 0x06
+#define VEML6030_REG_DATA(ch) (VEML6030_REG_ALS_DATA + (ch))
/* Bit masks for specific functionality */
#define VEML6030_ALS_IT GENMASK(9, 6)
@@ -56,6 +59,18 @@
#define VEML6035_INT_CHAN BIT(3)
#define VEML6035_CHAN_EN BIT(2)
+enum veml6030_scan {
+ VEML6030_SCAN_ALS,
+ VEML6030_SCAN_WH,
+ VEML6030_SCAN_TIMESTAMP,
+};
+
+static const unsigned long veml6030_avail_scan_masks[] = {
+ (BIT(VEML6030_SCAN_ALS) |
+ BIT(VEML6030_SCAN_WH)),
+ 0
+};
+
struct veml603x_chip {
const char *name;
const int(*scale_vals)[][2];
@@ -86,6 +101,10 @@ struct veml6030_data {
int cur_gain;
int cur_integration_time;
const struct veml603x_chip *chip;
+ struct {
+ __le16 chans[2];
+ aligned_s64 timestamp;
+ } scan;
};
static const int veml6030_it_times[][2] = {
@@ -242,6 +261,14 @@ static const struct iio_chan_spec veml6030_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.event_spec = veml6030_event_spec,
.num_event_specs = ARRAY_SIZE(veml6030_event_spec),
+ .scan_index = VEML6030_SCAN_ALS,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .shift = 0,
+ .storagebits = 16,
+ .endianness = IIO_LE,
+ },
},
{
.type = IIO_INTENSITY,
@@ -253,7 +280,16 @@ static const struct iio_chan_spec veml6030_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
BIT(IIO_CHAN_INFO_SCALE),
+ .scan_index = VEML6030_SCAN_WH,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .shift = 0,
+ .storagebits = 16,
+ .endianness = IIO_LE,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(VEML6030_SCAN_TIMESTAMP),
};
static const struct iio_chan_spec veml7700_channels[] = {
@@ -266,6 +302,14 @@ static const struct iio_chan_spec veml7700_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
BIT(IIO_CHAN_INFO_SCALE),
+ .scan_index = VEML6030_SCAN_ALS,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .shift = 0,
+ .storagebits = 16,
+ .endianness = IIO_LE,
+ },
},
{
.type = IIO_INTENSITY,
@@ -277,7 +321,16 @@ static const struct iio_chan_spec veml7700_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
BIT(IIO_CHAN_INFO_SCALE),
+ .scan_index = VEML6030_SCAN_WH,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .shift = 0,
+ .storagebits = 16,
+ .endianness = IIO_LE,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(VEML6030_SCAN_TIMESTAMP),
};
static const struct regmap_config veml6030_regmap_config = {
@@ -889,6 +942,30 @@ static irqreturn_t veml6030_event_handler(int irq, void *private)
return IRQ_HANDLED;
}
+static irqreturn_t veml6030_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *iio = pf->indio_dev;
+ struct veml6030_data *data = iio_priv(iio);
+ int i, ret, reg;
+ int j = 0;
+
+ iio_for_each_active_channel(iio, i) {
+ ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
+ if (ret)
+ goto done;
+
+ data->scan.chans[j++] = reg;
+ }
+
+ iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
+
+done:
+ iio_trigger_notify_done(iio->trig);
+
+ return IRQ_HANDLED;
+}
+
static int veml6030_set_info(struct iio_dev *indio_dev)
{
struct veml6030_data *data = iio_priv(indio_dev);
@@ -1068,6 +1145,7 @@ static int veml6030_probe(struct i2c_client *client)
indio_dev->channels = data->chip->channels;
indio_dev->num_channels = data->chip->num_channels;
indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->available_scan_masks = veml6030_avail_scan_masks;
ret = data->chip->set_info(indio_dev);
if (ret < 0)
@@ -1077,6 +1155,12 @@ static int veml6030_probe(struct i2c_client *client)
if (ret < 0)
return ret;
+ ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
+ veml6030_trigger_handler, NULL);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to register triggered buffer");
+
return devm_iio_device_register(&client->dev, indio_dev);
}
---
base-commit: c9f8285ec18c08fae0de08835eb8e5953339e664
change-id: 20241106-veml6030_triggered_buffer-a38886ca4cce
Best regards,
--
Javier Carrasco <javier.carrasco.cruz@gmail.com>On Thu, 07 Nov 2024 21:22:45 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> All devices supported by this driver (currently veml6030, veml6035
> and veml7700) have two 16-bit channels, and can profit for the same
> configuration to support data access via triggered buffers.
>
> The measurements are stored in two 16-bit consecutive registers
> (addresses 0x04 and 0x05) as little endian, unsigned data.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Hi Javier,
Some comments inline below.
Thanks,
Jonathan
> ---
> drivers/iio/light/Kconfig | 2 ++
> drivers/iio/light/veml6030.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 86 insertions(+)
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 29ffa8491927..0e2566ff5f7b 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -683,6 +683,8 @@ config VEML3235
> config VEML6030
> tristate "VEML6030 and VEML6035 ambient light sensors"
> select REGMAP_I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> depends on I2C
> help
> Say Y here if you want to build a driver for the Vishay VEML6030
> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> index ccb43dfd5cf7..d57ae0c4cae3 100644
> --- a/drivers/iio/light/veml6030.c
> +++ b/drivers/iio/light/veml6030.c
> @@ -28,6 +28,8 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> #include <linux/iio/events.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>
> /* Device registers */
> #define VEML6030_REG_ALS_CONF 0x00
> @@ -37,6 +39,7 @@
> #define VEML6030_REG_ALS_DATA 0x04
> #define VEML6030_REG_WH_DATA 0x05
> #define VEML6030_REG_ALS_INT 0x06
> +#define VEML6030_REG_DATA(ch) (VEML6030_REG_ALS_DATA + (ch))
>
> /* Bit masks for specific functionality */
> #define VEML6030_ALS_IT GENMASK(9, 6)
> @@ -56,6 +59,18 @@
> #define VEML6035_INT_CHAN BIT(3)
> #define VEML6035_CHAN_EN BIT(2)
>
> +enum veml6030_scan {
> + VEML6030_SCAN_ALS,
> + VEML6030_SCAN_WH,
> + VEML6030_SCAN_TIMESTAMP,
> +};
> +
> +static const unsigned long veml6030_avail_scan_masks[] = {
> + (BIT(VEML6030_SCAN_ALS) |
> + BIT(VEML6030_SCAN_WH)),
I'd not wrap the two lines above. Also outer brackets don't add much
so maybe drop them.
> + 0
> +};
> +
> struct veml603x_chip {
> const char *name;
> const int(*scale_vals)[][2];
> @@ -86,6 +101,10 @@ struct veml6030_data {
> int cur_gain;
> int cur_integration_time;
> const struct veml603x_chip *chip;
> + struct {
> + __le16 chans[2];
> + aligned_s64 timestamp;
> + } scan;
This is pretty small and as it's i2c, you don't need to be careful with alignment
(everything is bounce buffered anyway). So you could just have it on the stack
in the trigger_handler function.
> };
>
> static const int veml6030_it_times[][2] = {
> @@ -242,6 +261,14 @@ static const struct iio_chan_spec veml6030_channels[] = {
> BIT(IIO_CHAN_INFO_SCALE),
> .event_spec = veml6030_event_spec,
> .num_event_specs = ARRAY_SIZE(veml6030_event_spec),
> + .scan_index = VEML6030_SCAN_ALS,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .shift = 0,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> },
> {
> .type = IIO_INTENSITY,
> @@ -253,7 +280,16 @@ static const struct iio_chan_spec veml6030_channels[] = {
> BIT(IIO_CHAN_INFO_SCALE),
> .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = VEML6030_SCAN_WH,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .shift = 0,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> },
> + IIO_CHAN_SOFT_TIMESTAMP(VEML6030_SCAN_TIMESTAMP),
> };
>
> static const struct iio_chan_spec veml7700_channels[] = {
> @@ -266,6 +302,14 @@ static const struct iio_chan_spec veml7700_channels[] = {
> BIT(IIO_CHAN_INFO_SCALE),
> .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = VEML6030_SCAN_ALS,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .shift = 0,
Don't bother specifying shift when the value is 0 and obvious.
C spec will deal with setting that to 0 for you.
> + .storagebits = 16,
> + .endianness = IIO_LE,
As per other branch of the thread, seems this should be IIO_CPU
> + },
> },
> {
> .type = IIO_INTENSITY,
> @@ -277,7 +321,16 @@ static const struct iio_chan_spec veml7700_channels[] = {
> BIT(IIO_CHAN_INFO_SCALE),
> .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = VEML6030_SCAN_WH,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .shift = 0,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> },
> + IIO_CHAN_SOFT_TIMESTAMP(VEML6030_SCAN_TIMESTAMP),
> };
>
> static const struct regmap_config veml6030_regmap_config = {
> @@ -889,6 +942,30 @@ static irqreturn_t veml6030_event_handler(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t veml6030_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *iio = pf->indio_dev;
> + struct veml6030_data *data = iio_priv(iio);
> + int i, ret, reg;
> + int j = 0;
> +
> + iio_for_each_active_channel(iio, i) {
Given you've set the available_scan_masks such that all channels are on
or off, you should be able to read them unconditionally.
The IIO core demux code will break them up if the user requested a subset.
If it makes sense to allow individual channels (looks like it here)
then don't provide available_scan_masks.
A bulk read may make sense (I've not checked register values).
> + ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
> + if (ret)
> + goto done;
> +
> + data->scan.chans[j++] = reg;
> + }
> +
> + iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
> +
> +done:
> + iio_trigger_notify_done(iio->trig);
> +
> + return IRQ_HANDLED;
> +}
Hi Jonathan, thanks for your feedback.
On 09/11/2024 14:27, Jonathan Cameron wrote:
> On Thu, 07 Nov 2024 21:22:45 +0100
> Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
>
>> All devices supported by this driver (currently veml6030, veml6035
>> and veml7700) have two 16-bit channels, and can profit for the same
>> configuration to support data access via triggered buffers.
>>
>> The measurements are stored in two 16-bit consecutive registers
>> (addresses 0x04 and 0x05) as little endian, unsigned data.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Hi Javier,
>
> Some comments inline below.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/light/Kconfig | 2 ++
>> drivers/iio/light/veml6030.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 86 insertions(+)
>>
...
>> +static irqreturn_t veml6030_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *iio = pf->indio_dev;
>> + struct veml6030_data *data = iio_priv(iio);
>> + int i, ret, reg;
>> + int j = 0;
>> +
>> + iio_for_each_active_channel(iio, i) {
> Given you've set the available_scan_masks such that all channels are on
> or off, you should be able to read them unconditionally.
> The IIO core demux code will break them up if the user requested a subset.
>
What I wanted to model is that both channels (ALS and WH) should be
accessible, but allowing the user to request a single one, as the ALS
channel is usually more relevant. It seems that in that case I should
leave available_scan_masks as it is, right? I don't want to keep any
channel from being accessible.
> If it makes sense to allow individual channels (looks like it here)
> then don't provide available_scan_masks.
>
> A bulk read may make sense (I've not checked register values).
>
In my interpretation, I thought that I could read a single register if
there is only a single active channel, instead of using regmap_read_bulk
unconditionally. the data registers have consecutive addresses, and a
bulk read is possible, though.
What approach is preferred in this case? Reading and pushing both
channels at once without any loop, letting the IIO core demux do the
rest, or reading only the active channels as it is done here?
>> + ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
>> + if (ret)
>> + goto done;
>> +
>> + data->scan.chans[j++] = reg;
>> + }
>> +
>> + iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
>> +
>> +done:
>> + iio_trigger_notify_done(iio->trig);
>> +
>> + return IRQ_HANDLED;
>> +}
Thanks again and best regards,
Javier Carrasco
On Sat, 9 Nov 2024 15:32:45 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> Hi Jonathan, thanks for your feedback.
>
> On 09/11/2024 14:27, Jonathan Cameron wrote:
> > On Thu, 07 Nov 2024 21:22:45 +0100
> > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> >
> >> All devices supported by this driver (currently veml6030, veml6035
> >> and veml7700) have two 16-bit channels, and can profit for the same
> >> configuration to support data access via triggered buffers.
> >>
> >> The measurements are stored in two 16-bit consecutive registers
> >> (addresses 0x04 and 0x05) as little endian, unsigned data.
> >>
> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> > Hi Javier,
> >
> > Some comments inline below.
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/light/Kconfig | 2 ++
> >> drivers/iio/light/veml6030.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 86 insertions(+)
> >>
>
> ...
>
> >> +static irqreturn_t veml6030_trigger_handler(int irq, void *p)
> >> +{
> >> + struct iio_poll_func *pf = p;
> >> + struct iio_dev *iio = pf->indio_dev;
> >> + struct veml6030_data *data = iio_priv(iio);
> >> + int i, ret, reg;
> >> + int j = 0;
> >> +
> >> + iio_for_each_active_channel(iio, i) {
> > Given you've set the available_scan_masks such that all channels are on
> > or off, you should be able to read them unconditionally.
> > The IIO core demux code will break them up if the user requested a subset.
> >
>
> What I wanted to model is that both channels (ALS and WH) should be
> accessible, but allowing the user to request a single one, as the ALS
> channel is usually more relevant. It seems that in that case I should
> leave available_scan_masks as it is, right? I don't want to keep any
> channel from being accessible.
Ah. No that's not what it is for. If you don't set it at all, any
combination of channels may be enabled. If you set it you are restricting
the choice of what is enabled to particular combinations. In the simple
case this is used when you want to enforce (at the driver level) that
'all' channels are always captured - usually because there is an efficient
bulk channel read.
The IIO core will then pull out the data for the channels the user actually
requested.
In a case where the reads are independent just don't provide available_scan_masks
at all. That matches providing 0x3, 0x2, 0x1, 0x0 here So any combination of
channels.
>
> > If it makes sense to allow individual channels (looks like it here)
> > then don't provide available_scan_masks.
> >
> > A bulk read may make sense (I've not checked register values).
> >
>
> In my interpretation, I thought that I could read a single register if
> there is only a single active channel, instead of using regmap_read_bulk
> unconditionally. the data registers have consecutive addresses, and a
> bulk read is possible, though.
>
> What approach is preferred in this case? Reading and pushing both
> channels at once without any loop, letting the IIO core demux do the
> rest, or reading only the active channels as it is done here?
It depends a bit on the device. People tend not to want one axis of an
accelerometer, but here a single channel is a likely case to optimize
for, so just don't provide available_scan_masks and it will all work I think.
If it's is a significant saving you can always figure out if a bulk
read makes sense in your driver and do it if both channels are enabled.
Probably not worth the complexity here.
Jonathan
>
> >> + ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
> >> + if (ret)
> >> + goto done;
> >> +
> >> + data->scan.chans[j++] = reg;
> >> + }
> >> +
> >> + iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
> >> +
> >> +done:
> >> + iio_trigger_notify_done(iio->trig);
> >> +
> >> + return IRQ_HANDLED;
> >> +}
>
> Thanks again and best regards,
> Javier Carrasco
>
Hi Javier,
kernel test robot noticed the following build warnings:
[auto build test WARNING on c9f8285ec18c08fae0de08835eb8e5953339e664]
url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/iio-light-veml6030-add-support-for-triggered-buffer/20241108-042332
base: c9f8285ec18c08fae0de08835eb8e5953339e664
patch link: https://lore.kernel.org/r/20241107-veml6030_triggered_buffer-v1-1-4810ab86cc56%40gmail.com
patch subject: [PATCH] iio: light: veml6030: add support for triggered buffer
config: i386-randconfig-062-20241108 (https://download.01.org/0day-ci/archive/20241108/202411081703.Ft0YjqcK-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411081703.Ft0YjqcK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411081703.Ft0YjqcK-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/iio/light/veml6030.c:958:39: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 @@ got int [addressable] reg @@
drivers/iio/light/veml6030.c:958:39: sparse: expected restricted __le16
drivers/iio/light/veml6030.c:958:39: sparse: got int [addressable] reg
vim +958 drivers/iio/light/veml6030.c
944
945 static irqreturn_t veml6030_trigger_handler(int irq, void *p)
946 {
947 struct iio_poll_func *pf = p;
948 struct iio_dev *iio = pf->indio_dev;
949 struct veml6030_data *data = iio_priv(iio);
950 int i, ret, reg;
951 int j = 0;
952
953 iio_for_each_active_channel(iio, i) {
954 ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
955 if (ret)
956 goto done;
957
> 958 data->scan.chans[j++] = reg;
959 }
960
961 iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
962
963 done:
964 iio_trigger_notify_done(iio->trig);
965
966 return IRQ_HANDLED;
967 }
968
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 08/11/2024 10:41, kernel test robot wrote:
> Hi Javier,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on c9f8285ec18c08fae0de08835eb8e5953339e664]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/iio-light-veml6030-add-support-for-triggered-buffer/20241108-042332
> base: c9f8285ec18c08fae0de08835eb8e5953339e664
> patch link: https://lore.kernel.org/r/20241107-veml6030_triggered_buffer-v1-1-4810ab86cc56%40gmail.com
> patch subject: [PATCH] iio: light: veml6030: add support for triggered buffer
> config: i386-randconfig-062-20241108 (https://download.01.org/0day-ci/archive/20241108/202411081703.Ft0YjqcK-lkp@intel.com/config)
> compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411081703.Ft0YjqcK-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202411081703.Ft0YjqcK-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
>>> drivers/iio/light/veml6030.c:958:39: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 @@ got int [addressable] reg @@
> drivers/iio/light/veml6030.c:958:39: sparse: expected restricted __le16
> drivers/iio/light/veml6030.c:958:39: sparse: got int [addressable] reg
>
> vim +958 drivers/iio/light/veml6030.c
>
> 944
> 945 static irqreturn_t veml6030_trigger_handler(int irq, void *p)
> 946 {
> 947 struct iio_poll_func *pf = p;
> 948 struct iio_dev *iio = pf->indio_dev;
> 949 struct veml6030_data *data = iio_priv(iio);
> 950 int i, ret, reg;
> 951 int j = 0;
> 952
> 953 iio_for_each_active_channel(iio, i) {
> 954 ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
> 955 if (ret)
> 956 goto done;
> 957
> > 958 data->scan.chans[j++] = reg;
chans is currently declared as __le16 chans[2], but it should be u16
chans[2], as regmap already handles the endianness.
Then the direct assignment does not raise any warnings. When at it, I
will define reg as unsigned int.
> 959 }
> 960
> 961 iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
> 962
> 963 done:
> 964 iio_trigger_notify_done(iio->trig);
> 965
> 966 return IRQ_HANDLED;
> 967 }
> 968
>
Best regards,
Javier Carrasco
On Fri, 8 Nov 2024 11:33:27 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> On 08/11/2024 10:41, kernel test robot wrote:
> > Hi Javier,
> >
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on c9f8285ec18c08fae0de08835eb8e5953339e664]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/iio-light-veml6030-add-support-for-triggered-buffer/20241108-042332
> > base: c9f8285ec18c08fae0de08835eb8e5953339e664
> > patch link: https://lore.kernel.org/r/20241107-veml6030_triggered_buffer-v1-1-4810ab86cc56%40gmail.com
> > patch subject: [PATCH] iio: light: veml6030: add support for triggered buffer
> > config: i386-randconfig-062-20241108 (https://download.01.org/0day-ci/archive/20241108/202411081703.Ft0YjqcK-lkp@intel.com/config)
> > compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411081703.Ft0YjqcK-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202411081703.Ft0YjqcK-lkp@intel.com/
> >
> > sparse warnings: (new ones prefixed by >>)
> >>> drivers/iio/light/veml6030.c:958:39: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 @@ got int [addressable] reg @@
> > drivers/iio/light/veml6030.c:958:39: sparse: expected restricted __le16
> > drivers/iio/light/veml6030.c:958:39: sparse: got int [addressable] reg
> >
> > vim +958 drivers/iio/light/veml6030.c
> >
> > 944
> > 945 static irqreturn_t veml6030_trigger_handler(int irq, void *p)
> > 946 {
> > 947 struct iio_poll_func *pf = p;
> > 948 struct iio_dev *iio = pf->indio_dev;
> > 949 struct veml6030_data *data = iio_priv(iio);
> > 950 int i, ret, reg;
> > 951 int j = 0;
> > 952
> > 953 iio_for_each_active_channel(iio, i) {
> > 954 ret = regmap_read(data->regmap, VEML6030_REG_DATA(i), ®);
> > 955 if (ret)
> > 956 goto done;
> > 957
> > > 958 data->scan.chans[j++] = reg;
>
> chans is currently declared as __le16 chans[2], but it should be u16
> chans[2], as regmap already handles the endianness.
>
> Then the direct assignment does not raise any warnings. When at it, I
> will define reg as unsigned int.
Make sure you update the chan_spec as well to reflect that are CPU endian.
This makes sense because the registers are 16 bit on this device.
>
> > 959 }
> > 960
> > 961 iio_push_to_buffers_with_timestamp(iio, &data->scan, pf->timestamp);
> > 962
> > 963 done:
> > 964 iio_trigger_notify_done(iio->trig);
> > 965
> > 966 return IRQ_HANDLED;
> > 967 }
> > 968
> >
>
> Best regards,
> Javier Carrasco
>
© 2016 - 2024 Red Hat, Inc.