| Message ID | 20220130034441.15474-4-samuel@sholland.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: Silan SC7A20 accelerometer support | expand |
On Sat, 29 Jan 2022 21:44:41 -0600 Samuel Holland <samuel@sholland.org> wrote: > This chip appears to be a clone of the LIS2DH. The new description is a > copy of the LIS2DH's description with a different WAI value. > > Signed-off-by: Samuel Holland <samuel@sholland.org> Hi Samuel, This is nasty as 0x11 is a valid who am I for an LIS3DHH which is a very different device. One request inline for a bit more info in the code about what this device is etc. I've found a Chinese data sheet on one of the datasheet aggregator sites. If you know of a stable location to add a Datasheet: tag to this patch that would also be helpful. Thanks, Jonathan > --- > > drivers/iio/accel/st_accel.h | 2 + > drivers/iio/accel/st_accel_core.c | 79 +++++++++++++++++++++++++++++++ > drivers/iio/accel/st_accel_i2c.c | 5 ++ > 3 files changed, 86 insertions(+) > > diff --git a/drivers/iio/accel/st_accel.h b/drivers/iio/accel/st_accel.h > index 8750dea56fcb..00e056c21bfc 100644 > --- a/drivers/iio/accel/st_accel.h > +++ b/drivers/iio/accel/st_accel.h > @@ -36,6 +36,7 @@ enum st_accel_type { > LIS3DHH, > LIS2DE12, > LIS2HH12, > + SC7A20, > ST_ACCEL_MAX, > }; > > @@ -61,6 +62,7 @@ enum st_accel_type { > #define LIS3DE_ACCEL_DEV_NAME "lis3de" > #define LIS2DE12_ACCEL_DEV_NAME "lis2de12" > #define LIS2HH12_ACCEL_DEV_NAME "lis2hh12" > +#define SC7A20_ACCEL_DEV_NAME "sc7a20" > > #ifdef CONFIG_IIO_BUFFER > int st_accel_allocate_ring(struct iio_dev *indio_dev); > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c > index 31ea19d0ba71..d9aa0ff1922b 100644 > --- a/drivers/iio/accel/st_accel_core.c > +++ b/drivers/iio/accel/st_accel_core.c > @@ -1087,6 +1087,85 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = { > .multi_read_bit = true, > .bootime = 2, > }, > + { Please add a comment here about the fact it seems to be a clone of the LIS2DH despite the wrong WAI. It's worth noting that a good part of the maintenance of this driver is done by ST employees who are probably less than happy with seeing a clone (with bugs) supported. So support is likely to be somewhat best effort / not going to deliberately break support for this part but no means to test it. Hopefully we'll remember to cc you to test anything new added to the driver. Clones aren't always perfect! Jonathan > + .wai = 0x11, > + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, > + .sensors_supported = { > + [0] = SC7A20_ACCEL_DEV_NAME, > + }, > + .ch = (struct iio_chan_spec *)st_accel_12bit_channels, > + .odr = { > + .addr = 0x20, > + .mask = 0xf0, > + .odr_avl = { > + { .hz = 1, .value = 0x01, }, > + { .hz = 10, .value = 0x02, }, > + { .hz = 25, .value = 0x03, }, > + { .hz = 50, .value = 0x04, }, > + { .hz = 100, .value = 0x05, }, > + { .hz = 200, .value = 0x06, }, > + { .hz = 400, .value = 0x07, }, > + { .hz = 1600, .value = 0x08, }, > + }, > + }, > + .pw = { > + .addr = 0x20, > + .mask = 0xf0, > + .value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE, > + }, > + .enable_axis = { > + .addr = ST_SENSORS_DEFAULT_AXIS_ADDR, > + .mask = ST_SENSORS_DEFAULT_AXIS_MASK, > + }, > + .fs = { > + .addr = 0x23, > + .mask = 0x30, > + .fs_avl = { > + [0] = { > + .num = ST_ACCEL_FS_AVL_2G, > + .value = 0x00, > + .gain = IIO_G_TO_M_S_2(1000), > + }, > + [1] = { > + .num = ST_ACCEL_FS_AVL_4G, > + .value = 0x01, > + .gain = IIO_G_TO_M_S_2(2000), > + }, > + [2] = { > + .num = ST_ACCEL_FS_AVL_8G, > + .value = 0x02, > + .gain = IIO_G_TO_M_S_2(4000), > + }, > + [3] = { > + .num = ST_ACCEL_FS_AVL_16G, > + .value = 0x03, > + .gain = IIO_G_TO_M_S_2(12000), > + }, > + }, > + }, > + .bdu = { > + .addr = 0x23, > + .mask = 0x80, > + }, > + .drdy_irq = { > + .int1 = { > + .addr = 0x22, > + .mask = 0x10, > + }, > + .addr_ihl = 0x25, > + .mask_ihl = 0x02, > + .stat_drdy = { > + .addr = ST_SENSORS_DEFAULT_STAT_ADDR, > + .mask = 0x07, > + }, > + }, > + .sim = { > + .addr = 0x23, > + .value = BIT(0), > + }, > + .multi_read_bit = true, > + .bootime = 2, > + }, > }; > > /* Default accel DRDY is available on INT1 pin */ > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c > index c0ce78eebad9..7f5888570e87 100644 > --- a/drivers/iio/accel/st_accel_i2c.c > +++ b/drivers/iio/accel/st_accel_i2c.c > @@ -107,6 +107,10 @@ static const struct of_device_id st_accel_of_match[] = { > .compatible = "st,lis2hh12", > .data = LIS2HH12_ACCEL_DEV_NAME, > }, > + { > + .compatible = "silan,sc7a20", > + .data = SC7A20_ACCEL_DEV_NAME, > + }, > {}, > }; > MODULE_DEVICE_TABLE(of, st_accel_of_match); > @@ -142,6 +146,7 @@ static const struct i2c_device_id st_accel_id_table[] = { > { LIS3DE_ACCEL_DEV_NAME }, > { LIS2DE12_ACCEL_DEV_NAME }, > { LIS2HH12_ACCEL_DEV_NAME }, > + { SC7A20_ACCEL_DEV_NAME }, > {}, > }; > MODULE_DEVICE_TABLE(i2c, st_accel_id_table);
On Sun, Jan 30, 2022 at 3:04 PM Jonathan Cameron <jic23@kernel.org> wrote: > This is nasty as 0x11 is a valid who am I for an LIS3DHH which is a very > different device. The WAI value is just a second line check anyway, the device is probed from the compatible, this is just checked to make sure the device is what it says it is. Annoying, but it will do the job. > It's worth noting that a good part of the maintenance of this driver is > done by ST employees who are probably less than happy with seeing > a clone (with bugs) supported. So support is likely to be somewhat best > effort / not going to deliberately break support for this part but no > means to test it. Hopefully we'll remember to cc you to test anything new added to > the driver. Clones aren't always perfect! Linux began with people running it on IBM PC clones... ;) I would assume good faith, especially with the silicon shortage plaguing us it is perfectly possible that this is a fully licensed copy. I think the two derivatives of the AK8974 magnetometer for example are legally transferred IP, we just don't know the details behind the curtain. Yours, Linus Walleij
diff --git a/drivers/iio/accel/st_accel.h b/drivers/iio/accel/st_accel.h index 8750dea56fcb..00e056c21bfc 100644 --- a/drivers/iio/accel/st_accel.h +++ b/drivers/iio/accel/st_accel.h @@ -36,6 +36,7 @@ enum st_accel_type { LIS3DHH, LIS2DE12, LIS2HH12, + SC7A20, ST_ACCEL_MAX, }; @@ -61,6 +62,7 @@ enum st_accel_type { #define LIS3DE_ACCEL_DEV_NAME "lis3de" #define LIS2DE12_ACCEL_DEV_NAME "lis2de12" #define LIS2HH12_ACCEL_DEV_NAME "lis2hh12" +#define SC7A20_ACCEL_DEV_NAME "sc7a20" #ifdef CONFIG_IIO_BUFFER int st_accel_allocate_ring(struct iio_dev *indio_dev); diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c index 31ea19d0ba71..d9aa0ff1922b 100644 --- a/drivers/iio/accel/st_accel_core.c +++ b/drivers/iio/accel/st_accel_core.c @@ -1087,6 +1087,85 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = { .multi_read_bit = true, .bootime = 2, }, + { + .wai = 0x11, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, + .sensors_supported = { + [0] = SC7A20_ACCEL_DEV_NAME, + }, + .ch = (struct iio_chan_spec *)st_accel_12bit_channels, + .odr = { + .addr = 0x20, + .mask = 0xf0, + .odr_avl = { + { .hz = 1, .value = 0x01, }, + { .hz = 10, .value = 0x02, }, + { .hz = 25, .value = 0x03, }, + { .hz = 50, .value = 0x04, }, + { .hz = 100, .value = 0x05, }, + { .hz = 200, .value = 0x06, }, + { .hz = 400, .value = 0x07, }, + { .hz = 1600, .value = 0x08, }, + }, + }, + .pw = { + .addr = 0x20, + .mask = 0xf0, + .value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE, + }, + .enable_axis = { + .addr = ST_SENSORS_DEFAULT_AXIS_ADDR, + .mask = ST_SENSORS_DEFAULT_AXIS_MASK, + }, + .fs = { + .addr = 0x23, + .mask = 0x30, + .fs_avl = { + [0] = { + .num = ST_ACCEL_FS_AVL_2G, + .value = 0x00, + .gain = IIO_G_TO_M_S_2(1000), + }, + [1] = { + .num = ST_ACCEL_FS_AVL_4G, + .value = 0x01, + .gain = IIO_G_TO_M_S_2(2000), + }, + [2] = { + .num = ST_ACCEL_FS_AVL_8G, + .value = 0x02, + .gain = IIO_G_TO_M_S_2(4000), + }, + [3] = { + .num = ST_ACCEL_FS_AVL_16G, + .value = 0x03, + .gain = IIO_G_TO_M_S_2(12000), + }, + }, + }, + .bdu = { + .addr = 0x23, + .mask = 0x80, + }, + .drdy_irq = { + .int1 = { + .addr = 0x22, + .mask = 0x10, + }, + .addr_ihl = 0x25, + .mask_ihl = 0x02, + .stat_drdy = { + .addr = ST_SENSORS_DEFAULT_STAT_ADDR, + .mask = 0x07, + }, + }, + .sim = { + .addr = 0x23, + .value = BIT(0), + }, + .multi_read_bit = true, + .bootime = 2, + }, }; /* Default accel DRDY is available on INT1 pin */ diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c index c0ce78eebad9..7f5888570e87 100644 --- a/drivers/iio/accel/st_accel_i2c.c +++ b/drivers/iio/accel/st_accel_i2c.c @@ -107,6 +107,10 @@ static const struct of_device_id st_accel_of_match[] = { .compatible = "st,lis2hh12", .data = LIS2HH12_ACCEL_DEV_NAME, }, + { + .compatible = "silan,sc7a20", + .data = SC7A20_ACCEL_DEV_NAME, + }, {}, }; MODULE_DEVICE_TABLE(of, st_accel_of_match); @@ -142,6 +146,7 @@ static const struct i2c_device_id st_accel_id_table[] = { { LIS3DE_ACCEL_DEV_NAME }, { LIS2DE12_ACCEL_DEV_NAME }, { LIS2HH12_ACCEL_DEV_NAME }, + { SC7A20_ACCEL_DEV_NAME }, {}, }; MODULE_DEVICE_TABLE(i2c, st_accel_id_table);
This chip appears to be a clone of the LIS2DH. The new description is a copy of the LIS2DH's description with a different WAI value. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/iio/accel/st_accel.h | 2 + drivers/iio/accel/st_accel_core.c | 79 +++++++++++++++++++++++++++++++ drivers/iio/accel/st_accel_i2c.c | 5 ++ 3 files changed, 86 insertions(+)