| Message ID | cover.1679009443.git.mehdi.djait.k@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iio: accel: Add support for Kionix/ROHM KX132 accelerometer | expand |
On Fri, Mar 17, 2023 at 12:48:34AM +0100, Mehdi Djait wrote: > KX132 accelerometer is a sensor which: > - supports G-ranges of (+/-) 2, 4, 8, and 16G > - can be connected to I2C or SPI > - has internal HW FIFO buffer > - supports various ODRs (output data rates) > > The KX132 accelerometer is very similair to the KX022A. > One key difference is number of bits to report the number of data bytes that > have been stored in the sample buffer: 8 bits for KX022A vs 10 bits for KX132. > > A complete list of differences is listed in [1] > [1] https://kionixfs.azureedge.net/en/document/AN112-Transitioning-to-KX132-1211-Accelerometer.pdf1 Is it really the first version of this contribution?
Hi Andy, On Fri, Mar 17, 2023 at 02:07:49PM +0200, Andy Shevchenko wrote: > On Fri, Mar 17, 2023 at 12:48:34AM +0100, Mehdi Djait wrote: > > KX132 accelerometer is a sensor which: > > - supports G-ranges of (+/-) 2, 4, 8, and 16G > > - can be connected to I2C or SPI > > - has internal HW FIFO buffer > > - supports various ODRs (output data rates) > > > > The KX132 accelerometer is very similair to the KX022A. > > One key difference is number of bits to report the number of data bytes that > > have been stored in the sample buffer: 8 bits for KX022A vs 10 bits for KX132. > > > > A complete list of differences is listed in [1] > > [1] https://kionixfs.azureedge.net/en/document/AN112-Transitioning-to-KX132-1211-Accelerometer.pdf1 > > Is it really the first version of this contribution? > Yes this is my first series for the KX132 Support. -- Kind Regards Mehdi Djait
Hi Mehdi, Things have been piling up for me during last two weeks... I will do proper review during next week. On 3/17/23 01:48, Mehdi Djait wrote: > KX132 accelerometer is a sensor which: > - supports G-ranges of (+/-) 2, 4, 8, and 16G > - can be connected to I2C or SPI > - has internal HW FIFO buffer > - supports various ODRs (output data rates) > > The KX132 accelerometer is very similair to the KX022A. > One key difference is number of bits to report the number of data bytes that > have been stored in the sample buffer: 8 bits for KX022A vs 10 bits for KX132. The KX022A has 16bits of data in HiRes mode. This is the default for kx022a driver. > A complete list of differences is listed in [1] > > > [1] https://kionixfs.azureedge.net/en/document/AN112-Transitioning-to-KX132-1211-Accelerometer.pdf1 This document is somewhat misleading. It does not contain KX022A but the older KX022. Kionix has the somewhat confusing habit of having very similar names for models with - occasionally significant - differences. (My own opinion). I the "Technical referene manual" is more interesting document than the data-sheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf I have heard that there have been a few very different versions of KX132 as well. Not sure if they have "leaked" out to public though. In any case, for the kx132 it might be safest to use the full model name - especially in the DT compatibles. Finally, AFAIK the key "thing" in KX132 is the "ADP" (Advanced Data Path) feature which allows filtering the data "in sensor". Unfortunately, I am not really familiar with this feature. Do you think this is something that might get configured only once at start-up depending on the purpose of the board? If yes, this might be something that will end-up having properties in device-tree. If yes, then it might be a good idea to have own binding doc for KX132. Currently it seems Ok to have them in the same binding doc though. Anyways, I'll have proper look at this series during the next week - Thanks for the contribution! Much appreciated! Yours, -- Matti > Mehdi Djait (3): > dt-bindings: iio: Add KX132 accelerometer > iio: accel: kionix-kx022a: Add chip_info structure > iio: accel: Add support for Kionix/ROHM KX132 accelerometer > > .../bindings/iio/accel/kionix,kx022a.yaml | 13 +- > drivers/iio/accel/kionix-kx022a-i2c.c | 21 +- > drivers/iio/accel/kionix-kx022a-spi.c | 24 +- > drivers/iio/accel/kionix-kx022a.c | 413 +++++++++++------- > drivers/iio/accel/kionix-kx022a.h | 181 +++++++- > 5 files changed, 464 insertions(+), 188 deletions(-) >
Hi Matti, On Sun, Mar 19, 2023 at 09:43:19AM +0200, Matti Vaittinen wrote: > Hi Mehdi, > > Things have been piling up for me during last two weeks... I will do proper > review during next week. > > On 3/17/23 01:48, Mehdi Djait wrote: > > KX132 accelerometer is a sensor which: > > - supports G-ranges of (+/-) 2, 4, 8, and 16G > > - can be connected to I2C or SPI > > - has internal HW FIFO buffer > > - supports various ODRs (output data rates) > > > > The KX132 accelerometer is very similair to the KX022A. > > One key difference is number of bits to report the number of data bytes that > > have been stored in the sample buffer: 8 bits for KX022A vs 10 bits for KX132. > > The KX022A has 16bits of data in HiRes mode. This is the default for kx022a > driver. I am talking here about "Buffer Sample Level number of bits": kx022a uses 8 bits: just BUF_STATUS_1 to report the status of the buffer. kx132 uses BUF_STATUS_1 and the Bit0, Bit1 of BUF_STATUS_2. That's the reason for adding the kx022a_get_fifo_bytes function. > > > A complete list of differences is listed in [1] > > > > > > [1] https://kionixfs.azureedge.net/en/document/AN112-Transitioning-to-KX132-1211-Accelerometer.pdf1 > > This document is somewhat misleading. It does not contain KX022A but the > older KX022. Kionix has the somewhat confusing habit of having very similar > names for models with - occasionally significant - differences. (My own > opinion). Yes, I am aware that it does not contain KX022A, but from my understanding of your code, the document can be used to highlight the difference I mentioned > > I the "Technical referene manual" is more interesting document than the > data-sheet: > > https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > > I have heard that there have been a few very different versions of KX132 as > well. Not sure if they have "leaked" out to public though. In any case, for > the kx132 it might be safest to use the full model name - especially in the > DT compatibles. > I will change it to kx132-1211 > Finally, AFAIK the key "thing" in KX132 is the "ADP" (Advanced Data Path) > feature which allows filtering the data "in sensor". Unfortunately, I am not > really familiar with this feature. Do you think this is something that might > get configured only once at start-up depending on the purpose of the board? > If yes, this might be something that will end-up having properties in > device-tree. If yes, then it might be a good idea to have own binding doc > for KX132. Currently it seems Ok to have them in the same binding doc > though. > Correct me if I am wrong: the Devicetree is a description of the hardware and the transitioning document states: "From a hardware perspective, all these sensors have an identical pad/pin layouts and identical pin functionality" I was thinking about adding an advanced_data_path boolean to the chip_info struct and providing different driver callbacks depending on the value. Something like in the bmc150-accel-core: iio_info for bmc150_accel_info and iio_info for bmc150_accel_info_fifo -- Kind Regards Mehdi Djait
On 3/21/23 15:16, Mehdi Djait wrote: > Hi Matti, > > On Sun, Mar 19, 2023 at 09:43:19AM +0200, Matti Vaittinen wrote: >> Hi Mehdi, >> >> Things have been piling up for me during last two weeks... I will do proper >> review during next week. >> >> On 3/17/23 01:48, Mehdi Djait wrote: >>> KX132 accelerometer is a sensor which: >>> - supports G-ranges of (+/-) 2, 4, 8, and 16G >>> - can be connected to I2C or SPI >>> - has internal HW FIFO buffer >>> - supports various ODRs (output data rates) >>> >>> The KX132 accelerometer is very similair to the KX022A. >>> One key difference is number of bits to report the number of data bytes that >>> have been stored in the sample buffer: 8 bits for KX022A vs 10 bits for KX132. >> >> The KX022A has 16bits of data in HiRes mode. This is the default for kx022a >> driver. > > I am talking here about "Buffer Sample Level number of bits": Ah. My bad. I misunderstood. Mentioning the number of bits made me to instantly think of the format of measured data not the size of the FIFO and how many bits are needed to represent the full FIFO. kx022a uses > 8 bits: just BUF_STATUS_1 to report the status of the buffer. kx132 uses > BUF_STATUS_1 and the Bit0, Bit1 of BUF_STATUS_2. > > That's the reason for adding the kx022a_get_fifo_bytes function. > >> >>> A complete list of differences is listed in [1] >>> >>> >>> [1] https://kionixfs.azureedge.net/en/document/AN112-Transitioning-to-KX132-1211-Accelerometer.pdf1 >> >> This document is somewhat misleading. It does not contain KX022A but the >> older KX022. Kionix has the somewhat confusing habit of having very similar >> names for models with - occasionally significant - differences. (My own >> opinion). > > Yes, I am aware that it does not contain KX022A, but from my > understanding of your code, the document can be used to highlight the > difference I mentioned I don't remember all the differences between the KX022 and KX022A - but I believe at least the supported G-ranges were different. I think there probably were also some other things. >> Finally, AFAIK the key "thing" in KX132 is the "ADP" (Advanced Data Path) >> feature which allows filtering the data "in sensor". Unfortunately, I am not >> really familiar with this feature. Do you think this is something that might >> get configured only once at start-up depending on the purpose of the board? >> If yes, this might be something that will end-up having properties in >> device-tree. If yes, then it might be a good idea to have own binding doc >> for KX132. Currently it seems Ok to have them in the same binding doc >> though. >> > > Correct me if I am wrong: the Devicetree is a description of the > hardware Yes. > and the transitioning document states: > > "From a hardware perspective, all these sensors > have an identical pad/pin layouts and identical pin functionality" Sometimes (fixed) hardware _configuration_ can be visible in device-tree. Eg, device-tree can be used to tell: "On this board this configurable piece of hardware shall look like this". My understanding of ADP is very limited, but I thought it may be used to adjust the hardware by for example adding some filters. As I said, I however don't know if the ADP will be used with 'static configurations' which could be seen as hardware properties. Hence I asked if you had insight to this. > I was thinking about adding an advanced_data_path boolean to the chip_info > struct and providing different driver callbacks depending on the value. I can't really comment on this much as I lack of knowledge. I have understood the ADP can be configured to do very different type of filtering(?) I wonder how the boolean flag suits these needs but I guess we can go into those details when ADP support is being implemented. Right now I am happy if you say you have analyzed the ADP to the point you don't expect many kx132 specific dt-bindings to be needed. And. even if this analysis was wrong, we can later separate the kx132 bindings to own doc if that is needed. So, I am happy with the bindings being in same file now. > Something like in the bmc150-accel-core: iio_info for bmc150_accel_info > and iio_info for bmc150_accel_info_fifo I will see those later, thanks. Yours -- Matti