[PATCH] input: adxl34x: Add OF match support

[PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

The I2C subsystem can match devices without explicit OF support based on
the part of their compatible property after the comma. However, this
mechanism uses the first compatible value only. For adxl34x OF device
nodes the compatible property should list the more specific
"adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
fallback value second. This prevents the device node from being matched
with the adxl34x driver.

Fix this by adding an OF match table with an "adi,adxl34x" compatible
entry.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Another option would have been to add "adxl325" and "adxl326" entries to the
adxl34x_id I2C match table, but it would have had the drawback of requiring a
driver update for every new device.

diff --git a/drivers/input/misc/adxl34x-i2c.c b/drivers/input/misc/adxl34x-i2c.c
index 416f47ddcc90..22687b738811 100644
--- a/drivers/input/misc/adxl34x-i2c.c
+++ b/drivers/input/misc/adxl34x-i2c.c
@@ -10,6 +10,7 @@
 #include <linux/input.h>	/* BUS_I2C */
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/types.h>
 #include <linux/pm.h>
 #include "adxl34x.h"
@@ -137,11 +138,21 @@ static const struct i2c_device_id adxl34x_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, adxl34x_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id adxl34x_of_id[] = {
+	{ .compatible = "adi,adxl34x", },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, adxl34x_of_id);
+#endif
+
 static struct i2c_driver adxl34x_driver = {
 	.driver = {
 		.name = "adxl34x",
 		.owner = THIS_MODULE,
 		.pm = &adxl34x_i2c_pm,
+		.of_match_table = of_match_ptr(adxl34x_of_id),
 	},
 	.probe    = adxl34x_i2c_probe,
 	.remove   = adxl34x_i2c_remove,
-- 
Regards,

Laurent Pinchart

[PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

The I2C subsystem can match devices without explicit OF support based on
the part of their compatible property after the comma. However, this
mechanism uses the first compatible value only. For adxl34x OF device
nodes the compatible property should list the more specific
"adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
fallback value second. This prevents the device node from being matched
with the adxl34x driver.

Fix this by adding an OF match table with an "adi,adxl34x" compatible
entry.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Another option would have been to add "adxl325" and "adxl326" entries to the
adxl34x_id I2C match table, but it would have had the drawback of requiring a
driver update for every new device.

diff --git a/drivers/input/misc/adxl34x-i2c.c b/drivers/input/misc/adxl34x-i2c.c
index 416f47ddcc90..22687b738811 100644
--- a/drivers/input/misc/adxl34x-i2c.c
+++ b/drivers/input/misc/adxl34x-i2c.c
@@ -10,6 +10,7 @@
 #include <linux/input.h>	/* BUS_I2C */
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/types.h>
 #include <linux/pm.h>
 #include "adxl34x.h"
@@ -137,11 +138,21 @@ static const struct i2c_device_id adxl34x_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, adxl34x_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id adxl34x_of_id[] = {
+	{ .compatible = "adi,adxl34x", },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, adxl34x_of_id);
+#endif
+
 static struct i2c_driver adxl34x_driver = {
 	.driver = {
 		.name = "adxl34x",
 		.owner = THIS_MODULE,
 		.pm = &adxl34x_i2c_pm,
+		.of_match_table = of_match_ptr(adxl34x_of_id),
 	},
 	.probe    = adxl34x_i2c_probe,
 	.remove   = adxl34x_i2c_remove,
-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Wolfram Sang]

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

On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> The I2C subsystem can match devices without explicit OF support based on
> the part of their compatible property after the comma. However, this
> mechanism uses the first compatible value only. For adxl34x OF device
> nodes the compatible property should list the more specific
> "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> fallback value second. This prevents the device node from being matched
> with the adxl34x driver.
> 
> Fix this by adding an OF match table with an "adi,adxl34x" compatible
> entry.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Another option would have been to add "adxl325" and "adxl326" entries to the
> adxl34x_id I2C match table, but it would have had the drawback of requiring a
> driver update for every new device.

AFAIK this is even required for compatible entries, to be as specific as
possible. I think this makes sense. With platform_ids, we already had
the problem that pca954x was too generic and was used for both GPIO
extenders and I2C muxers (IIRC).


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] input: adxl34x: Add OF match support

[Wolfram Sang]

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

On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> The I2C subsystem can match devices without explicit OF support based on
> the part of their compatible property after the comma. However, this
> mechanism uses the first compatible value only. For adxl34x OF device
> nodes the compatible property should list the more specific
> "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> fallback value second. This prevents the device node from being matched
> with the adxl34x driver.
> 
> Fix this by adding an OF match table with an "adi,adxl34x" compatible
> entry.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Another option would have been to add "adxl325" and "adxl326" entries to the
> adxl34x_id I2C match table, but it would have had the drawback of requiring a
> driver update for every new device.

AFAIK this is even required for compatible entries, to be as specific as
possible. I think this makes sense. With platform_ids, we already had
the problem that pca954x was too generic and was used for both GPIO
extenders and I2C muxers (IIRC).


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Wolfram,

On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> > The I2C subsystem can match devices without explicit OF support based on
> > the part of their compatible property after the comma. However, this
> > mechanism uses the first compatible value only. For adxl34x OF device
> > nodes the compatible property should list the more specific
> > "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> > fallback value second. This prevents the device node from being matched
> > with the adxl34x driver.
> > 
> > Fix this by adding an OF match table with an "adi,adxl34x" compatible
> > entry.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > Another option would have been to add "adxl325" and "adxl326" entries to
> > the adxl34x_id I2C match table, but it would have had the drawback of
> > requiring a driver update for every new device.
> 
> AFAIK this is even required for compatible entries, to be as specific as
> possible. I think this makes sense. With platform_ids, we already had
> the problem that pca954x was too generic and was used for both GPIO
> extenders and I2C muxers (IIRC).

There are three compatible strings defined for the ADXL345 and ADXL346 in 
Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345", 
"adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the 
first two I don't see a need to add the specific compatible strings to the 
driver for now. If a new totally incompatible chip named ADXL347 comes out we 
will need a new driver which won't be allowed to use the "adi,adxl34x" 
compatible string.

An option would be to remove "adi,adxl34x" from 
Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the 
driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might 
clash with the DT ABI stability requirements though.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Wolfram,

On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> > The I2C subsystem can match devices without explicit OF support based on
> > the part of their compatible property after the comma. However, this
> > mechanism uses the first compatible value only. For adxl34x OF device
> > nodes the compatible property should list the more specific
> > "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> > fallback value second. This prevents the device node from being matched
> > with the adxl34x driver.
> > 
> > Fix this by adding an OF match table with an "adi,adxl34x" compatible
> > entry.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > Another option would have been to add "adxl325" and "adxl326" entries to
> > the adxl34x_id I2C match table, but it would have had the drawback of
> > requiring a driver update for every new device.
> 
> AFAIK this is even required for compatible entries, to be as specific as
> possible. I think this makes sense. With platform_ids, we already had
> the problem that pca954x was too generic and was used for both GPIO
> extenders and I2C muxers (IIRC).

There are three compatible strings defined for the ADXL345 and ADXL346 in 
Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345", 
"adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the 
first two I don't see a need to add the specific compatible strings to the 
driver for now. If a new totally incompatible chip named ADXL347 comes out we 
will need a new driver which won't be allowed to use the "adi,adxl34x" 
compatible string.

An option would be to remove "adi,adxl34x" from 
Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the 
driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might 
clash with the DT ABI stability requirements though.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> There are three compatible strings defined for the ADXL345 and ADXL346 in
> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the
> first two I don't see a need to add the specific compatible strings to the
> driver for now. If a new totally incompatible chip named ADXL347 comes out we
> will need a new driver which won't be allowed to use the "adi,adxl34x"
> compatible string.

FWIW, I'm the evil person who added the adxl entries to that file...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> There are three compatible strings defined for the ADXL345 and ADXL346 in
> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the
> first two I don't see a need to add the specific compatible strings to the
> driver for now. If a new totally incompatible chip named ADXL347 comes out we
> will need a new driver which won't be allowed to use the "adi,adxl34x"
> compatible string.

FWIW, I'm the evil person who added the adxl entries to that file...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Geert,

On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
> On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
> > There are three compatible strings defined for the ADXL345 and ADXL346 in
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
> > the first two I don't see a need to add the specific compatible strings to
> > the driver for now. If a new totally incompatible chip named ADXL347 comes
> > out we will need a new driver which won't be allowed to use the
> > "adi,adxl34x" compatible string.
> 
> FWIW, I'm the evil person who added the adxl entries to that file...

git-blame had already reported you.

Do you think we should remove that compatible string ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Geert,

On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
> On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
> > There are three compatible strings defined for the ADXL345 and ADXL346 in
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
> > the first two I don't see a need to add the specific compatible strings to
> > the driver for now. If a new totally incompatible chip named ADXL347 comes
> > out we will need a new driver which won't be allowed to use the
> > "adi,adxl34x" compatible string.
> 
> FWIW, I'm the evil person who added the adxl entries to that file...

git-blame had already reported you.

Do you think we should remove that compatible string ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Geert,

On Thursday 18 December 2014 21:23:46 Laurent Pinchart wrote:
> On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
> > On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
> > > There are three compatible strings defined for the ADXL345 and ADXL346
> > > in Documentation/devicetree/bindings/i2c/trivial-devices.txt:
> > > "adi,adxl345", "adi,adxl346", "adi,adxl34x". Given that the last one is
> > > a fallback for the first two I don't see a need to add the specific
> > > compatible strings to the driver for now. If a new totally incompatible
> > > chip named ADXL347 comes out we will need a new driver which won't be
> > > allowed to use the "adi,adxl34x" compatible string.
> > 
> > FWIW, I'm the evil person who added the adxl entries to that file...
> 
> git-blame had already reported you.
> 
> Do you think we should remove that compatible string ?

Ping ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Geert,

On Thursday 18 December 2014 21:23:46 Laurent Pinchart wrote:
> On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
> > On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
> > > There are three compatible strings defined for the ADXL345 and ADXL346
> > > in Documentation/devicetree/bindings/i2c/trivial-devices.txt:
> > > "adi,adxl345", "adi,adxl346", "adi,adxl34x". Given that the last one is
> > > a fallback for the first two I don't see a need to add the specific
> > > compatible strings to the driver for now. If a new totally incompatible
> > > chip named ADXL347 comes out we will need a new driver which won't be
> > > allowed to use the "adi,adxl34x" compatible string.
> > 
> > FWIW, I'm the evil person who added the adxl entries to that file...
> 
> git-blame had already reported you.
> 
> Do you think we should remove that compatible string ?

Ping ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

Hi Laurent, Wolfram,

On Thu, Dec 18, 2014 at 8:23 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
>> On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
>> > There are three compatible strings defined for the ADXL345 and ADXL346 in
>> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
>> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
>> > the first two I don't see a need to add the specific compatible strings to
>> > the driver for now. If a new totally incompatible chip named ADXL347 comes
>> > out we will need a new driver which won't be allowed to use the
>> > "adi,adxl34x" compatible string.
>>
>> FWIW, I'm the evil person who added the adxl entries to that file...
>
> git-blame had already reported you.
>
> Do you think we should remove that compatible string ?

Remove it from trivial-devices.txt, or from the .dts?

Can't we just fix i2c_device_match(), to consider all compatible values
of the device's node instead of just the first one when doing a legacy match?

For now, I'd remove the "adi,adxl345" entry from sh73a0-kzm9g.dts,
until matching is fixed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

Hi Laurent, Wolfram,

On Thu, Dec 18, 2014 at 8:23 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday 18 December 2014 14:03:18 Geert Uytterhoeven wrote:
>> On Thu, Dec 18, 2014 at 1:49 PM, Laurent Pinchart wrote:
>> > There are three compatible strings defined for the ADXL345 and ADXL346 in
>> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
>> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
>> > the first two I don't see a need to add the specific compatible strings to
>> > the driver for now. If a new totally incompatible chip named ADXL347 comes
>> > out we will need a new driver which won't be allowed to use the
>> > "adi,adxl34x" compatible string.
>>
>> FWIW, I'm the evil person who added the adxl entries to that file...
>
> git-blame had already reported you.
>
> Do you think we should remove that compatible string ?

Remove it from trivial-devices.txt, or from the .dts?

Can't we just fix i2c_device_match(), to consider all compatible values
of the device's node instead of just the first one when doing a legacy match?

For now, I'd remove the "adi,adxl345" entry from sh73a0-kzm9g.dts,
until matching is fixed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Wolfram Sang]

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

On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote:
> Hi Wolfram,
> 
> On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> > On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> > > The I2C subsystem can match devices without explicit OF support based on
> > > the part of their compatible property after the comma. However, this
> > > mechanism uses the first compatible value only. For adxl34x OF device
> > > nodes the compatible property should list the more specific
> > > "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> > > fallback value second. This prevents the device node from being matched
> > > with the adxl34x driver.
> > > 
> > > Fix this by adding an OF match table with an "adi,adxl34x" compatible
> > > entry.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > 
> > >  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > Another option would have been to add "adxl325" and "adxl326" entries to
> > > the adxl34x_id I2C match table, but it would have had the drawback of
> > > requiring a driver update for every new device.
> > 
> > AFAIK this is even required for compatible entries, to be as specific as
> > possible. I think this makes sense. With platform_ids, we already had
> > the problem that pca954x was too generic and was used for both GPIO
> > extenders and I2C muxers (IIRC).
> 
> There are three compatible strings defined for the ADXL345 and ADXL346 in 
> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345", 
> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the 
> first two I don't see a need to add the specific compatible strings to the 
> driver for now. If a new totally incompatible chip named ADXL347 comes out we 
> will need a new driver which won't be allowed to use the "adi,adxl34x" 
> compatible string.

Been there, got bitten. We only found out too late, because one driver
was in i2c and the other in GPIO (or LED even?), both using "953x" :(

> An option would be to remove "adi,adxl34x" from 
> Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the 
> driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might 
> clash with the DT ABI stability requirements though.

I do prefer this:

1) add specific compatible values to the driver. We do those updates for
new devices all the time
2) also add "34x" as a compatible but mark it as deprecateed
3) delete "34x" from trivial devices

