Linux IIO development
 help / color / mirror / Atom feed
From: Andy Spencer <aspencer@spacex.com>
To: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Kevin Bosien <kbosien@spacex.com>, Jim Gruen <jgruen@spacex.com>,
	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:41:37 -0700	[thread overview]
Message-ID: <20230905074137.GB10702@spacex.com> (raw)
In-Reply-To: <ZPBa40RHJ93proj0@lore-desk>

Hi Lorenzo,

Thanks for reviewing this!

Regarding the TODR and ODR_T_BATCH settings:

> > +			[ST_LSM6DSX_ID_TEMP] = {
> > +				/*
> > +				 * 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,
> 
> please consider we do not support low-power mode iirc (just
> high-performance - bit 4 in CTRL6_C (15h)), so even enabling accel
> sensor, the temp sensor will always runs at 52Hz. Here we should add
> just one entry, like:
> 
> 				.odr_avl[0] = { 52000, 0x03 },
> 				.odr_len = 1,

I didn't see a way to configure the batch data rate in the IIO driver
aside from the "odr_avl" table.

It seemed useful to allow reading the gyro/accel at a high rate, such as
416 Hz, while still allowing the lower temperature sub-sampling rates of
1.6 and 12.5 Hz. My original intent in adding the lower ODR table
entries here was to reuse the sampling_frequency sysfs attr to
configure the batch data rate, since that seems to be what most people
would care about.

Alternately, we could add a separate "odr_batch_avl" table along with a
separate sysfs attr (e.g. "buffer_sampling_frequency"). That would be a
lot more work though, especially since the actual ODR for the temp
sensor won't be configurable anyway.

Note, I don't have any specific need for the lower rates, so if 52 Hz is
the only rate supported, that still "works for me".

  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 [this message]
2023-09-05  7:00 ` Andy Spencer

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=20230905074137.GB10702@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