> On Mon, 1 Jan 2018 11:32:21 -0800 > Martin Kelly <martin@xxxxxxxxxxxxxxxx> wrote: > >> On 01/01/2018 02:42 AM, Jonathan Cameron wrote: >> > On Sun, 31 Dec 2017 13:06:52 -0800 >> > Martin Kelly <martin@xxxxxxxxxxxxxxxx> wrote: >> > >> >> From: Martin Kelly <martin@xxxxxxxxxxxxxxxx> >> >> >> >> Update the sensor settings to support the LSM9DS1 sensor. >> >> >> >> Signed-off-by: Martin Kelly <martin@xxxxxxxxxxxxxxxx> >> > >> > Hi Martin, >> > >> > I'm rather unsure if we should be adding more gyro only >> > support for these devices. Ths lsm9ds0 was in theory >> > a stop gap whilst support was properly sorted out. >> > >> > Looking back Crestez did post an RFC for each of the >> > parts as well for reference but made the point that this was >> > the wrong way to do things. >> > >> > This part (and the ds0) really need to be supported by a proper gyro/accel >> > driver rather than just cherry picking out the gyro part. >> > >> > We are stuck having to maintain precise backwards compatibility >> > for the ds0 part by the fact it is upstream already. I'm not >> > particularly keen to make things worse. >> > >> > https://patchwork.kernel.org/patch/8824981/ >> > https://marc.info/?t=146057421800001&r=1&w=4 >> > >> > Looking at the datasheet this is a mess of a device. >> > >> > The accel and gyro feed one fifo, the magnetometer only supports >> > direct access. The magnetometer is effectively a separate part >> > so supporting that should be easy enough. >> > >> > Denis, did your rework of the st driver to support these dual >> > devices get anywhere? >> > >> > Anyhow, lets hold this one for now until we hear from Denis or >> > someone else at ST on the current status of supporting these >> > properly. >> > >> >> Sure, I'll wait for Denis. It seems to me we have two possible situations: >> >> (1) A proper combo driver is ready soon, or can be made to work with not >> much more effort than extending the current driver. In this case, I >> should rewrite the patch to use the combo driver, or wait until the >> combo driver is available. I'm not sure if the proper combo driver is >> something Denis is working on or if it's Lorenzo's driver. >> >> (2) The combo driver won't be ready for a while, or it takes a large >> refactor to make it work with the LSM9X combo devices. In this case, it >> seems to me that adding another device should not significantly increase >> maintenance burden, as the main maintenance is in maintaining the >> infrastructure rather than the settings struct for each device itself. >> We still need to solve the need for a proper combo driver, but in the >> meantime (which could be a while), we'll have support for a new device, >> which is good for everyone. > > The issue is not so much code maintenance but rather the fact that any > new driver that supports the combo modes will inherently add some > degree of ABI change to userspace (even if it is just the name of the > device changing). Definitely less than ideal given in theory we > guarantee never to break userspace - however fragile that userspace > is... Honestly I should never have taken the first part that does > this but clearly had a weak moment. > > Jonathan I completely agree, in this case maintain UABI will be a pain :) > >> >> > Jonathan >> > >> >> --- >> >> .../devicetree/bindings/iio/st-sensors.txt | 1 + >> >> drivers/iio/gyro/Kconfig | 3 +- >> >> drivers/iio/gyro/st_gyro.h | 1 + >> >> drivers/iio/gyro/st_gyro_core.c | 95 ++++++++++++++++++++++ >> >> drivers/iio/gyro/st_gyro_i2c.c | 5 ++ >> >> drivers/iio/gyro/st_gyro_spi.c | 5 ++ >> >> 6 files changed, 109 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt >> >> index 6f626f73417e..e3d2b5d5d707 100644 >> >> --- a/Documentation/devicetree/bindings/iio/st-sensors.txt >> >> +++ b/Documentation/devicetree/bindings/iio/st-sensors.txt >> >> @@ -59,6 +59,7 @@ Gyroscopes: >> >> - st,l3g4is-gyro >> >> - st,lsm330-gyro >> >> - st,lsm9ds0-gyro >> >> +- st,lsm9ds1-gyro >> >> >> >> Magnetometers: >> >> - st,lsm303agr-magn >> >> diff --git a/drivers/iio/gyro/Kconfig b/drivers/iio/gyro/Kconfig >> >> index 3126cf05e6b9..0d63c84c5312 100644 >> >> --- a/drivers/iio/gyro/Kconfig >> >> +++ b/drivers/iio/gyro/Kconfig >> >> @@ -111,7 +111,8 @@ config IIO_ST_GYRO_3AXIS >> >> select IIO_TRIGGERED_BUFFER if (IIO_BUFFER) >> >> help >> >> Say yes here to build support for STMicroelectronics gyroscopes: >> >> - L3G4200D, LSM330DL, L3GD20, LSM330DLC, L3G4IS, LSM330, LSM9DS0. >> >> + L3G4200D, LSM330DL, L3GD20, LSM330DLC, L3G4IS, LSM330, LSM9DS0, >> >> + LSM9DS1. >> > >> > Please copy the spaces vs tabs in Kconfig > >> >> Ooops, will do if we go forward with the patch as-is. >> >> >> >> >> This driver can also be built as a module. If so, these modules >> >> will be created: >> >> diff --git a/drivers/iio/gyro/st_gyro.h b/drivers/iio/gyro/st_gyro.h >> >> index 48923ae6ac3b..f89de446d066 100644 >> >> --- a/drivers/iio/gyro/st_gyro.h >> >> +++ b/drivers/iio/gyro/st_gyro.h >> >> @@ -23,6 +23,7 @@ >> >> #define L3G4IS_GYRO_DEV_NAME "l3g4is_ui" >> >> #define LSM330_GYRO_DEV_NAME "lsm330_gyro" >> >> #define LSM9DS0_GYRO_DEV_NAME "lsm9ds0_gyro" >> >> +#define LSM9DS1_GYRO_DEV_NAME "lsm9ds1_gyro" >> >> >> >> /** >> >> * struct st_sensors_platform_data - gyro platform data >> >> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c >> >> index b31064ba37b9..ebfce311e855 100644 >> >> --- a/drivers/iio/gyro/st_gyro_core.c >> >> +++ b/drivers/iio/gyro/st_gyro_core.c >> >> @@ -56,6 +56,23 @@ static const struct iio_chan_spec st_gyro_16bit_channels[] = { >> >> IIO_CHAN_SOFT_TIMESTAMP(3) >> >> }; >> >> >> >> +/* The LSM9DS1 uses different output registers that the other chips. */ >> >> +static const struct iio_chan_spec st_gyro_lsm9ds1_16bit_channels[] = { >> >> + ST_SENSORS_LSM_CHANNELS(IIO_ANGL_VEL, >> >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >> >> + ST_SENSORS_SCAN_X, 1, IIO_MOD_X, 's', IIO_LE, 16, 16, >> >> + 0x18), >> >> + ST_SENSORS_LSM_CHANNELS(IIO_ANGL_VEL, >> >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >> >> + ST_SENSORS_SCAN_Y, 1, IIO_MOD_Y, 's', IIO_LE, 16, 16, >> >> + 0x1a), >> >> + ST_SENSORS_LSM_CHANNELS(IIO_ANGL_VEL, >> >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >> >> + ST_SENSORS_SCAN_Z, 1, IIO_MOD_Z, 's', IIO_LE, 16, 16, >> >> + 0x1c), >> >> + IIO_CHAN_SOFT_TIMESTAMP(3) >> >> +}; >> >> + >> >> static const struct st_sensor_settings st_gyro_sensors_settings[] = { >> >> { >> >> .wai = 0xd3, >> >> @@ -285,6 +302,84 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = { >> >> .multi_read_bit = true, >> >> .bootime = 2, >> >> }, >> >> + { >> >> + .wai = 0x68, >> >> + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, >> >> + .sensors_supported = { >> >> + [0] = LSM9DS1_GYRO_DEV_NAME, >> >> + }, >> >> + .ch = (struct iio_chan_spec *)st_gyro_lsm9ds1_16bit_channels, >> >> + .odr = { >> >> + .addr = 0x10, >> >> + .mask = 0xe0, >> >> + .odr_avl = { >> >> + { .hz = 15, .value = 0x01, }, >> >> + { .hz = 60, .value = 0x02, }, >> >> + { .hz = 119, .value = 0x03, }, >> >> + { .hz = 238, .value = 0x04, }, >> >> + { .hz = 476, .value = 0x05, }, >> >> + { .hz = 952, .value = 0x06, }, >> >> + }, >> >> + }, >> >> + .pw = { >> >> + .addr = 0x10, >> >> + .mask = 0xe0, >> >> + .value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE, >> >> + .value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE, >> >> + }, >> >> + .enable_axis = { >> >> + .addr = 0x1e, >> >> + .mask = 0x38, >> >> + }, >> >> + .fs = { >> >> + .addr = 0x10, >> >> + .mask = 0x38, >> >> + .fs_avl = { >> >> + [0] = { >> >> + .num = ST_GYRO_FS_AVL_250DPS, >> >> + .value = 0x00, >> >> + .gain = IIO_DEGREE_TO_RAD(8750), >> >> + }, >> >> + [1] = { >> >> + .num = ST_GYRO_FS_AVL_500DPS, >> >> + .value = 0x01, >> >> + .gain = IIO_DEGREE_TO_RAD(17500), >> >> + }, >> >> + [2] = { >> >> + .num = ST_GYRO_FS_AVL_2000DPS, >> >> + /* 2 is "Not Available". */ >> >> + .value = 0x03, >> >> + .gain = IIO_DEGREE_TO_RAD(70000), >> >> + }, >> >> + }, >> >> + }, >> >> + .bdu = { >> >> + .addr = 0x22, >> >> + .mask = 0x40, >> >> + }, >> >> + .drdy_irq = { >> >> + .int1 = { >> >> + .addr = 0x0c, >> >> + .mask = 0x02, >> >> + }, >> >> + .int2 = { >> >> + .addr = 0x0d, >> >> + .mask = 0x02, >> >> + }, >> >> + .addr_ihl = 0x22, >> >> + .mask_ihl = 0x20, >> >> + .stat_drdy = { >> >> + .addr = ST_SENSORS_DEFAULT_STAT_ADDR, >> >> + .mask = 0x02, >> >> + }, >> >> + }, >> >> + .sim = { >> >> + .addr = 0x22, >> >> + .value = BIT(3), >> >> + }, >> >> + .multi_read_bit = false, >> >> + .bootime = 2, >> >> + }, >> >> }; >> >> >> >> static int st_gyro_read_raw(struct iio_dev *indio_dev, >> >> diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c >> >> index b405b82b9177..97faa3ddac17 100644 >> >> --- a/drivers/iio/gyro/st_gyro_i2c.c >> >> +++ b/drivers/iio/gyro/st_gyro_i2c.c >> >> @@ -56,6 +56,10 @@ static const struct of_device_id st_gyro_of_match[] = { >> >> .compatible = "st,lsm9ds0-gyro", >> >> .data = LSM9DS0_GYRO_DEV_NAME, >> >> }, >> >> + { >> >> + .compatible = "st,lsm9ds1-gyro", >> >> + .data = LSM9DS1_GYRO_DEV_NAME, >> >> + }, >> >> {}, >> >> }; >> >> MODULE_DEVICE_TABLE(of, st_gyro_of_match); >> >> @@ -104,6 +108,7 @@ static const struct i2c_device_id st_gyro_id_table[] = { >> >> { L3G4IS_GYRO_DEV_NAME }, >> >> { LSM330_GYRO_DEV_NAME }, >> >> { LSM9DS0_GYRO_DEV_NAME }, >> >> + { LSM9DS1_GYRO_DEV_NAME }, >> >> {}, >> >> }; >> >> MODULE_DEVICE_TABLE(i2c, st_gyro_id_table); >> >> diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c >> >> index 0b52ed577dc2..7d4661cc260c 100644 >> >> --- a/drivers/iio/gyro/st_gyro_spi.c >> >> +++ b/drivers/iio/gyro/st_gyro_spi.c >> >> @@ -61,6 +61,10 @@ static const struct of_device_id st_gyro_of_match[] = { >> >> .compatible = "st,lsm9ds0-gyro", >> >> .data = LSM9DS0_GYRO_DEV_NAME, >> >> }, >> >> + { >> >> + .compatible = "st,lsm9ds1-gyro", >> >> + .data = LSM9DS1_GYRO_DEV_NAME, >> >> + }, >> >> {}, >> >> }; >> >> MODULE_DEVICE_TABLE(of, st_gyro_of_match); >> >> @@ -108,6 +112,7 @@ static const struct spi_device_id st_gyro_id_table[] = { >> >> { L3G4IS_GYRO_DEV_NAME }, >> >> { LSM330_GYRO_DEV_NAME }, >> >> { LSM9DS0_GYRO_DEV_NAME }, >> >> + { LSM9DS1_GYRO_DEV_NAME }, >> >> {}, >> >> }; >> >> MODULE_DEVICE_TABLE(spi, st_gyro_id_table); >> > >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