Everyone OK with that?

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] input: adxl34x: Add OF match support

[Wolfram Sang]

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

On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote:
> Hi Wolfram,
> 
> On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> > On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> > > The I2C subsystem can match devices without explicit OF support based on
> > > the part of their compatible property after the comma. However, this
> > > mechanism uses the first compatible value only. For adxl34x OF device
> > > nodes the compatible property should list the more specific
> > > "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> > > fallback value second. This prevents the device node from being matched
> > > with the adxl34x driver.
> > > 
> > > Fix this by adding an OF match table with an "adi,adxl34x" compatible
> > > entry.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > 
> > >  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > Another option would have been to add "adxl325" and "adxl326" entries to
> > > the adxl34x_id I2C match table, but it would have had the drawback of
> > > requiring a driver update for every new device.
> > 
> > AFAIK this is even required for compatible entries, to be as specific as
> > possible. I think this makes sense. With platform_ids, we already had
> > the problem that pca954x was too generic and was used for both GPIO
> > extenders and I2C muxers (IIRC).
> 
> There are three compatible strings defined for the ADXL345 and ADXL346 in 
> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345", 
> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the 
> first two I don't see a need to add the specific compatible strings to the 
> driver for now. If a new totally incompatible chip named ADXL347 comes out we 
> will need a new driver which won't be allowed to use the "adi,adxl34x" 
> compatible string.

