linux-kernel - AW: [PATCH v3 2/2] iio: imu: smi330: Add driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <AM8PR10MB47217D838CA7DDACBE162D15CD49A@AM8PR10MB4721.EURPRD10.PROD.OUTLOOK.COM>
Date: Wed, 9 Jul 2025 19:38:18 +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,

"available_scan_masks" works not as expected.  We test it using kernel version v6.16. see the 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.

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 ?

>
>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;
>> +}