linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: accel: mxc4005: add support for mxc6655
@ 2020-05-29 20:05 Christian Oder
  2020-05-31 10:30 ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Christian Oder @ 2020-05-29 20:05 UTC (permalink / raw)
  Cc: myself5, Christian Oder, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Thomas Gleixner,
	Allison Randal, Greg Kroah-Hartman, Chuhong Yuan, linux-iio,
	linux-kernel

The mxc6655 is fully working with the existing mxc4005 driver.
Add support for it.

Signed-off-by: Christian Oder <me@myself5.de>
---
 drivers/iio/accel/mxc4005.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
index 3d5bea651923..3b8614352cb4 100644
--- a/drivers/iio/accel/mxc4005.c
+++ b/drivers/iio/accel/mxc4005.c
@@ -474,12 +474,14 @@ static int mxc4005_probe(struct i2c_client *client,
 
 static const struct acpi_device_id mxc4005_acpi_match[] = {
 	{"MXC4005",	0},
+	{"MXC6655",	0},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, mxc4005_acpi_match);
 
 static const struct i2c_device_id mxc4005_id[] = {
 	{"mxc4005",	0},
+	{"mxc6655",	0},
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, mxc4005_id);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: accel: mxc4005: add support for mxc6655
  2020-05-29 20:05 [PATCH] iio: accel: mxc4005: add support for mxc6655 Christian Oder
@ 2020-05-31 10:30 ` Jonathan Cameron
  2020-05-31 13:16   ` Christian Oder
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2020-05-31 10:30 UTC (permalink / raw)
  To: Christian Oder
  Cc: myself5, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Thomas Gleixner, Allison Randal,
	Greg Kroah-Hartman, Chuhong Yuan, linux-iio, linux-kernel

On Fri, 29 May 2020 22:05:49 +0200
Christian Oder <me@myself5.de> wrote:

> The mxc6655 is fully working with the existing mxc4005 driver.
> Add support for it.
> 
> Signed-off-by: Christian Oder <me@myself5.de>

One query on ACPI bindings.  What is there already may
be missleading :(


> ---
>  drivers/iio/accel/mxc4005.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
> index 3d5bea651923..3b8614352cb4 100644
> --- a/drivers/iio/accel/mxc4005.c
> +++ b/drivers/iio/accel/mxc4005.c
> @@ -474,12 +474,14 @@ static int mxc4005_probe(struct i2c_client *client,
>  
>  static const struct acpi_device_id mxc4005_acpi_match[] = {
>  	{"MXC4005",	0},
> +	{"MXC6655",	0},

Do we have a reference for these ACPI bindings?  While they may seem
obvious, memsic don't have a registered PNP or ACPI ID that I can
find.  If these are in the wild (i.e. in shipping firmware) then we
can take them as defacto bindings, otherwise we should avoid making
them so by putting them in the driver.

Quite a few similar bindings got in a while back that I should have
noticed, but I wasn't so familiar with ACPI back then.  Some
scrubbing of these has gone on recently, but there are lots still
left in IIO.

If we aren't sure, then drop the ACPI addition and just leave the 
i2c one below.  Adding an explicit DT binding table would also be
good if that is method you are using to probe this (or PRP0001
from acpi which uses the dt bindings table)

Thanks,

Jonathan


>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, mxc4005_acpi_match);
>  
>  static const struct i2c_device_id mxc4005_id[] = {
>  	{"mxc4005",	0},
> +	{"mxc6655",	0},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, mxc4005_id);


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: accel: mxc4005: add support for mxc6655
  2020-05-31 10:30 ` Jonathan Cameron
@ 2020-05-31 13:16   ` Christian Oder
  2020-05-31 13:52     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Christian Oder @ 2020-05-31 13:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Christian Oder, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Thomas Gleixner, Allison Randal,
	Greg Kroah-Hartman, Chuhong Yuan, linux-iio, linux-kernel

Hi Jonathan,

I tested the sensor on a Chuwi Hi10 X and only went by what I've seen in other
commits before[1].

I just ran another test to see what entry is necessary and it appears the sensor
still works when removing the i2c entry, but is not working anymore when
removing the ACPI match. I got the ACPI IDs from udevadm info -e[2].
Would that mean, that I should remove the i2c entry given it's working fine
with ACPI on its own then, or am I missing something?

I'm also successfully using the ACPI ID for the touchscreen orientation quirk
in systemd[3].

> Adding an explicit DT binding table would also be
> good if that is method you are using to probe this (or PRP0001
> from acpi which uses the dt bindings table)

Frankly, I have no idea how to do that or if that would still be required when
using ACPI. Can you point me in a rough direction in case it's still needed?

Regards,
Christian

---

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iio/accel/mxc6255.c?h=v5.6.15&id=06777c562a50a09c4a2becfb2bf63c762a45df17

[2]
P: /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:22/MXC6655:00
L: 0
E: DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:22/MXC6655:00
E: SUBSYSTEM=acpi
E: MODALIAS=acpi:MXC6655:MXC6655:
E: USEC_INITIALIZED=5319671
E: ID_VENDOR_FROM_DATABASE=The Linux Foundation

P: /devices/pci0000:00/0000:00:16.0/i2c_designware.0/i2c-0/i2c-MXC6655:00
L: 0
E: DEVPATH=/devices/pci0000:00/0000:00:16.0/i2c_designware.0/i2c-0/i2c-MXC6655:00
E: SUBSYSTEM=i2c
E: MODALIAS=acpi:MXC6655:MXC6655:

[3]
https://github.com/systemd/systemd/commit/5e0676c2cad60b1ea029b9bfb9737e1967abb93a

On Sun, May 31, 2020 at 12:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 29 May 2020 22:05:49 +0200
> Christian Oder <me@myself5.de> wrote:
>
> > The mxc6655 is fully working with the existing mxc4005 driver.
> > Add support for it.
> >
> > Signed-off-by: Christian Oder <me@myself5.de>
>
> One query on ACPI bindings.  What is there already may
> be missleading :(
>
>
> > ---
> >  drivers/iio/accel/mxc4005.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
> > index 3d5bea651923..3b8614352cb4 100644
> > --- a/drivers/iio/accel/mxc4005.c
> > +++ b/drivers/iio/accel/mxc4005.c
> > @@ -474,12 +474,14 @@ static int mxc4005_probe(struct i2c_client *client,
> >
> >  static const struct acpi_device_id mxc4005_acpi_match[] = {
> >       {"MXC4005",     0},
> > +     {"MXC6655",     0},
>
> Do we have a reference for these ACPI bindings?  While they may seem
> obvious, memsic don't have a registered PNP or ACPI ID that I can
> find.  If these are in the wild (i.e. in shipping firmware) then we
> can take them as defacto bindings, otherwise we should avoid making
> them so by putting them in the driver.
>
> Quite a few similar bindings got in a while back that I should have
> noticed, but I wasn't so familiar with ACPI back then.  Some
> scrubbing of these has gone on recently, but there are lots still
> left in IIO.
>
> If we aren't sure, then drop the ACPI addition and just leave the
> i2c one below.  Adding an explicit DT binding table would also be
> good if that is method you are using to probe this (or PRP0001
> from acpi which uses the dt bindings table)
>
> Thanks,
>
> Jonathan
>
>
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(acpi, mxc4005_acpi_match);
> >
> >  static const struct i2c_device_id mxc4005_id[] = {
> >       {"mxc4005",     0},
> > +     {"mxc6655",     0},
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(i2c, mxc4005_id);
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: accel: mxc4005: add support for mxc6655
  2020-05-31 13:16   ` Christian Oder
@ 2020-05-31 13:52     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2020-05-31 13:52 UTC (permalink / raw)
  To: Christian Oder
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Thomas Gleixner, Allison Randal, Greg Kroah-Hartman,
	Chuhong Yuan, linux-iio, linux-kernel

On Sun, 31 May 2020 15:16:00 +0200
Christian Oder <me@myself5.de> wrote:

> Hi Jonathan,
> 
> I tested the sensor on a Chuwi Hi10 X and only went by what I've seen in other
> commits before[1].
> 
> I just ran another test to see what entry is necessary and it appears the sensor
> still works when removing the i2c entry, but is not working anymore when
> removing the ACPI match. I got the ACPI IDs from udevadm info -e[2].
> Would that mean, that I should remove the i2c entry given it's working fine
> with ACPI on its own then, or am I missing something?
The i2c entry is fine.
> 
> I'm also successfully using the ACPI ID for the touchscreen orientation quirk
> in systemd[3].
Great.  That means it is out there so is a defacto binding even if
there isn't an official Doc.  Sadly a lot of device manufacturers
don't do this stuff the way they are supposed to!

> 
> > Adding an explicit DT binding table would also be
> > good if that is method you are using to probe this (or PRP0001
> > from acpi which uses the dt bindings table)  
> 
> Frankly, I have no idea how to do that or if that would still be required when
> using ACPI. Can you point me in a rough direction in case it's still needed?

It's all good given you've confirmed the ID is out there in the wild.

Applied to the togreg branch of iio.git and pushed out as testing for
autobuilders to poke at.

Note we've missed the merge window now so this will take a while to get
into the mainline kernel - should be in linux-next in a few weeks though.

Thanks,

Jonathan

> 
> Regards,
> Christian
> 
> ---
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iio/accel/mxc6255.c?h=v5.6.15&id=06777c562a50a09c4a2becfb2bf63c762a45df17
> 
> [2]
> P: /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:22/MXC6655:00
> L: 0
> E: DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:22/MXC6655:00
> E: SUBSYSTEM=acpi
> E: MODALIAS=acpi:MXC6655:MXC6655:
> E: USEC_INITIALIZED=5319671
> E: ID_VENDOR_FROM_DATABASE=The Linux Foundation
> 
> P: /devices/pci0000:00/0000:00:16.0/i2c_designware.0/i2c-0/i2c-MXC6655:00
> L: 0
> E: DEVPATH=/devices/pci0000:00/0000:00:16.0/i2c_designware.0/i2c-0/i2c-MXC6655:00
> E: SUBSYSTEM=i2c
> E: MODALIAS=acpi:MXC6655:MXC6655:
> 
> [3]
> https://github.com/systemd/systemd/commit/5e0676c2cad60b1ea029b9bfb9737e1967abb93a
> 
> On Sun, May 31, 2020 at 12:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Fri, 29 May 2020 22:05:49 +0200
> > Christian Oder <me@myself5.de> wrote:
> >  
> > > The mxc6655 is fully working with the existing mxc4005 driver.
> > > Add support for it.
> > >
> > > Signed-off-by: Christian Oder <me@myself5.de>  
> >
> > One query on ACPI bindings.  What is there already may
> > be missleading :(
> >
> >  
> > > ---
> > >  drivers/iio/accel/mxc4005.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
> > > index 3d5bea651923..3b8614352cb4 100644
> > > --- a/drivers/iio/accel/mxc4005.c
> > > +++ b/drivers/iio/accel/mxc4005.c
> > > @@ -474,12 +474,14 @@ static int mxc4005_probe(struct i2c_client *client,
> > >
> > >  static const struct acpi_device_id mxc4005_acpi_match[] = {
> > >       {"MXC4005",     0},
> > > +     {"MXC6655",     0},  
> >
> > Do we have a reference for these ACPI bindings?  While they may seem
> > obvious, memsic don't have a registered PNP or ACPI ID that I can
> > find.  If these are in the wild (i.e. in shipping firmware) then we
> > can take them as defacto bindings, otherwise we should avoid making
> > them so by putting them in the driver.
> >
> > Quite a few similar bindings got in a while back that I should have
> > noticed, but I wasn't so familiar with ACPI back then.  Some
> > scrubbing of these has gone on recently, but there are lots still
> > left in IIO.
> >
> > If we aren't sure, then drop the ACPI addition and just leave the
> > i2c one below.  Adding an explicit DT binding table would also be
> > good if that is method you are using to probe this (or PRP0001
> > from acpi which uses the dt bindings table)
> >
> > Thanks,
> >
> > Jonathan
> >
> >  
> > >       { },
> > >  };
> > >  MODULE_DEVICE_TABLE(acpi, mxc4005_acpi_match);
> > >
> > >  static const struct i2c_device_id mxc4005_id[] = {
> > >       {"mxc4005",     0},
> > > +     {"mxc6655",     0},
> > >       { },
> > >  };
> > >  MODULE_DEVICE_TABLE(i2c, mxc4005_id);  
> >  


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-05-31 13:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29 20:05 [PATCH] iio: accel: mxc4005: add support for mxc6655 Christian Oder
2020-05-31 10:30 ` Jonathan Cameron
2020-05-31 13:16   ` Christian Oder
2020-05-31 13:52     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).