Been there, got bitten. We only found out too late, because one driver
was in i2c and the other in GPIO (or LED even?), both using "953x" :(

> An option would be to remove "adi,adxl34x" from 
> Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the 
> driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might 
> clash with the DT ABI stability requirements though.

I do prefer this:

1) add specific compatible values to the driver. We do those updates for
new devices all the time
2) also add "34x" as a compatible but mark it as deprecateed
3) delete "34x" from trivial devices

Everyone OK with that?

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

On Thu, Jan 15, 2015 at 1:53 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> There are three compatible strings defined for the ADXL345 and ADXL346 in
>> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
>> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the
>> first two I don't see a need to add the specific compatible strings to the
>> driver for now. If a new totally incompatible chip named ADXL347 comes out we
>> will need a new driver which won't be allowed to use the "adi,adxl34x"
>> compatible string.
>
> Been there, got bitten. We only found out too late, because one driver
> was in i2c and the other in GPIO (or LED even?), both using "953x" :(

Yep, pca953x => leds-pca953x

>> An option would be to remove "adi,adxl34x" from
>> Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the
>> driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might
>> clash with the DT ABI stability requirements though.
>
> I do prefer this:
>
> 1) add specific compatible values to the driver. We do those updates for
> new devices all the time
> 2) also add "34x" as a compatible but mark it as deprecateed
> 3) delete "34x" from trivial devices
>
> Everyone OK with that?

Sounds fine to me. Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

On Thu, Jan 15, 2015 at 1:53 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> There are three compatible strings defined for the ADXL345 and ADXL346 in
>> Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
>> "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for the
>> first two I don't see a need to add the specific compatible strings to the
>> driver for now. If a new totally incompatible chip named ADXL347 comes out we
>> will need a new driver which won't be allowed to use the "adi,adxl34x"
>> compatible string.
>
> Been there, got bitten. We only found out too late, because one driver
> was in i2c and the other in GPIO (or LED even?), both using "953x" :(

Yep, pca953x => leds-pca953x

>> An option would be to remove "adi,adxl34x" from
>> Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case the
>> driver should match explicitly on "adi,adxl345" and "adi,adxl346". That might
>> clash with the DT ABI stability requirements though.
>
> I do prefer this:
>
> 1) add specific compatible values to the driver. We do those updates for
> new devices all the time
> 2) also add "34x" as a compatible but mark it as deprecateed
> 3) delete "34x" from trivial devices
>
> Everyone OK with that?

Sounds fine to me. Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Wolfram,

On Thursday 15 January 2015 13:53:22 Wolfram Sang wrote:
> On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote:
> > On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> >> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> >>> The I2C subsystem can match devices without explicit OF support based
> >>> on the part of their compatible property after the comma. However,
> >>> this mechanism uses the first compatible value only. For adxl34x OF
> >>> device nodes the compatible property should list the more specific
> >>> "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> >>> fallback value second. This prevents the device node from being
> >>> matched with the adxl34x driver.
> >>> 
> >>> Fix this by adding an OF match table with an "adi,adxl34x" compatible
> >>> entry.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> 
> >>>  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>> 
> >>> Another option would have been to add "adxl325" and "adxl326" entries
> >>> to the adxl34x_id I2C match table, but it would have had the drawback
> >>> of requiring a driver update for every new device.
> >> 
> >> AFAIK this is even required for compatible entries, to be as specific as
> >> possible. I think this makes sense. With platform_ids, we already had
> >> the problem that pca954x was too generic and was used for both GPIO
> >> extenders and I2C muxers (IIRC).
> > 
> > There are three compatible strings defined for the ADXL345 and ADXL346 in
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
> > the first two I don't see a need to add the specific compatible strings to
> > the driver for now. If a new totally incompatible chip named ADXL347 comes
> > out we will need a new driver which won't be allowed to use the
> > "adi,adxl34x" compatible string.
> 
> Been there, got bitten. We only found out too late, because one driver
> was in i2c and the other in GPIO (or LED even?), both using "953x" :(

That seems like a development, review and/or merge process failure to me, I 
wouldn't avoid generic compatible strings for that reason only.

> > An option would be to remove "adi,adxl34x" from
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
> > the driver should match explicitly on "adi,adxl345" and "adi,adxl346".
> > That might clash with the DT ABI stability requirements though.
> 
> I do prefer this:
> 
> 1) add specific compatible values to the driver. We do those updates for
> new devices all the time

Do you mean OF compatible values, or I2C match table entries ? I assume OF 
compatible values.

As the ADXL346 is backward-compatible with the ADXL345, and as the driver 
doesn't support the ADXL346-specific features, how about adding only the 
adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for the 
ADXL346 ?

> 2) also add "34x" as a compatible but mark it as deprecateed
> 3) delete "34x" from trivial devices

OK.

> Everyone OK with that?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

Hi Wolfram,

On Thursday 15 January 2015 13:53:22 Wolfram Sang wrote:
> On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote:
> > On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> >> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> >>> The I2C subsystem can match devices without explicit OF support based
> >>> on the part of their compatible property after the comma. However,
> >>> this mechanism uses the first compatible value only. For adxl34x OF
> >>> device nodes the compatible property should list the more specific
> >>> "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> >>> fallback value second. This prevents the device node from being
> >>> matched with the adxl34x driver.
> >>> 
> >>> Fix this by adding an OF match table with an "adi,adxl34x" compatible
> >>> entry.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
> >>> 
> >>>  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>> 
> >>> Another option would have been to add "adxl325" and "adxl326" entries
> >>> to the adxl34x_id I2C match table, but it would have had the drawback
> >>> of requiring a driver update for every new device.
> >> 
> >> AFAIK this is even required for compatible entries, to be as specific as
> >> possible. I think this makes sense. With platform_ids, we already had
> >> the problem that pca954x was too generic and was used for both GPIO
> >> extenders and I2C muxers (IIRC).
> > 
> > There are three compatible strings defined for the ADXL345 and ADXL346 in
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt: "adi,adxl345",
> > "adi,adxl346", "adi,adxl34x". Given that the last one is a fallback for
> > the first two I don't see a need to add the specific compatible strings to
> > the driver for now. If a new totally incompatible chip named ADXL347 comes
> > out we will need a new driver which won't be allowed to use the
> > "adi,adxl34x" compatible string.
> 
> Been there, got bitten. We only found out too late, because one driver
> was in i2c and the other in GPIO (or LED even?), both using "953x" :(

That seems like a development, review and/or merge process failure to me, I 
wouldn't avoid generic compatible strings for that reason only.

> > An option would be to remove "adi,adxl34x" from
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
> > the driver should match explicitly on "adi,adxl345" and "adi,adxl346".
> > That might clash with the DT ABI stability requirements though.
> 
> I do prefer this:
> 
> 1) add specific compatible values to the driver. We do those updates for
> new devices all the time

Do you mean OF compatible values, or I2C match table entries ? I assume OF 
compatible values.

As the ADXL346 is backward-compatible with the ADXL345, and as the driver 
doesn't support the ADXL346-specific features, how about adding only the 
adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for the 
ADXL346 ?

> 2) also add "34x" as a compatible but mark it as deprecateed
> 3) delete "34x" from trivial devices

OK.

> Everyone OK with that?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

