Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: Martin Kepplinger <martin.kepplinger@puri.sm>,
	lorenzo.bianconi83@gmail.com, knaack.h@gmx.de, lars@metafoo.de,
	pmeerw@pmeerw.net, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
Date: Mon, 5 Aug 2019 14:44:38 +0100
Message-ID: <20190805144438.133867b2@archlinux> (raw)
In-Reply-To: <20190728175234.GA7652@lore-desk-wlan.lan>

On Sun, 28 Jul 2019 19:52:34 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> > On Sun, 28 Jul 2019 08:04:51 +0200
> > Martin Kepplinger <martin.kepplinger@puri.sm> wrote:
> >   
> > > On 27.07.19 19:48, Jonathan Cameron wrote:  
> > > > On Thu, 25 Jul 2019 07:31:31 +0200
> > > > Martin Kepplinger <martin.kepplinger@puri.sm> wrote:
> > > >     
> > > >> The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer (separately
> > > >> supported in iio/magnetometer/st_magn*) are located on a separate i2c addresses
> > > >> on the bus.
> > > >>
> > > >> For the datasheet, see https://www.st.com/resource/en/datasheet/lsm9ds1.pdf
> > > >>
> > > >> Treat it just like the LSM6* devices and, despite it's name, hook it up
> > > >> to the st_lsm6dsx driver, using it's basic functionality.
> > > >>
> > > >> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>    
> > > > I'm a little confused on this hardware.
> > > > 
> > > > How does buffered output work if these are independently clocked?
> > > > 
> > > > I took a quick look at the datasheet, and 'suspect' the answer is that
> > > > it runs at the gyro frequencies if both are enable. Is that right?
> > > >     
> > > 
> > > Thanks for reviewing, Jonathan,
> > > 
> > > Correct. It says so in chapter 7.12. But that's a "problem" with all
> > > these imu devices, not specific to this addition right?  
> > It's not a problem as such, but there is a related difference in this
> > device to the others supported by this driver.
> > 
> > The other parts seem to allow for independent data rate setting, with
> > streaming to the buffer that isn't in 'lock step'.  I.e you can get
> > 
> > Ax_1, Ay_1, Az_1, Gx_1, Gy_1, Gz_1, Gx_2, Gy_2, Gz_2, Ax_2, Ay_2, Az_2, Gy_3...  
> 
> correct
> 
> > 
> > That required us to split them up into two devices and means that, to fuse
> > data from these two source, userspace has to do the harder job of
> > aligning the two datasets.
> > 
> > For this device, things are simpler in that you always a 'scan' that goes
> > across both accelerometer and gyroscope channels.  That allows us to
> > represent it as a single IIO device with a single buffer.
> > 
> > I'm not seeing any reference in the lsm9ds1 to the pattern registers
> > that are used to handle difference in frequency for the other
> > parts by letting us know what is actually present in each data set
> > in the fifo.
> > 
> > Now, that doesn't meant we can't still handle them separately given
> > we already do that for other parts.  
> 
> what about reusing st_lsm6dsx_read_fifo() for lsm6ds0/lsm9ds1 but setting hw->sip to:
> - hw->sip = 1 (acc_sip = 1, gyro_sip = 0) when just the acc is enabled
> - hw->sip = 2 (acc_sip = 1, gyro_sip = 1) when both devices are enabled
> 
> I guess it is just a matter of adding a 'bool fixed_pattern' in
> st_lsm6dsx_settings. What do you think?

If I understand this, the intent is still to split it to two separate
IIO devices?  I don't really mind if that's the case and it seems like something
similar to what you describe should work for that.

If we go this way, some comments should be added to explain that whilst
we could have handled this as a single IIO device, we have split it to
be more consistent with the other more flexible devices.

Thanks,

Jonathan

> 
> Regards,
> Lorenzo
> 
> > 
> > Anyhow, is my understanding correct?
> > 
> > Jonathan
> >   
> > > 
> > > Sidenote: I thought about renaming things to "lsm6ds0" here just because
> > > of the name and because the registers are (almost) the same as for my
> > > lsm9ds1. But I'm not a fan of blindly doing that without being able to
> > > test. When the current patchset looks good to you, let's keep it that way.
> > > 
> > >                             martin  
> >   


  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-25  5:31 [PATCH v3 0/5] iio: imu: st_lsm6dsx: Add support for LSM9DS1 Martin Kepplinger
2019-07-25  5:31 ` [PATCH v3 1/5] iio: imu: st_lsm6dsx: move odr_table in st_lsm6dsx_sensor_settings Martin Kepplinger
2019-07-25  5:31 ` [PATCH v3 2/5] iio: imu: st_lsm6dsx: move fs_table " Martin Kepplinger
2019-07-25  5:31 ` [PATCH v3 3/5] iio: imu: st_lsm6sdx: move register definitions to sensor_settings struct Martin Kepplinger
2019-07-25  5:31 ` [PATCH v3 4/5] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1 Martin Kepplinger
     [not found]   ` <CAA2SeNK-pjkSWVsrv5dehEJbakZqR0oPF3DQ5mzhiiOD3Npxkg@mail.gmail.com>
2019-07-25  8:14     ` Martin Kepplinger
     [not found]       ` <CAA2SeN+xZGykSos=j6rFGM1cJBVEV5BTYSpoKYPWTLKFnEjKtw@mail.gmail.com>
2019-07-27  8:51         ` Martin Kepplinger
2019-07-27 17:48   ` Jonathan Cameron
2019-07-28  6:04     ` Martin Kepplinger
2019-07-28  8:34       ` Jonathan Cameron
2019-07-28 17:52         ` Lorenzo Bianconi
2019-08-05 13:44           ` Jonathan Cameron [this message]
2019-07-25  5:31 ` [PATCH v3 5/5] dt-bindings: iio: imu: st_lsm6dsx: add lsm9ds1 device bindings Martin Kepplinger

Reply instructions:

You may reply publically 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=20190805144438.133867b2@archlinux \
    --to=jic23@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.bianconi83@gmail.com \
    --cc=lorenzo@kernel.org \
    --cc=martin.kepplinger@puri.sm \
    --cc=pmeerw@pmeerw.net \
    /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

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org linux-iio@archiver.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox