Hi Martin, Lorenzo,
Inline my comments.
-----Original Message-----
From: Lorenzo Bianconi <lorenzo.bianconi83@xxxxxxxxx>
Sent: Thursday, October 25, 2018 1:17 AM
To: Martin Kelly <martin@xxxxxxxxxxxxxxxx>
Cc: linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Denis CIOCCA <denis.ciocca@xxxxxx>; Jonathan Cameron <jic23@xxxxxxxxxx>; robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx
Subject: Re: [PATCH 1/2] iio:magnetometer: st_magn: add LSM9DS1 support
>
> From: Martin Kelly <martin@xxxxxxxxxxxxxxxx>
>
> Update the sensor settings to support the LSM9DS1 sensor. Although the
> LSM9DS1 accelerometer and gyroscope are coupled together to use the
> same FIFO, the magnetometer is separate and can be cleanly supported
> without refactoring the existing driver.
>
> Signed-off-by: Martin Kelly <martin@xxxxxxxxxxxxxxxx>
> ---
> drivers/iio/magnetometer/st_magn.h | 1 +
> drivers/iio/magnetometer/st_magn_core.c | 68
> +++++++++++++++++++++++++++++++++
> drivers/iio/magnetometer/st_magn_spi.c | 5 +++
> 3 files changed, 74 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/st_magn.h
> b/drivers/iio/magnetometer/st_magn.h
> index 8fe51ce427bd..3a4abcd1f106 100644
> --- a/drivers/iio/magnetometer/st_magn.h
> +++ b/drivers/iio/magnetometer/st_magn.h
> @@ -20,6 +20,7 @@
> #define LIS3MDL_MAGN_DEV_NAME "lis3mdl"
> #define LSM303AGR_MAGN_DEV_NAME "lsm303agr_magn"
> #define LIS2MDL_MAGN_DEV_NAME "lis2mdl"
> +#define LSM9DS1_MAGN_DEV_NAME "lsm9ds1"
It should be "lsm9ds1_magn" since lsm9ds1 identify A+G+M. As you can see in lsm303xxx series.
>
> int st_magn_common_probe(struct iio_dev *indio_dev); void
> st_magn_common_remove(struct iio_dev *indio_dev); diff --git
> a/drivers/iio/magnetometer/st_magn_core.c
> b/drivers/iio/magnetometer/st_magn_core.c
> index 72f6d1335a04..dfbdeb428467 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -378,6 +378,74 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = {
> .multi_read_bit = false,
> .bootime = 2,
> },
> + {
> + .wai = 0x3d,
> + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
> + .sensors_supported = {
> + [0] = LSM9DS1_MAGN_DEV_NAME,
according to the following register map I guess we can simply add lsm9ds1-magn device name in lis3mdl sensors_supported list
Agree with Lorenzo
> + },
> + .ch = (struct iio_chan_spec *)st_magn_2_16bit_channels,
> + .odr = {
> + /* Fast ODR mode currently not supported. */
> + .addr = 0x20,
> + .mask = 0x1c,
> + .odr_avl = {
> + { .hz = 5, .value = 0x03 },
> + { .hz = 10, .value = 0x04 },
> + { .hz = 20, .value = 0x05 },
> + { .hz = 40, .value = 0x06 },
> + { .hz = 80, .value = 0x07 },
> + },
> + },
> + .pw = {
> + .addr = 0x22,
> + .mask = 0x03,
> + .value_on = 0x00,
> + .value_off = 0x03,
> + },
> + .fs = {
> + .addr = 0x21,
> + .mask = 0x60,
> + .fs_avl = {
> + [0] = {
> + .num = ST_MAGN_FS_AVL_4000MG,
> + .value = 0x00,
> + .gain = 140,
> + },
> + [1] = {
> + .num = ST_MAGN_FS_AVL_8000MG,
> + .value = 0x01,
> + .gain = 290,
> + },
> + [2] = {
> + .num = ST_MAGN_FS_AVL_12000MG,
> + .value = 0x02,
> + .gain = 430,
> + },
> + [3] = {
> + .num = ST_MAGN_FS_AVL_16000MG,
> + .value = 0x03,
> + .gain = 580,
> + },
> + },
> + },
> + .bdu = {
> + .addr = 0x24,
> + .mask = 0x40,
> + },
> + .drdy_irq = {
> + .stat_drdy = {
> + .addr = ST_SENSORS_DEFAULT_STAT_ADDR,
> + .mask = 0x07,
> + },
> + },
> + .sim = {
> + .addr = 0x22,
> + .value = BIT(2),
> + },
> + .multi_read_bit = true,
> + .bootime = 2,
> + },
> };
>
> static int st_magn_read_raw(struct iio_dev *indio_dev, diff --git
> a/drivers/iio/magnetometer/st_magn_spi.c
> b/drivers/iio/magnetometer/st_magn_spi.c
> index 7b7cd08fcc32..433456920673 100644
> --- a/drivers/iio/magnetometer/st_magn_spi.c
> +++ b/drivers/iio/magnetometer/st_magn_spi.c
> @@ -37,6 +37,10 @@ static const struct of_device_id st_magn_of_match[] = {
> .compatible = "st,lis2mdl",
> .data = LIS2MDL_MAGN_DEV_NAME,
> },
> + {
> + .compatible = "st,lsm9ds1",
As suggested from Lorenzo, it should be "st,lsm9ds1-magn"
> + .data = LSM9DS1_MAGN_DEV_NAME,
> + },
> {}
> };
> MODULE_DEVICE_TABLE(of, st_magn_of_match); @@ -79,6 +83,7 @@ static
> const struct spi_device_id st_magn_id_table[] = {
> { LIS3MDL_MAGN_DEV_NAME },
> { LSM303AGR_MAGN_DEV_NAME },
> { LIS2MDL_MAGN_DEV_NAME },
> + { LSM9DS1_MAGN_DEV_NAME },
> {},
> };
> MODULE_DEVICE_TABLE(spi, st_magn_id_table);
> --
I guess you missed the i2c counterpart.
Regards,
Lorenzo
> 2.11.0
>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch; unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp; umount; make clean; sleep
