| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<AM8PR10MB4721BAD5BD78B8FD0F5C9798CD57A@AM8PR10MB4721.EURPRD10.PROD.OUTLOOK.COM>
Date: Tue, 15 Jul 2025 18:35:48 +0000
From: "Shen Jianping (ME-SE/EAD2)" <Jianping.Shen@...bosch.com>
To: Jonathan Cameron <jic23@...nel.org>
CC: "lars@...afoo.de" <lars@...afoo.de>, "robh@...nel.org" <robh@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
<conor+dt@...nel.org>, "dima.fedrau@...il.com" <dima.fedrau@...il.com>,
"marcelo.schmitt1@...il.com" <marcelo.schmitt1@...il.com>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Lorenz
Christian (ME-SE/EAD2)" <Christian.Lorenz3@...bosch.com>, "Frauendorf Ulrike
(ME/PJ-SW3)" <Ulrike.Frauendorf@...bosch.com>, "Dolde Kai (ME-SE/PAE-A3)"
<Kai.Dolde@...bosch.com>
Subject: AW: [PATCH v3 2/2] iio: imu: smi330: Add driver
Hi Jonathan,
as you suggested we just set available_scan_masks = { SMI330_ALL_CHAN_MSK, 0 }; and push the whole buffer every time.
We enable a subset (3 channels) from user space. This time the channel data is correct in iio buffer, nevertheless invalid timestamp.
See test result inline
Best Regards
Jianping
>>
>> >> +
>> >> +static irqreturn_t smi330_trigger_handler(int irq, void *p) {
>> >> + struct iio_poll_func *pf = p;
>> >> + struct iio_dev *indio_dev = pf->indio_dev;
>> >> + struct smi330_data *data = iio_priv(indio_dev);
>> >> + int ret, chan;
>> >> + int i = 0;
>> >> +
>> >> + ret = regmap_bulk_read(data->regmap, SMI330_ACCEL_X_REG, data-
>> >>buf,
>> >> + ARRAY_SIZE(smi330_channels));
>> >> + if (ret)
>> >> + goto out;
>> >> +
>> >> + if (*indio_dev->active_scan_mask != SMI330_ALL_CHAN_MSK) {
>> >> + iio_for_each_active_channel(indio_dev, chan)
>> >> + data->buf[i++] = data->buf[chan];
>> >
>> >If I follow this correctly you are reading all the channels and just
>> >copying out the ones you want. Just let the IIO core do that for you
>> >by setting iio_dev-
>> >>available_scan_masks = { SMI330_ALL_CHAN_MSK, 0 }; and push the
>> >>whole
>> >buffer every time.
>>
>> For the most frequent use cases, we define available_scan_masks = {
>SMI330_ALL_CHAN_MSK, SMI330_ACC_XYZ_MSK, SMI330_GYRO_XYZ_MSK,
>0 }; and push the whole buffer every time.
>> From the user space we just enable 3 channels gyro_x, gyro_y, and gyro_z.
>Then we enable buffer and expect that only the gyro values and timestamp in
>iio_buffer. Nevertheless, we have 3 accelerometer values and the timestamp in
>iio_buffer.
>
>> It seems that the iio core does not take care which channel is enabled, just
>copy the first 3 values (acc x,y,z) into iio_buffer. Our driver code still needs to
>take care and just copy the enabled channel value to buffer.
>
>Look again at how it works. If you provide ACC_XYZ_MSK, then your driver has
>to handle it.
>available_scan_masks is saying what your driver supports. The driver can check
>active_scan_mask to find out what is enabled. So right option here is only {
>SMI330_ALL_CHAN_MSK, 0, } In that case the driver never needs to check as
>there is only one option.
>
>Then if any subset of channels is enabled the IIO core copy out just the data
>that is relevant.
>
>
>>
>> Another side effect after using available_scan_masks is that the
>active_scan_masks sometimes does not reflect current channel activation
>status.
>>
>> Is some step missing to properly use available_scan_masks ? How can a user
>find out from user space which channel combination is defined in
>available_scan_masks ?
>
>Why would userspace want to? Userspace requested a subset of channels and
>it gets that subset. So it if asks for the channels that make up
>SMI330_ACC_XYZ_MSK, if available_scan_mask == { SMI330_ALL_CHAN_MSK,
>0 } then the IIO core handling selects SMI330_ALL_CHAN_MSK (smallest
>available mask that is superset of what we asked for) and sets
>active_scan_mask to that. The driver follows what active_scan_mask specifies
>and passes all channel data via the iio_push_to_buffers*() call. The demux in
>the IIO core than takes that 'scan' and repacks it so that userspace receives just
>the data it asked for formatting exactly as the driver would have done it if you
>had handled each channels separately in the driver.
>
Set available_scan_masks = { SMI330_ALL_CHAN_MSK, 0 } and push the whole buffer. iio_push_to_buffers_with_timestamp (indio_dev, data->buf, pf->timestamp);
We enable the accX, accY, and accZ from userspace. And expect 3 acc values and the timestamp in iio buffer.
Raw iio buffer data:
00000000: 3c00 d6ff 7510 0000 6100 f3ff 0000 0000 <...u...a.......
00000010: 3f00 d2ff 8910 0000 0300 f6ff 0000 0000 ?...............
00000020: 4900 dcff 7a10 0000 caff 0100 0000 0000 I...z...........
00000030: 4c00 d9ff 7910 0000 2f00 f8ff 0000 0000 L...y.../.......
00000040: 4b00 d9ff 8410 0000 1f00 0800 0000 0000 K...............
00000050: 4700 daff 7f10 0000 3b00 eeff 0000 0000 G.......;.......
00000060: 3f00 d8ff 8410 0000 0c00 0900 0000 0000 ?...............
00000070: 4600 d9ff 8010 0000 0e00 0800 0000 0000 F...............
00000080: 4700 d7ff 7d10 0000 3400 feff 0000 0000 G...}...4.......
00000090: 4b00 d4ff 8010 0000 3e00 1200 0000 0000 K.......>.......
000000a0: 4600 d6ff 8d10 0000 4300 0000 0000 0000 F.......C.......
000000b0: 4900 d6ff 7710 0000 2500 f0ff 0000 0000 I...w...%.......
Converted value
0.015625 -0.009277 1.024411 589929
0.015869 -0.009521 1.040769 4294901719
0.020508 -0.008301 1.025632 458712
0.018799 -0.006836 1.032956 851960
0.019287 -0.009521 1.033201 4294836275
0.015625 -0.010498 1.031003 4293328982
0.015137 -0.010498 1.031980 4293853176
0.015869 -0.009521 1.031492 4293722141
0.018555 -0.011475 1.033445 4294311886
The 3 acc values is correct in buffer. Nevertheless, invalid timestamp. The timestamp is actually the value of the gyroscope, which directly followed by acc values.
If we enable the gyroX, gyroY, and gyroZ from userspace, then all the data is correct. Since the gyro values are the last 3 values and flowed by timestamp.
Conclusion: Setting available_scan_masks = { SMI330_ALL_CHAN_MSK, 0 }, the iio core is able to correct handle the enabled channel data, but not the timestamp.
The working solution for now is that our driver takes care and just copys the enabled channel value to buffer without using available_scan_masks.
>So the aim is that userspace never knows anything about this. Just set what
>channels you want and get that data.
>
>Jonathan
>
>
>>
>> >
>> >The handling the core code is reasonably sophisticated and will use
>> >bulk copying where appropriate.
>> >
>> >If there is a strong reason to not use that, add a comment here so we
>> >don't have anyone 'fix' this code in future.
>> >
>> >> + }
>> >> +
>> >> + iio_push_to_buffers_with_timestamp(indio_dev, data->buf,
>> >> +pf->timestamp);
>> >> +
>> >> +out:
>> >> + iio_trigger_notify_done(indio_dev->trig);
>> >> +
>> >> + return IRQ_HANDLED;
>> >> +}
>>
Powered by blists - more mailing lists