Linux IIO development
 help / color / mirror / Atom feed
From: Andy Spencer <aspencer@spacex.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Kevin Bosien <kbosien@spacex.com>, Jim Gruen <jgruen@spacex.com>,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Mario Tesi <mario.tesi@st.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	<linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] RFC: iio: lsm6dsx: Support temperature channel on some devices
Date: Tue, 5 Sep 2023 00:00:10 -0700	[thread overview]
Message-ID: <20230905070010.GA10702@spacex.com> (raw)
In-Reply-To: <20230829-iio-spacex-lsm6ds0-v2-1-584e161b612f@linaro.org>

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.

      parent reply	other threads:[~2023-09-05 16:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-29  7:44 [PATCH v2] RFC: iio: lsm6dsx: Support temperature channel on some devices Linus Walleij
2023-08-31  9:18 ` Lorenzo Bianconi
2023-09-05  7:41   ` Andy Spencer
2023-09-05  7:00 ` Andy Spencer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230905070010.GA10702@spacex.com \
    --to=aspencer@spacex.com \
    --cc=jgruen@spacex.com \
    --cc=jic23@kernel.org \
    --cc=kbosien@spacex.com \
    --cc=lars@metafoo.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=mario.tesi@st.com \
    --cc=miquel.raynal@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox