diff mbox series

[v3] iio: accel: st_accel: add LIS2DS12

Message ID 20240601192914.141906-1-kauschluss@disroot.org (mailing list archive)
State Changes Requested
Headers show
Series [v3] iio: accel: st_accel: add LIS2DS12 | expand

Commit Message

Kaustabh Chakraborty June 1, 2024, 7:26 p.m. UTC 
The LIS2DS12 accelerometer by STMicroelectronics is mostly compatible
with the LIS2DE12 variant, except the WhoAmI value (0x43).

Define sensor settings for LIS2DS12, and add support in the I2C and
SPI drivers.

Datasheet: https://www.st.com/resource/en/datasheet/lis2ds12.pdf

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v3:
- fix code formatting

v2: https://lore.kernel.org/linux-iio/20240601183233.118397-1-kauschluss@disroot.org/
Changes in v2:
- add SPI support
- link datasheet in commit description

v1: https://lore.kernel.org/linux-iio/20240526083302.87172-1-kauschluss@disroot.org/
---
 drivers/iio/accel/st_accel.h      |  1 +
 drivers/iio/accel/st_accel_core.c | 76 +++++++++++++++++++++++++++++++
 drivers/iio/accel/st_accel_i2c.c  |  5 ++
 drivers/iio/accel/st_accel_spi.c  |  5 ++
 4 files changed, 87 insertions(+)

Comments