On Thursday 15 January 2015 16:19:19 Laurent Pinchart wrote:
> On Thursday 15 January 2015 13:53:22 Wolfram Sang wrote:
> > On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote:
> >> On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> >>> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> >>>> The I2C subsystem can match devices without explicit OF support based
> >>>> on the part of their compatible property after the comma. However,
> >>>> this mechanism uses the first compatible value only. For adxl34x OF
> >>>> device nodes the compatible property should list the more specific
> >>>> "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> >>>> fallback value second. This prevents the device node from being
> >>>> matched with the adxl34x driver.
> >>>> 
> >>>> Fix this by adding an OF match table with an "adi,adxl34x" compatible
> >>>> entry.
> >>>> 
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>> 
> >>>>  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >>>>  1 file changed, 11 insertions(+)
> >>>> 
> >>>> Another option would have been to add "adxl325" and "adxl326" entries
> >>>> to the adxl34x_id I2C match table, but it would have had the drawback
> >>>> of requiring a driver update for every new device.
> >>> 
> >>> AFAIK this is even required for compatible entries, to be as specific
> >>> as possible. I think this makes sense. With platform_ids, we already
> >>> had the problem that pca954x was too generic and was used for both GPIO
> >>> extenders and I2C muxers (IIRC).
> >> 
> >> There are three compatible strings defined for the ADXL345 and ADXL346
> >> in Documentation/devicetree/bindings/i2c/trivial-devices.txt:
> >> "adi,adxl345", "adi,adxl346", "adi,adxl34x". Given that the last one is
> >> a fallback for the first two I don't see a need to add the specific
> >> compatible strings to the driver for now. If a new totally incompatible
> >> chip named ADXL347 comes out we will need a new driver which won't be
> >> allowed to use the "adi,adxl34x" compatible string.
> > 
> > Been there, got bitten. We only found out too late, because one driver
> > was in i2c and the other in GPIO (or LED even?), both using "953x" :(
> 
> That seems like a development, review and/or merge process failure to me, I
> wouldn't avoid generic compatible strings for that reason only.
> 
> > > An option would be to remove "adi,adxl34x" from
> > > Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
> > > the driver should match explicitly on "adi,adxl345" and "adi,adxl346".
> > > That might clash with the DT ABI stability requirements though.
> > 
> > I do prefer this:
> > 
> > 1) add specific compatible values to the driver. We do those updates for
> > new devices all the time
> 
> Do you mean OF compatible values, or I2C match table entries ? I assume OF
> compatible values.
> 
> As the ADXL346 is backward-compatible with the ADXL345, and as the driver
> doesn't support the ADXL346-specific features, how about adding only the
> adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for
> the ADXL346 ?

I spoke too fast. The driver supports ADXL346-specific features, but does so 
by detecting the device model at runtime.

I still believe it would make sense to list both the 346 and 345 models in DT 
for 346 devices, as they're compatible with the 345.

> > 2) also add "34x" as a compatible but mark it as deprecateed
> > 3) delete "34x" from trivial devices
> 
> OK.
> 
> > Everyone OK with that?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

On Thursday 15 January 2015 16:19:19 Laurent Pinchart wrote:
> On Thursday 15 January 2015 13:53:22 Wolfram Sang wrote:
> > On Thu, Dec 18, 2014 at 02:49:28PM +0200, Laurent Pinchart wrote:
> >> On Thursday 18 December 2014 09:21:51 Wolfram Sang wrote:
> >>> On Thu, Dec 18, 2014 at 04:15:23AM +0200, Laurent Pinchart wrote:
> >>>> The I2C subsystem can match devices without explicit OF support based
> >>>> on the part of their compatible property after the comma. However,
> >>>> this mechanism uses the first compatible value only. For adxl34x OF
> >>>> device nodes the compatible property should list the more specific
> >>>> "adi,adxl345" or "adi,adxl346" value first and the "adi,adxl34x"
> >>>> fallback value second. This prevents the device node from being
> >>>> matched with the adxl34x driver.
> >>>> 
> >>>> Fix this by adding an OF match table with an "adi,adxl34x" compatible
> >>>> entry.
> >>>> 
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>> 
> >>>>  drivers/input/misc/adxl34x-i2c.c | 11 +++++++++++
> >>>>  1 file changed, 11 insertions(+)
> >>>> 
> >>>> Another option would have been to add "adxl325" and "adxl326" entries
> >>>> to the adxl34x_id I2C match table, but it would have had the drawback
> >>>> of requiring a driver update for every new device.
> >>> 
> >>> AFAIK this is even required for compatible entries, to be as specific
> >>> as possible. I think this makes sense. With platform_ids, we already
> >>> had the problem that pca954x was too generic and was used for both GPIO
> >>> extenders and I2C muxers (IIRC).
> >> 
> >> There are three compatible strings defined for the ADXL345 and ADXL346
> >> in Documentation/devicetree/bindings/i2c/trivial-devices.txt:
> >> "adi,adxl345", "adi,adxl346", "adi,adxl34x". Given that the last one is
> >> a fallback for the first two I don't see a need to add the specific
> >> compatible strings to the driver for now. If a new totally incompatible
> >> chip named ADXL347 comes out we will need a new driver which won't be
> >> allowed to use the "adi,adxl34x" compatible string.
> > 
> > Been there, got bitten. We only found out too late, because one driver
> > was in i2c and the other in GPIO (or LED even?), both using "953x" :(
> 
> That seems like a development, review and/or merge process failure to me, I
> wouldn't avoid generic compatible strings for that reason only.
> 
> > > An option would be to remove "adi,adxl34x" from
> > > Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
> > > the driver should match explicitly on "adi,adxl345" and "adi,adxl346".
> > > That might clash with the DT ABI stability requirements though.
> > 
> > I do prefer this:
> > 
> > 1) add specific compatible values to the driver. We do those updates for
> > new devices all the time
> 
> Do you mean OF compatible values, or I2C match table entries ? I assume OF
> compatible values.
> 
> As the ADXL346 is backward-compatible with the ADXL345, and as the driver
> doesn't support the ADXL346-specific features, how about adding only the
> adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for
> the ADXL346 ?

