linux-kernel - Re: [PATCH v2] RFC: iio: lsm6dsx: Support temperature channel on some devices

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] [day] [month] [year] [list]

Message-ID: <20230905070010.GA10702@spacex.com>
Date:   Tue, 5 Sep 2023 00:00:10 -0700
From:   Andy Spencer <aspencer@...cex.com>
To:     Linus Walleij <linus.walleij@...aro.org>
CC:     Kevin Bosien <kbosien@...cex.com>, Jim Gruen <jgruen@...cex.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Mario Tesi <mario.tesi@...com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] RFC: iio: lsm6dsx: Support temperature channel on
 some devices

Hi Linus,

Thanks for helping get this into mainline :)

> Changes in v2:
> - Put to RFC because I can't test changes.
> - Added some mail addresses to SpaceX to the header. Maybe you
>   guys can check if this works for you. Or forward to designated
>   open source ambassador or whatever can help. (Addresses found
>   in SpaceX code drop.)

I am familiar with these changes and can help review and test this.

> - Drop the code with strings for ism330dhc as we concluded that
>   this is probably ism330dhcx which is already supported.
>   (Would be nice if SpaceX can confirm.)

I believe we did use the ism330dhc a long time ago but have since
switched to the ism330dhcx. The register maps are compatible, so the old
name stuck around. Adding support for ism330dhcx only is fine.

> - Don't write in nonsense register 0x0a for temperature sensor
> - More elaborate code to just avoid writing ODR for the temperature
>   sensor and instead rely on gyro or accelerometer to drive
>   the odr

OK, we only use this in batch mode, I'll confirm that removing this
write (for non-batch mode) works as expected.

> +				/*
> +				 * NOTE: this ODR will be capped and controllerd by the
> +				 * gyro and accelerometer don't have any reg to configure
> +				 * this ODR.
> +				 */
> +				.odr_avl[0] = {  12500, 0x01 },
> +				.odr_avl[1] = {  26000, 0x02 },
> +				.odr_avl[2] = {  52000, 0x03 },
> +				.odr_len = 3,

As per the other thread, the data rate should be 1600, 12500, and 52000.

> +		/*
> +		 * The temperature sensor has a fixed scale and offset such
> +		 * that: temp_C = (raw / 256) + 25
> +		 */
> +		sensor->gain = 3906;
> +		sensor->offset = 25;

I believe the gain should be set to 3906250 now (1e9 / 256). 3906 was an
approximation from an older kernel that used IIO_VAL_INT_PLUS_MICRO
instead of IIO_VAL_INT_PLUS_NANO.

Also in st_lsm6dsx_alloc_iiodev(), I believe we can/should avoid setting
available_scan_masks, the mask (0x7) isn't really valid for the
temperature sensor that has only one channel.