Conor Dooley June 1, 2024, 7:49 p.m. UTC | #1 
On Sun, Jun 02, 2024 at 12:56:41AM +0530, Kaustabh Chakraborty wrote:
> diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> index fd3749871121..329a4d6fb2ec 100644
> --- a/drivers/iio/accel/st_accel_i2c.c
> +++ b/drivers/iio/accel/st_accel_i2c.c
> @@ -102,6 +102,10 @@ static const struct of_device_id st_accel_of_match[] = {
>  		.compatible = "st,lis2de12",
>  		.data = LIS2DE12_ACCEL_DEV_NAME,
>  	},
> +	{
> +		.compatible = "st,lis2ds12",
> +		.data = LIS2DS12_ACCEL_DEV_NAME,
> +	},
>  	{
>  		.compatible = "st,lis2hh12",
>  		.data = LIS2HH12_ACCEL_DEV_NAME,

> diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> index f72a24f45322..825adab37105 100644
> --- a/drivers/iio/accel/st_accel_spi.c
> +++ b/drivers/iio/accel/st_accel_spi.c
> @@ -64,6 +64,10 @@ static const struct of_device_id st_accel_of_match[] = {
>  		.compatible = "st,lis2dh12-accel",
>  		.data = LIS2DH12_ACCEL_DEV_NAME,
>  	},
> +	{
> +		.compatible = "st,lis2ds12",
> +		.data = LIS2DS12_ACCEL_DEV_NAME,
> +	},
>  	{
>  		.compatible = "st,lis3l02dq",
>  		.data = LIS3L02DQ_ACCEL_DEV_NAME,

Any new compatibles need to be documented in st,st-sensors.yaml

Thanks,
Conor.
Jonathan Cameron June 2, 2024, 8:54 a.m. UTC | #2 
On Sat, 1 Jun 2024 20:49:25 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Sun, Jun 02, 2024 at 12:56:41AM +0530, Kaustabh Chakraborty wrote:
> > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> > index fd3749871121..329a4d6fb2ec 100644
> > --- a/drivers/iio/accel/st_accel_i2c.c
> > +++ b/drivers/iio/accel/st_accel_i2c.c
> > @@ -102,6 +102,10 @@ static const struct of_device_id st_accel_of_match[] = {
> >  		.compatible = "st,lis2de12",
> >  		.data = LIS2DE12_ACCEL_DEV_NAME,
> >  	},
> > +	{
> > +		.compatible = "st,lis2ds12",
> > +		.data = LIS2DS12_ACCEL_DEV_NAME,
> > +	},
> >  	{
> >  		.compatible = "st,lis2hh12",
> >  		.data = LIS2HH12_ACCEL_DEV_NAME,  
> 
> > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> > index f72a24f45322..825adab37105 100644
> > --- a/drivers/iio/accel/st_accel_spi.c
> > +++ b/drivers/iio/accel/st_accel_spi.c
> > @@ -64,6 +64,10 @@ static const struct of_device_id st_accel_of_match[] = {
> >  		.compatible = "st,lis2dh12-accel",
> >  		.data = LIS2DH12_ACCEL_DEV_NAME,
> >  	},
> > +	{
> > +		.compatible = "st,lis2ds12",
> > +		.data = LIS2DS12_ACCEL_DEV_NAME,
> > +	},
> >  	{
> >  		.compatible = "st,lis3l02dq",
> >  		.data = LIS3L02DQ_ACCEL_DEV_NAME,  
> 
> Any new compatibles need to be documented in st,st-sensors.yaml

At the moment the st_sensors core is doing hard matching against whoami values
which isn't good.  That should ideally be fixed and the binding for this
device should use a fallback compatible if the statement about compatibility
is accurate.

It may just be a case of relaxing the check in st_sensors_verify_id()
to printing a warning not an error message and not returning an error code
(reserving error returns in that function for bus error etc.

That doesn't need to be in this patch though.  Just have the fallback
stuff in the binding and for now we can rely on matching the more
precise compatible.

Jonathan


> 
> Thanks,
> Conor.
>
Conor Dooley June 4, 2024, 5:52 p.m. UTC | #3 
On Sun, Jun 02, 2024 at 09:54:59AM +0100, Jonathan Cameron wrote:
> On Sat, 1 Jun 2024 20:49:25 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Sun, Jun 02, 2024 at 12:56:41AM +0530, Kaustabh Chakraborty wrote:
> > > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> > > index fd3749871121..329a4d6fb2ec 100644
> > > --- a/drivers/iio/accel/st_accel_i2c.c
> > > +++ b/drivers/iio/accel/st_accel_i2c.c
> > > @@ -102,6 +102,10 @@ static const struct of_device_id st_accel_of_match[] = {
> > >  		.compatible = "st,lis2de12",
> > >  		.data = LIS2DE12_ACCEL_DEV_NAME,
> > >  	},
> > > +	{
> > > +		.compatible = "st,lis2ds12",
> > > +		.data = LIS2DS12_ACCEL_DEV_NAME,
> > > +	},
> > >  	{
> > >  		.compatible = "st,lis2hh12",
> > >  		.data = LIS2HH12_ACCEL_DEV_NAME,  
> > 
> > > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> > > index f72a24f45322..825adab37105 100644
> > > --- a/drivers/iio/accel/st_accel_spi.c
> > > +++ b/drivers/iio/accel/st_accel_spi.c
> > > @@ -64,6 +64,10 @@ static const struct of_device_id st_accel_of_match[] = {
> > >  		.compatible = "st,lis2dh12-accel",
> > >  		.data = LIS2DH12_ACCEL_DEV_NAME,
> > >  	},
> > > +	{
> > > +		.compatible = "st,lis2ds12",
> > > +		.data = LIS2DS12_ACCEL_DEV_NAME,
> > > +	},
> > >  	{
> > >  		.compatible = "st,lis3l02dq",
> > >  		.data = LIS3L02DQ_ACCEL_DEV_NAME,  
> > 
> > Any new compatibles need to be documented in st,st-sensors.yaml
> 
> At the moment the st_sensors core is doing hard matching against whoami values
> which isn't good.  That should ideally be fixed and the binding for this
> device should use a fallback compatible if the statement about compatibility
> is accurate.

Ye, at worst, drivers should moan when the whoami value doesn't match...

> It may just be a case of relaxing the check in st_sensors_verify_id()
> to printing a warning not an error message and not returning an error code
> (reserving error returns in that function for bus error etc.

...which seems to be what you suggest here.

> That doesn't need to be in this patch though.  Just have the fallback
> stuff in the binding and for now we can rely on matching the more
> precise compatible.

That seems ideal. At least get the ball rolling and make it more likely
that we'll direct future additions to fallbacks. With things like
sensors (and especially with the st driver) it can be hard for someone
like myself to figure out what is an isn't compatible without digging
through datasheets, and at least I would start asking.
Kaustabh Chakraborty June 6, 2024, 10:12 a.m. UTC | #4 
On 2024-06-02 08:54, Jonathan Cameron wrote:
> On Sat, 1 Jun 2024 20:49:25 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
>> On Sun, Jun 02, 2024 at 12:56:41AM +0530, Kaustabh Chakraborty wrote:
>> > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
>> > index fd3749871121..329a4d6fb2ec 100644
>> > --- a/drivers/iio/accel/st_accel_i2c.c
>> > +++ b/drivers/iio/accel/st_accel_i2c.c
>> > @@ -102,6 +102,10 @@ static const struct of_device_id st_accel_of_match[] = {
>> >  		.compatible = "st,lis2de12",
>> >  		.data = LIS2DE12_ACCEL_DEV_NAME,
>> >  	},
>> > +	{
>> > +		.compatible = "st,lis2ds12",
>> > +		.data = LIS2DS12_ACCEL_DEV_NAME,
>> > +	},
>> >  	{
>> >  		.compatible = "st,lis2hh12",
>> >  		.data = LIS2HH12_ACCEL_DEV_NAME,
>> 
>> > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
>> > index f72a24f45322..825adab37105 100644
>> > --- a/drivers/iio/accel/st_accel_spi.c
>> > +++ b/drivers/iio/accel/st_accel_spi.c
>> > @@ -64,6 +64,10 @@ static const struct of_device_id st_accel_of_match[] = {
>> >  		.compatible = "st,lis2dh12-accel",
>> >  		.data = LIS2DH12_ACCEL_DEV_NAME,
>> >  	},
>> > +	{
>> > +		.compatible = "st,lis2ds12",
>> > +		.data = LIS2DS12_ACCEL_DEV_NAME,
>> > +	},
>> >  	{
>> >  		.compatible = "st,lis3l02dq",
>> >  		.data = LIS3L02DQ_ACCEL_DEV_NAME,
>> 
>> Any new compatibles need to be documented in st,st-sensors.yaml
> 
> At the moment the st_sensors core is doing hard matching against whoami values
> which isn't good.  That should ideally be fixed and the binding for this
> device should use a fallback compatible if the statement about compatibility
> is accurate.

I apologize for not wording the description accurately. By "compatibility",
I mean that the sensor settings of LIS2DE12 (such as the gain values) seem
to be well-suited for LIS2DS12, as per my experimentation. Both devices are
manufactured by ST and have no correlation regarding compatibility whatsoever.
In that case, a fallback compatible isn't required, right?

I'll make sure to rewrite the description more accurately in v4.
 
> It may just be a case of relaxing the check in st_sensors_verify_id()
> to printing a warning not an error message and not returning an error code
> (reserving error returns in that function for bus error etc.

I agree, if you want I may send a patch for that after I'm done with this
one.

> That doesn't need to be in this patch though.  Just have the fallback
> stuff in the binding and for now we can rely on matching the more
> precise compatible.
> 
> Jonathan
> 
> 
>> 
>> Thanks,
>> Conor.
>>
Jonathan Cameron June 6, 2024, 8:05 p.m. UTC | #5 
On Thu, 06 Jun 2024 10:12:11 +0000
kauschluss <kauschluss@disroot.org> wrote:

> On 2024-06-02 08:54, Jonathan Cameron wrote:
> > On Sat, 1 Jun 2024 20:49:25 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> >   
> >> On Sun, Jun 02, 2024 at 12:56:41AM +0530, Kaustabh Chakraborty wrote:  
> >> > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> >> > index fd3749871121..329a4d6fb2ec 100644
> >> > --- a/drivers/iio/accel/st_accel_i2c.c
> >> > +++ b/drivers/iio/accel/st_accel_i2c.c
> >> > @@ -102,6 +102,10 @@ static const struct of_device_id st_accel_of_match[] = {
> >> >  		.compatible = "st,lis2de12",
> >> >  		.data = LIS2DE12_ACCEL_DEV_NAME,
> >> >  	},
> >> > +	{
> >> > +		.compatible = "st,lis2ds12",
> >> > +		.data = LIS2DS12_ACCEL_DEV_NAME,
> >> > +	},
> >> >  	{
> >> >  		.compatible = "st,lis2hh12",
> >> >  		.data = LIS2HH12_ACCEL_DEV_NAME,  
> >>   
> >> > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> >> > index f72a24f45322..825adab37105 100644
> >> > --- a/drivers/iio/accel/st_accel_spi.c
> >> > +++ b/drivers/iio/accel/st_accel_spi.c
> >> > @@ -64,6 +64,10 @@ static const struct of_device_id st_accel_of_match[] = {
> >> >  		.compatible = "st,lis2dh12-accel",
> >> >  		.data = LIS2DH12_ACCEL_DEV_NAME,
> >> >  	},
> >> > +	{
> >> > +		.compatible = "st,lis2ds12",
> >> > +		.data = LIS2DS12_ACCEL_DEV_NAME,
> >> > +	},
> >> >  	{
> >> >  		.compatible = "st,lis3l02dq",
> >> >  		.data = LIS3L02DQ_ACCEL_DEV_NAME,  
> >> 
> >> Any new compatibles need to be documented in st,st-sensors.yaml  
> > 
> > At the moment the st_sensors core is doing hard matching against whoami values
> > which isn't good.  That should ideally be fixed and the binding for this
> > device should use a fallback compatible if the statement about compatibility
> > is accurate.  
> 
> I apologize for not wording the description accurately. By "compatibility",
> I mean that the sensor settings of LIS2DE12 (such as the gain values) seem
> to be well-suited for LIS2DS12, as per my experimentation. Both devices are
> manufactured by ST and have no correlation regarding compatibility whatsoever.
> In that case, a fallback compatible isn't required, right?

If only the Who Am I value prevents it working if you give the compatible
as lisde12 then even though ST rarely if ever identifies it in datasheets, they are
software compatible.  In that case we should allow for a fallback compatible.
+ fix the driver not to fail on the whoami mismatch.
Note we don't care if they have totally different packages as long as
the driver doesn't need to know that.  If they have different numbers
of interrupts though or power supplies, then they aren't compatible and
we shouldn't provide a fallback.

Roughly speaking you have to compare datahsheet sections for pins (not which
but what) and register maps.

That applies even if the current driver will fail to probe (for now)
> 
> I'll make sure to rewrite the description more accurately in v4.
>  
> > It may just be a case of relaxing the check in st_sensors_verify_id()
> > to printing a warning not an error message and not returning an error code
> > (reserving error returns in that function for bus error etc.  
> 
> I agree, if you want I may send a patch for that after I'm done with this
> one.
Thanks,

Jonathan

> 
> > That doesn't need to be in this patch though.  Just have the fallback
> > stuff in the binding and for now we can rely on matching the more
> > precise compatible.
> > 
> > Jonathan
> > 
> >   
> >> 
> >> Thanks,
> >> Conor.
> >>
diff mbox series

Patch

diff --git a/drivers/iio/accel/st_accel.h b/drivers/iio/accel/st_accel.h
index e7525615712b..2659f536cef6 100644
--- a/drivers/iio/accel/st_accel.h
+++ b/drivers/iio/accel/st_accel.h
@@ -35,6 +35,7 @@ 
 #define LIS3DHH_ACCEL_DEV_NAME		"lis3dhh"
 #define LIS3DE_ACCEL_DEV_NAME		"lis3de"
 #define LIS2DE12_ACCEL_DEV_NAME		"lis2de12"
+#define LIS2DS12_ACCEL_DEV_NAME		"lis2ds12"
 #define LIS2HH12_ACCEL_DEV_NAME		"lis2hh12"
 #define LIS302DL_ACCEL_DEV_NAME		"lis302dl"
 #define LSM303C_ACCEL_DEV_NAME		"lsm303c_accel"
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index d2104e14e255..8552faea1913 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -925,6 +925,82 @@  static const struct st_sensor_settings st_accel_sensors_settings[] = {
 		.multi_read_bit = true,
 		.bootime = 2,
 	},
+	{
+		.wai = 0x43,
+		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
+		.sensors_supported = {
+			[0] = LIS2DS12_ACCEL_DEV_NAME,
+		},
+		.ch = (struct iio_chan_spec *)st_accel_8bit_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 = 1620, .value = 0x08, },
+				{ .hz = 5376, .value = 0x09, },
+			},
+		},
+		.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(15600),
+				},
+				[1] = {
+					.num = ST_ACCEL_FS_AVL_4G,
+					.value = 0x01,
+					.gain = IIO_G_TO_M_S_2(31200),
+				},
+				[2] = {
+					.num = ST_ACCEL_FS_AVL_8G,
+					.value = 0x02,
+					.gain = IIO_G_TO_M_S_2(62500),
+				},
+				[3] = {
+					.num = ST_ACCEL_FS_AVL_16G,
+					.value = 0x03,
+					.gain = IIO_G_TO_M_S_2(187500),
+				},
+			},
+		},
+		.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,
+	},
 	{
 		.wai = 0x41,
 		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
index fd3749871121..329a4d6fb2ec 100644
--- a/drivers/iio/accel/st_accel_i2c.c
+++ b/drivers/iio/accel/st_accel_i2c.c
@@ -102,6 +102,10 @@  static const struct of_device_id st_accel_of_match[] = {
 		.compatible = "st,lis2de12",
 		.data = LIS2DE12_ACCEL_DEV_NAME,
 	},
+	{
+		.compatible = "st,lis2ds12",
+		.data = LIS2DS12_ACCEL_DEV_NAME,
+	},
 	{
 		.compatible = "st,lis2hh12",
 		.data = LIS2HH12_ACCEL_DEV_NAME,
@@ -154,6 +158,7 @@  static const struct i2c_device_id st_accel_id_table[] = {
 	{ LIS2DW12_ACCEL_DEV_NAME },
 	{ LIS3DE_ACCEL_DEV_NAME },
 	{ LIS2DE12_ACCEL_DEV_NAME },
+	{ LIS2DS12_ACCEL_DEV_NAME },
 	{ LIS2HH12_ACCEL_DEV_NAME },
 	{ LIS302DL_ACCEL_DEV_NAME },
 	{ LSM303C_ACCEL_DEV_NAME },
diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
index f72a24f45322..825adab37105 100644
--- a/drivers/iio/accel/st_accel_spi.c
+++ b/drivers/iio/accel/st_accel_spi.c
@@ -64,6 +64,10 @@  static const struct of_device_id st_accel_of_match[] = {
 		.compatible = "st,lis2dh12-accel",
 		.data = LIS2DH12_ACCEL_DEV_NAME,
 	},
+	{
+		.compatible = "st,lis2ds12",
+		.data = LIS2DS12_ACCEL_DEV_NAME,
+	},
 	{
 		.compatible = "st,lis3l02dq",
 		.data = LIS3L02DQ_ACCEL_DEV_NAME,
@@ -151,6 +155,7 @@  static const struct spi_device_id st_accel_id_table[] = {
 	{ LSM330_ACCEL_DEV_NAME },
 	{ LSM303AGR_ACCEL_DEV_NAME },
 	{ LIS2DH12_ACCEL_DEV_NAME },
+	{ LIS2DS12_ACCEL_DEV_NAME },
 	{ LIS3L02DQ_ACCEL_DEV_NAME },
 	{ LNG2DM_ACCEL_DEV_NAME },
 	{ H3LIS331DL_ACCEL_DEV_NAME },