> On 01/02/2018 04:30 AM, Jonathan Cameron wrote: >> >> 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 >> > > OK, makes sense. It sounds like if I want to get the LSM9DS1 supported, I > should help out with the combo driver work and get it supported properly > with accel+gyro. Is that right? I have a couple of trivial st_lsm6dsx queued patches that would help Martin to add LSM9DS{0,1} support to st_lsm6dsx (they are not related to LSM9DS{0,1}, just modify the driver to be more general respect to FIFO parsing). I will send them as soon as regmap patchset has been reviewed. Regards, Lorenzo > > >>> >>>> 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 >> >> >> -- >> 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
