On 10/25/18 2:13 PM, Denis CIOCCA wrote:
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 supportFrom: 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
Ah, I didn't notice that, thanks! The one difference between the two is that LIS3MDL doesn't specify .bdu, but I'll add that in as a separate patch in v2.
+ }, + .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.
Yeah, I misread the datasheet as supporting only SPI for the magnetometer. I tested with I2C and it's working now in my v2 patch.
Regards, Lorenzo2.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