I spoke too fast. The driver supports ADXL346-specific features, but does so 
by detecting the device model at runtime.

I still believe it would make sense to list both the 346 and 345 models in DT 
for 346 devices, as they're compatible with the 345.

> > 2) also add "34x" as a compatible but mark it as deprecateed
> > 3) delete "34x" from trivial devices
> 
> OK.
> 
> > Everyone OK with that?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Wolfram Sang]

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


> > > Been there, got bitten. We only found out too late, because one driver
> > > was in i2c and the other in GPIO (or LED even?), both using "953x" :(
> > 
> > That seems like a development, review and/or merge process failure to me, I
> > wouldn't avoid generic compatible strings for that reason only.

Well, I think different here, but let's skip this discussion as it is
not really needed right now...

> > As the ADXL346 is backward-compatible with the ADXL345, and as the driver
> > doesn't support the ADXL346-specific features, how about adding only the
> > adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for
> > the ADXL346 ?
> 
> I spoke too fast. The driver supports ADXL346-specific features, but does so 
> by detecting the device model at runtime.
> 
> I still believe it would make sense to list both the 346 and 345 models in DT 
> for 346 devices, as they're compatible with the 345.

I agree.

> > > 2) also add "34x" as a compatible but mark it as deprecateed
> > > 3) delete "34x" from trivial devices
> > 
> > OK.

Yay :)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] input: adxl34x: Add OF match support

[Wolfram Sang]

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


> > > Been there, got bitten. We only found out too late, because one driver
> > > was in i2c and the other in GPIO (or LED even?), both using "953x" :(
> > 
> > That seems like a development, review and/or merge process failure to me, I
> > wouldn't avoid generic compatible strings for that reason only.

Well, I think different here, but let's skip this discussion as it is
not really needed right now...

> > As the ADXL346 is backward-compatible with the ADXL345, and as the driver
> > doesn't support the ADXL346-specific features, how about adding only the
> > adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for
> > the ADXL346 ?
> 
> I spoke too fast. The driver supports ADXL346-specific features, but does so 
> by detecting the device model at runtime.
> 
> I still believe it would make sense to list both the 346 and 345 models in DT 
> for 346 devices, as they're compatible with the 345.

I agree.

> > > 2) also add "34x" as a compatible but mark it as deprecateed
> > > 3) delete "34x" from trivial devices
> > 
> > OK.

Yay :)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

On Thursday 15 January 2015 15:36:37 Wolfram Sang wrote:
> >>> Been there, got bitten. We only found out too late, because one driver
> >>> was in i2c and the other in GPIO (or LED even?), both using "953x" :(
> >> 
> >> That seems like a development, review and/or merge process failure to
> >> me, I wouldn't avoid generic compatible strings for that reason only.
> 
> Well, I think different here, but let's skip this discussion as it is
> not really needed right now...
> 
> >> As the ADXL346 is backward-compatible with the ADXL345, and as the
> >> driver doesn't support the ADXL346-specific features, how about adding
> >> only the adxl345 for now, and using compatible = "adi,adxl346",
> >> "adi,adxl345"; for the ADXL346 ?
> > 
> > I spoke too fast. The driver supports ADXL346-specific features, but does
> > so by detecting the device model at runtime.
> > 
> > I still believe it would make sense to list both the 346 and 345 models in
> > DT for 346 devices, as they're compatible with the 345.
> 
> I agree.
> 
> >>> 2) also add "34x" as a compatible but mark it as deprecateed
> >>> 3) delete "34x" from trivial devices
> >> 
> >> OK.
> 
> Yay :)

