Re: [PATCH v3] iio: accel: st_accel: add LIS2DS12

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2630 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]