I'll submit patches very soon.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Laurent Pinchart]

On Thursday 15 January 2015 15:36:37 Wolfram Sang wrote:
> >>> Been there, got bitten. We only found out too late, because one driver
> >>> was in i2c and the other in GPIO (or LED even?), both using "953x" :(
> >> 
> >> That seems like a development, review and/or merge process failure to
> >> me, I wouldn't avoid generic compatible strings for that reason only.
> 
> Well, I think different here, but let's skip this discussion as it is
> not really needed right now...
> 
> >> As the ADXL346 is backward-compatible with the ADXL345, and as the
> >> driver doesn't support the ADXL346-specific features, how about adding
> >> only the adxl345 for now, and using compatible = "adi,adxl346",
> >> "adi,adxl345"; for the ADXL346 ?
> > 
> > I spoke too fast. The driver supports ADXL346-specific features, but does
> > so by detecting the device model at runtime.
> > 
> > I still believe it would make sense to list both the 346 and 345 models in
> > DT for 346 devices, as they're compatible with the 345.
> 
> I agree.
> 
> >>> 2) also add "34x" as a compatible but mark it as deprecateed
> >>> 3) delete "34x" from trivial devices
> >> 
> >> OK.
> 
> Yay :)

I'll submit patches very soon.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

On Thu, Jan 15, 2015 at 3:19 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > An option would be to remove "adi,adxl34x" from
>> > Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
>> > the driver should match explicitly on "adi,adxl345" and "adi,adxl346".
>> > That might clash with the DT ABI stability requirements though.
>>
>> I do prefer this:
>>
>> 1) add specific compatible values to the driver. We do those updates for
>> new devices all the time
>
> Do you mean OF compatible values, or I2C match table entries ? I assume OF
> compatible values.
>
> As the ADXL346 is backward-compatible with the ADXL345, and as the driver
> doesn't support the ADXL346-specific features, how about adding only the
> adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for the
> ADXL346 ?

Adxl34x is instantiated in:
arch/arm/boot/dts/sh73a0-kzm9g.dts
arch/arm/mach-shmobile/board-kzm9g.c
arch/arm/mach-shmobile/board-mackerel.c
arch/blackfin/mach-bf527/boards/tll6527m.c
arch/blackfin/mach-bf537/boards/stamp.c
arch/blackfin/mach-bf548/boards/ezkit.c
arch/blackfin/mach-bf609/boards/ezkit.c

The shmobile variants are all adxl345.
The tll6527m is also an adxl345.

For the remaining, it's not clear to me. I Googled a bit, but no luck.
So they'll have to live with adxl345, too ;-)

>> 2) also add "34x" as a compatible but mark it as deprecateed
>> 3) delete "34x" from trivial devices
>
> OK.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] input: adxl34x: Add OF match support

[Geert Uytterhoeven]

On Thu, Jan 15, 2015 at 3:19 PM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
>> > An option would be to remove "adi,adxl34x" from
>> > Documentation/devicetree/bindings/i2c/trivial-devices.txt, in which case
>> > the driver should match explicitly on "adi,adxl345" and "adi,adxl346".
>> > That might clash with the DT ABI stability requirements though.
>>
>> I do prefer this:
>>
>> 1) add specific compatible values to the driver. We do those updates for
>> new devices all the time
>
> Do you mean OF compatible values, or I2C match table entries ? I assume OF
> compatible values.
>
> As the ADXL346 is backward-compatible with the ADXL345, and as the driver
> doesn't support the ADXL346-specific features, how about adding only the
> adxl345 for now, and using compatible = "adi,adxl346", "adi,adxl345"; for the
> ADXL346 ?

Adxl34x is instantiated in:
arch/arm/boot/dts/sh73a0-kzm9g.dts
arch/arm/mach-shmobile/board-kzm9g.c
arch/arm/mach-shmobile/board-mackerel.c
arch/blackfin/mach-bf527/boards/tll6527m.c
arch/blackfin/mach-bf537/boards/stamp.c
arch/blackfin/mach-bf548/boards/ezkit.c
arch/blackfin/mach-bf609/boards/ezkit.c

The shmobile variants are all adxl345.
The tll6527m is also an adxl345.

For the remaining, it's not clear to me. I Googled a bit, but no luck.
So they'll have to live with adxl345, too ;-)

>> 2) also add "34x" as a compatible but mark it as deprecateed
>> 3) delete "34x" from trivial devices
>
> OK.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds