All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Fix OF match for adxl34x driver
@ 2015-01-15 14:54 ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 14:54 UTC (permalink / raw)
  To: linux-input; +Cc: linux-i2c, linux-sh, Wolfram Sang, Geert Uytterhoeven

Hello,

This patch set fixes OF matching for the adxl34x driver when the DT node lists
a device-specific "adi,adxl345" or "adi,adxl346" compatible value first.

The first version (see http://www.spinics.net/lists/linux-i2c/msg18107.html)
added an OF match entry for the "adi,adxl34x" compatible string. The
discussion that followed concluded that that compatible string should be
deprecated and that the driver should match the device-specific strings
instead.

The first patch thus deprecates the "adi,adxl34x" compatible string by
removing it the DT trivial devices list, and the second patch then adds an OF
match table to the adxl34x driver.

Laurent Pinchart (2):
  DT: i2c: Deprecate adi,adxl34x compatible string
  input: adxl34x: Add OF match support

 .../devicetree/bindings/i2c/trivial-devices.txt     |  3 +--
 drivers/input/misc/adxl34x-i2c.c                    | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

-- 
Regards,

Laurent Pinchart


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

* [PATCH v2 0/2] Fix OF match for adxl34x driver
@ 2015-01-15 14:54 ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 14:54 UTC (permalink / raw)
  To: linux-input; +Cc: linux-i2c, linux-sh, Wolfram Sang, Geert Uytterhoeven

Hello,

This patch set fixes OF matching for the adxl34x driver when the DT node lists
a device-specific "adi,adxl345" or "adi,adxl346" compatible value first.

The first version (see http://www.spinics.net/lists/linux-i2c/msg18107.html)
added an OF match entry for the "adi,adxl34x" compatible string. The
discussion that followed concluded that that compatible string should be
deprecated and that the driver should match the device-specific strings
instead.

The first patch thus deprecates the "adi,adxl34x" compatible string by
removing it the DT trivial devices list, and the second patch then adds an OF
match table to the adxl34x driver.

Laurent Pinchart (2):
  DT: i2c: Deprecate adi,adxl34x compatible string
  input: adxl34x: Add OF match support

 .../devicetree/bindings/i2c/trivial-devices.txt     |  3 +--
 drivers/input/misc/adxl34x-i2c.c                    | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

-- 
Regards,

Laurent Pinchart


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

* [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 14:54 ` Laurent Pinchart
@ 2015-01-15 14:54   ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 14:54 UTC (permalink / raw)
  To: linux-input; +Cc: linux-i2c, linux-sh, Wolfram Sang, Geert Uytterhoeven

DT nodes should use the more specific adi,adxl345 and adi,adxl346
compatible values instead. As the ADXL346 is backward-compatible with
the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
in that order.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 9f41d05be3be..757e42510517 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -18,8 +18,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
 adi,adt7476		+/-1C TDM Extended Temp Range I.C
 adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
-adi,adxl346		Three-Axis Digital Accelerometer
-adi,adxl34x		Three-Axis Digital Accelerometer
+adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,24c00		i2c serial eeprom  (24cxx)
 atmel,24c01		i2c serial eeprom  (24cxx)
-- 
2.0.5


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

* [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 14:54   ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 14:54 UTC (permalink / raw)
  To: linux-input; +Cc: linux-i2c, linux-sh, Wolfram Sang, Geert Uytterhoeven

DT nodes should use the more specific adi,adxl345 and adi,adxl346
compatible values instead. As the ADXL346 is backward-compatible with
the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
in that order.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 9f41d05be3be..757e42510517 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -18,8 +18,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
 adi,adt7476		+/-1C TDM Extended Temp Range I.C
 adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
-adi,adxl346		Three-Axis Digital Accelerometer
-adi,adxl34x		Three-Axis Digital Accelerometer
+adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,24c00		i2c serial eeprom  (24cxx)
 atmel,24c01		i2c serial eeprom  (24cxx)
-- 
2.0.5


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

* [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 14:54 ` Laurent Pinchart
@ 2015-01-15 14:54   ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 14:54 UTC (permalink / raw)
  To: linux-input; +Cc: linux-i2c, linux-sh, Wolfram Sang, Geert Uytterhoeven

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 will contain the more specific
"adi,adxl345" or "adi,adxl346" value first. This prevents the device
node from being matched with the adxl34x driver.

Fix this by adding an OF match table with an "adi,adxl345" compatible
entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
backward-compatible with the ADXL345 with differences handled by runtime
detection of the device model.

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

diff --git a/drivers/input/misc/adxl34x-i2c.c b/drivers/input/misc/adxl34x-i2c.c
index 416f47ddcc90..1e028f1f221c 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,31 @@ 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[] = {
+	/*
+	 * The ADXL346 is backward-compatible with the ADXL345. Differences are
+	 * handled by runtime detection of the device model, there's thus no
+	 * need for listing the "adi,adxl346" compatible value explicitly.
+	 */
+	{ .compatible = "adi,adxl345", },
+	/*
+	 * Deprecated, DT nodes should use one or more of the device-specific
+	 * compatible values "adi,adxl345" and "adi,adxl346".
+	 */
+	{ .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,
-- 
2.0.5


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

* [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 14:54   ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 14:54 UTC (permalink / raw)
  To: linux-input; +Cc: linux-i2c, linux-sh, Wolfram Sang, Geert Uytterhoeven

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 will contain the more specific
"adi,adxl345" or "adi,adxl346" value first. This prevents the device
node from being matched with the adxl34x driver.

Fix this by adding an OF match table with an "adi,adxl345" compatible
entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
backward-compatible with the ADXL345 with differences handled by runtime
detection of the device model.

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

diff --git a/drivers/input/misc/adxl34x-i2c.c b/drivers/input/misc/adxl34x-i2c.c
index 416f47ddcc90..1e028f1f221c 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,31 @@ 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[] = {
+	/*
+	 * The ADXL346 is backward-compatible with the ADXL345. Differences are
+	 * handled by runtime detection of the device model, there's thus no
+	 * need for listing the "adi,adxl346" compatible value explicitly.
+	 */
+	{ .compatible = "adi,adxl345", },
+	/*
+	 * Deprecated, DT nodes should use one or more of the device-specific
+	 * compatible values "adi,adxl345" and "adi,adxl346".
+	 */
+	{ .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,
-- 
2.0.5


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 14:54   ` Laurent Pinchart
@ 2015-01-15 16:55     ` Wolfram Sang
  -1 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-15 16:55 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-input, linux-i2c, linux-sh, Geert Uytterhoeven

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

On Thu, Jan 15, 2015 at 04:54:15PM +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 will contain the more specific
> "adi,adxl345" or "adi,adxl346" value first. This prevents the device
> node from being matched with the adxl34x driver.
> 
> Fix this by adding an OF match table with an "adi,adxl345" compatible
> entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
> backward-compatible with the ADXL345 with differences handled by runtime
> detection of the device model.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


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

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 16:55     ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-15 16:55 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-input, linux-i2c, linux-sh, Geert Uytterhoeven

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

On Thu, Jan 15, 2015 at 04:54:15PM +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 will contain the more specific
> "adi,adxl345" or "adi,adxl346" value first. This prevents the device
> node from being matched with the adxl34x driver.
> 
> Fix this by adding an OF match table with an "adi,adxl345" compatible
> entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
> backward-compatible with the ADXL345 with differences handled by runtime
> detection of the device model.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
       [not found]   ` <1421333655-31029-2-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
@ 2015-01-15 17:02       ` Wolfram Sang
  2015-01-26 12:12       ` Wolfram Sang
  1 sibling, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-15 17:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven

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

On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote:
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index 9f41d05be3be..757e42510517 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -18,8 +18,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
>  adi,adt7476		+/-1C TDM Extended Temp Range I.C
>  adi,adt7490		+/-1C TDM Extended Temp Range I.C
>  adi,adxl345		Three-Axis Digital Accelerometer
> -adi,adxl346		Three-Axis Digital Accelerometer
> -adi,adxl34x		Three-Axis Digital Accelerometer
> +adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)

I'd rather drop 346 because there is no compatible for that one anywhere.
No need to resend, I can fix it here...


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 17:02       ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-15 17:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven

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

On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote:
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnhk3lzF8UVTdg@public.gmane.orgm>
> ---
>  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index 9f41d05be3be..757e42510517 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -18,8 +18,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
>  adi,adt7476		+/-1C TDM Extended Temp Range I.C
>  adi,adt7490		+/-1C TDM Extended Temp Range I.C
>  adi,adxl345		Three-Axis Digital Accelerometer
> -adi,adxl346		Three-Axis Digital Accelerometer
> -adi,adxl34x		Three-Axis Digital Accelerometer
> +adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)

I'd rather drop 346 because there is no compatible for that one anywhere.
No need to resend, I can fix it here...


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 17:02       ` Wolfram Sang
@ 2015-01-15 17:27         ` Dmitry Torokhov
  -1 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 17:27 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input, linux-i2c, linux-sh, Geert Uytterhoeven

On Thu, Jan 15, 2015 at 06:02:09PM +0100, Wolfram Sang wrote:
> On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote:
> > DT nodes should use the more specific adi,adxl345 and adi,adxl346
> > compatible values instead. As the ADXL346 is backward-compatible with
> > the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> > in that order.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > index 9f41d05be3be..757e42510517 100644
> > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > @@ -18,8 +18,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
> >  adi,adt7476		+/-1C TDM Extended Temp Range I.C
> >  adi,adt7490		+/-1C TDM Extended Temp Range I.C
> >  adi,adxl345		Three-Axis Digital Accelerometer
> > -adi,adxl346		Three-Axis Digital Accelerometer
> > -adi,adxl34x		Three-Axis Digital Accelerometer
> > +adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
> 
> I'd rather drop 346 because there is no compatible for that one anywhere.
> No need to resend, I can fix it here...

So color me confused now. We deprecated adi,adxl34x because it is
generic, but then we adding "specific" adi,adxl345 that covers again
both devices? Why do we do this exactly? What is broken at the moment?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 17:27         ` Dmitry Torokhov
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 17:27 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input, linux-i2c, linux-sh, Geert Uytterhoeven

On Thu, Jan 15, 2015 at 06:02:09PM +0100, Wolfram Sang wrote:
> On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote:
> > DT nodes should use the more specific adi,adxl345 and adi,adxl346
> > compatible values instead. As the ADXL346 is backward-compatible with
> > the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> > in that order.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > index 9f41d05be3be..757e42510517 100644
> > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > @@ -18,8 +18,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
> >  adi,adt7476		+/-1C TDM Extended Temp Range I.C
> >  adi,adt7490		+/-1C TDM Extended Temp Range I.C
> >  adi,adxl345		Three-Axis Digital Accelerometer
> > -adi,adxl346		Three-Axis Digital Accelerometer
> > -adi,adxl34x		Three-Axis Digital Accelerometer
> > +adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
> 
> I'd rather drop 346 because there is no compatible for that one anywhere.
> No need to resend, I can fix it here...

So color me confused now. We deprecated adi,adxl34x because it is
generic, but then we adding "specific" adi,adxl345 that covers again
both devices? Why do we do this exactly? What is broken at the moment?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 17:02       ` Wolfram Sang
@ 2015-01-15 17:32         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:32 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list

On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
>>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
>>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
>>  adi,adxl345          Three-Axis Digital Accelerometer
>> -adi,adxl346          Three-Axis Digital Accelerometer
>> -adi,adxl34x          Three-Axis Digital Accelerometer
>> +adi,adxl346          Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
>
> I'd rather drop 346 because there is no compatible for that one anywhere.
> No need to resend, I can fix it here...

If you drop adi,adxl346, checkpatch will start complaining if it encounters
it in a .dts.

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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 17:32         ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:32 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list

On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
>>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
>>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
>>  adi,adxl345          Three-Axis Digital Accelerometer
>> -adi,adxl346          Three-Axis Digital Accelerometer
>> -adi,adxl34x          Three-Axis Digital Accelerometer
>> +adi,adxl346          Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
>
> I'd rather drop 346 because there is no compatible for that one anywhere.
> No need to resend, I can fix it here...

If you drop adi,adxl346, checkpatch will start complaining if it encounters
it in a .dts.

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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
       [not found]         ` <CAMuHMdW7ETcFGSPiE9BWA2dAE93477fzoyF-+_EaiPSDT9WMWA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-01-15 17:43             ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-15 17:43 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list

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

On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
> On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
> >>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
> >>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
> >>  adi,adxl345          Three-Axis Digital Accelerometer
> >> -adi,adxl346          Three-Axis Digital Accelerometer
> >> -adi,adxl34x          Three-Axis Digital Accelerometer
> >> +adi,adxl346          Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
> >
> > I'd rather drop 346 because there is no compatible for that one anywhere.
> > No need to resend, I can fix it here...
> 
> If you drop adi,adxl346, checkpatch will start complaining if it encounters
> it in a .dts.

Boah, this is annoying. That means we need an 346 entry even if it is
not different from 345 (which is fine by me).

If checkpatch does it this way, that means the rule of thumb is to
*always* have a dedicated compatible entry? Can someone confirm this?

Why did we discuss then? Now, I am confused as well...


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 17:43             ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-15 17:43 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list

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

On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
> On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
> >>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
> >>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
> >>  adi,adxl345          Three-Axis Digital Accelerometer
> >> -adi,adxl346          Three-Axis Digital Accelerometer
> >> -adi,adxl34x          Three-Axis Digital Accelerometer
> >> +adi,adxl346          Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
> >
> > I'd rather drop 346 because there is no compatible for that one anywhere.
> > No need to resend, I can fix it here...
> 
> If you drop adi,adxl346, checkpatch will start complaining if it encounters
> it in a .dts.

Boah, this is annoying. That means we need an 346 entry even if it is
not different from 345 (which is fine by me).

If checkpatch does it this way, that means the rule of thumb is to
*always* have a dedicated compatible entry? Can someone confirm this?

Why did we discuss then? Now, I am confused as well...


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

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
       [not found]   ` <1421333655-31029-3-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
@ 2015-01-15 17:45       ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:45 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Wolfram Sang

On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> 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 will contain the more specific
> "adi,adxl345" or "adi,adxl346" value first. This prevents the device
> node from being matched with the adxl34x driver.
>
> Fix this by adding an OF match table with an "adi,adxl345" compatible
> entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
> backward-compatible with the ADXL345 with differences handled by runtime
> detection of the device model.

Thanks!

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/drivers/input/misc/adxl34x-i2c.c
> +++ b/drivers/input/misc/adxl34x-i2c.c

> +#ifdef CONFIG_OF
> +static const struct of_device_id adxl34x_of_id[] = {
> +       /*
> +        * The ADXL346 is backward-compatible with the ADXL345. Differences are
> +        * handled by runtime detection of the device model, there's thus no
> +        * need for listing the "adi,adxl346" compatible value explicitly.
> +        */
> +       { .compatible = "adi,adxl345", },
> +       /*
> +        * Deprecated, DT nodes should use one or more of the device-specific
> +        * compatible values "adi,adxl345" and "adi,adxl346".

Ideally, the two comments above are moved to a real DT binding document ;-)

> +        */
> +       { .compatible = "adi,adxl34x", },

I'd append "/* deprecated */" to the line above, so "git grep adxl34x"
will show its deprecated status.

> +       { }

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

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 17:45       ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:45 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Wolfram Sang

On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
<laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> 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 will contain the more specific
> "adi,adxl345" or "adi,adxl346" value first. This prevents the device
> node from being matched with the adxl34x driver.
>
> Fix this by adding an OF match table with an "adi,adxl345" compatible
> entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
> backward-compatible with the ADXL345 with differences handled by runtime
> detection of the device model.

Thanks!

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Acked-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

> --- a/drivers/input/misc/adxl34x-i2c.c
> +++ b/drivers/input/misc/adxl34x-i2c.c

> +#ifdef CONFIG_OF
> +static const struct of_device_id adxl34x_of_id[] = {
> +       /*
> +        * The ADXL346 is backward-compatible with the ADXL345. Differences are
> +        * handled by runtime detection of the device model, there's thus no
> +        * need for listing the "adi,adxl346" compatible value explicitly.
> +        */
> +       { .compatible = "adi,adxl345", },
> +       /*
> +        * Deprecated, DT nodes should use one or more of the device-specific
> +        * compatible values "adi,adxl345" and "adi,adxl346".

Ideally, the two comments above are moved to a real DT binding document ;-)

> +        */
> +       { .compatible = "adi,adxl34x", },

I'd append "/* deprecated */" to the line above, so "git grep adxl34x"
will show its deprecated status.

> +       { }

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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 14:54   ` Laurent Pinchart
@ 2015-01-15 17:49     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:49 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-input, Linux I2C, Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 17:49     ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:49 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-input, Linux I2C, Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 17:43             ` Wolfram Sang
@ 2015-01-15 17:51               ` Geert Uytterhoeven
  -1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:51 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list

On Thu, Jan 15, 2015 at 6:43 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
>> On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
>> >>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
>> >>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
>> >>  adi,adxl345          Three-Axis Digital Accelerometer
>> >> -adi,adxl346          Three-Axis Digital Accelerometer
>> >> -adi,adxl34x          Three-Axis Digital Accelerometer
>> >> +adi,adxl346          Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
>> >
>> > I'd rather drop 346 because there is no compatible for that one anywhere.
>> > No need to resend, I can fix it here...
>>
>> If you drop adi,adxl346, checkpatch will start complaining if it encounters
>> it in a .dts.
>
> Boah, this is annoying. That means we need an 346 entry even if it is
> not different from 345 (which is fine by me).

To be clear: you need the entry in the documentation. It can be omitted
from the driver if it's not (yet) used for matching.

> If checkpatch does it this way, that means the rule of thumb is to
> *always* have a dedicated compatible entry? Can someone confirm this?

All compatible values that are in use must be documented.
Checkpatch just greps in Documentation/devicetree/bindings/.

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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 17:51               ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 17:51 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list

On Thu, Jan 15, 2015 at 6:43 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
>> On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
>> >>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
>> >>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
>> >>  adi,adxl345          Three-Axis Digital Accelerometer
>> >> -adi,adxl346          Three-Axis Digital Accelerometer
>> >> -adi,adxl34x          Three-Axis Digital Accelerometer
>> >> +adi,adxl346          Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
>> >
>> > I'd rather drop 346 because there is no compatible for that one anywhere.
>> > No need to resend, I can fix it here...
>>
>> If you drop adi,adxl346, checkpatch will start complaining if it encounters
>> it in a .dts.
>
> Boah, this is annoying. That means we need an 346 entry even if it is
> not different from 345 (which is fine by me).

To be clear: you need the entry in the documentation. It can be omitted
from the driver if it's not (yet) used for matching.

> If checkpatch does it this way, that means the rule of thumb is to
> *always* have a dedicated compatible entry? Can someone confirm this?

All compatible values that are in use must be documented.
Checkpatch just greps in Documentation/devicetree/bindings/.

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

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
       [not found]       ` <CAMuHMdWmGi6qKKt5YJm1i7FqDceqKosxsSRJRd8zqXO9jMzyoQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-01-15 18:54           ` Dmitry Torokhov
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 18:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 06:45:33PM +0100, Geert Uytterhoeven wrote:
> On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> 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 will contain the more specific
> > "adi,adxl345" or "adi,adxl346" value first. This prevents the device
> > node from being matched with the adxl34x driver.
> >
> > Fix this by adding an OF match table with an "adi,adxl345" compatible
> > entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
> > backward-compatible with the ADXL345 with differences handled by runtime
> > detection of the device model.
> 
> Thanks!
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/drivers/input/misc/adxl34x-i2c.c
> > +++ b/drivers/input/misc/adxl34x-i2c.c
> 
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id adxl34x_of_id[] = {
> > +       /*
> > +        * The ADXL346 is backward-compatible with the ADXL345. Differences are
> > +        * handled by runtime detection of the device model, there's thus no
> > +        * need for listing the "adi,adxl346" compatible value explicitly.
> > +        */
> > +       { .compatible = "adi,adxl345", },
> > +       /*
> > +        * Deprecated, DT nodes should use one or more of the device-specific
> > +        * compatible values "adi,adxl345" and "adi,adxl346".
> 
> Ideally, the two comments above are moved to a real DT binding document ;-)
> 
> > +        */
> > +       { .compatible = "adi,adxl34x", },
> 
> I'd append "/* deprecated */" to the line above, so "git grep adxl34x"
> will show its deprecated status.

I still do not understand what we are trying to fix here. Why is
"adi,adxl34x" compatible string no good anymore? If we start using exact
models and the physical device does not match do we abort probe? What is
the problem that we are solving here?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 18:54           ` Dmitry Torokhov
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 18:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 06:45:33PM +0100, Geert Uytterhoeven wrote:
> On Thu, Jan 15, 2015 at 3:54 PM, Laurent Pinchart
> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> 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 will contain the more specific
> > "adi,adxl345" or "adi,adxl346" value first. This prevents the device
> > node from being matched with the adxl34x driver.
> >
> > Fix this by adding an OF match table with an "adi,adxl345" compatible
> > entry. There's no need to add the "adi,adxl346" entry as the ADXL346 is
> > backward-compatible with the ADXL345 with differences handled by runtime
> > detection of the device model.
> 
> Thanks!
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> 
> Acked-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> 
> > --- a/drivers/input/misc/adxl34x-i2c.c
> > +++ b/drivers/input/misc/adxl34x-i2c.c
> 
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id adxl34x_of_id[] = {
> > +       /*
> > +        * The ADXL346 is backward-compatible with the ADXL345. Differences are
> > +        * handled by runtime detection of the device model, there's thus no
> > +        * need for listing the "adi,adxl346" compatible value explicitly.
> > +        */
> > +       { .compatible = "adi,adxl345", },
> > +       /*
> > +        * Deprecated, DT nodes should use one or more of the device-specific
> > +        * compatible values "adi,adxl345" and "adi,adxl346".
> 
> Ideally, the two comments above are moved to a real DT binding document ;-)
> 
> > +        */
> > +       { .compatible = "adi,adxl34x", },
> 
> I'd append "/* deprecated */" to the line above, so "git grep adxl34x"
> will show its deprecated status.

I still do not understand what we are trying to fix here. Why is
"adi,adxl34x" compatible string no good anymore? If we start using exact
models and the physical device does not match do we abort probe? What is
the problem that we are solving here?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 18:54           ` Dmitry Torokhov
@ 2015-01-15 20:00             ` Geert Uytterhoeven
  -1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 20:00 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> I still do not understand what we are trying to fix here. Why is
> "adi,adxl34x" compatible string no good anymore? If we start using exact
> models and the physical device does not match do we abort probe? What is
> the problem that we are solving here?

Because there's no guarantee that the driver actually supports all
"adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

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

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 20:00             ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-01-15 20:00 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Laurent Pinchart, linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov
<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> I still do not understand what we are trying to fix here. Why is
> "adi,adxl34x" compatible string no good anymore? If we start using exact
> models and the physical device does not match do we abort probe? What is
> the problem that we are solving here?

Because there's no guarantee that the driver actually supports all
"adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

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

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
       [not found]             ` <CAMuHMdXrRtgAahaEUh8x61E-koE25VL-6LOJrkjx4_fohKQtwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-01-15 20:34                 ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 20:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Dmitry Torokhov, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Wolfram Sang

On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > I still do not understand what we are trying to fix here. Why is
> > "adi,adxl34x" compatible string no good anymore? If we start using exact
> > models and the physical device does not match do we abort probe? What is
> > the problem that we are solving here?
> 
> Because there's no guarantee that the driver actually supports all
> "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

That's one of the reasons. Another one is that the adxl34x driver won't match 
DT nodes that list the "adi,adxl34x" compatible value in positions other than 
the first.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 20:34                 ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 20:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Dmitry Torokhov, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Wolfram Sang

On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > I still do not understand what we are trying to fix here. Why is
> > "adi,adxl34x" compatible string no good anymore? If we start using exact
> > models and the physical device does not match do we abort probe? What is
> > the problem that we are solving here?
> 
> Because there's no guarantee that the driver actually supports all
> "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

That's one of the reasons. Another one is that the adxl34x driver won't match 
DT nodes that list the "adi,adxl34x" compatible value in positions other than 
the first.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 17:43             ` Wolfram Sang
@ 2015-01-15 20:51               ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 20:51 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list

Hi Wolfram,

On Thursday 15 January 2015 18:43:33 Wolfram Sang wrote:
> On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
> > On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >>> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
> >>> 
> >>>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
> >>>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
> >>>  adi,adxl345          Three-Axis Digital Accelerometer
> >>> 
> >>> -adi,adxl346          Three-Axis Digital Accelerometer
> >>> -adi,adxl34x          Three-Axis Digital Accelerometer
> >>> +adi,adxl346          Three-Axis Digital Accelerometer
> >>> (backward-compatibility value "adi,adxl345" must be listed too)
> >>
> >> I'd rather drop 346 because there is no compatible for that one
> >> anywhere. No need to resend, I can fix it here...
> > 
> > If you drop adi,adxl346, checkpatch will start complaining if it
> > encounters it in a .dts.
> 
> Boah, this is annoying. That means we need an 346 entry even if it is
> not different from 345 (which is fine by me).
> 
> If checkpatch does it this way, that means the rule of thumb is to
> *always* have a dedicated compatible entry? Can someone confirm this?

I believe we should register a new compatible entry for a device when the 
device isn't identical, from a compatibility point of view, to an already 
registered device. In this specific case the adxl346 offers addition features 
compared to the adxl345. It thus qualify for its own compatible string. Its DT 
nodes should list both the adi,adxl346 and adi,adxl345 compatible strings in 
that order, as the chip is compatible with the adxl345. On the driver side, 
given that we don't need to differentiate between the devices based on the 
compatible string (as runtime model detection is possible) we don't need to 
add a match entry for adi,adxl346.

> Why did we discuss then? Now, I am confused as well...

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-15 20:51               ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 20:51 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list

Hi Wolfram,

On Thursday 15 January 2015 18:43:33 Wolfram Sang wrote:
> On Thu, Jan 15, 2015 at 06:32:31PM +0100, Geert Uytterhoeven wrote:
> > On Thu, Jan 15, 2015 at 6:02 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >>> @@ -18,8 +18,7 @@ adi,adt7475         +/-1C TDM Extended Temp Range I.C
> >>> 
> >>>  adi,adt7476          +/-1C TDM Extended Temp Range I.C
> >>>  adi,adt7490          +/-1C TDM Extended Temp Range I.C
> >>>  adi,adxl345          Three-Axis Digital Accelerometer
> >>> 
> >>> -adi,adxl346          Three-Axis Digital Accelerometer
> >>> -adi,adxl34x          Three-Axis Digital Accelerometer
> >>> +adi,adxl346          Three-Axis Digital Accelerometer
> >>> (backward-compatibility value "adi,adxl345" must be listed too)
> >>
> >> I'd rather drop 346 because there is no compatible for that one
> >> anywhere. No need to resend, I can fix it here...
> > 
> > If you drop adi,adxl346, checkpatch will start complaining if it
> > encounters it in a .dts.
> 
> Boah, this is annoying. That means we need an 346 entry even if it is
> not different from 345 (which is fine by me).
> 
> If checkpatch does it this way, that means the rule of thumb is to
> *always* have a dedicated compatible entry? Can someone confirm this?

I believe we should register a new compatible entry for a device when the 
device isn't identical, from a compatibility point of view, to an already 
registered device. In this specific case the adxl346 offers addition features 
compared to the adxl345. It thus qualify for its own compatible string. Its DT 
nodes should list both the adi,adxl346 and adi,adxl345 compatible strings in 
that order, as the chip is compatible with the adxl345. On the driver side, 
given that we don't need to differentiate between the devices based on the 
compatible string (as runtime model detection is possible) we don't need to 
add a match entry for adi,adxl346.

> Why did we discuss then? Now, I am confused as well...

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 20:34                 ` Laurent Pinchart
@ 2015-01-15 21:06                   ` Dmitry Torokhov
  -1 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:06 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Wolfram Sang

On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> > On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > > I still do not understand what we are trying to fix here. Why is
> > > "adi,adxl34x" compatible string no good anymore? If we start using exact
> > > models and the physical device does not match do we abort probe? What is
> > > the problem that we are solving here?
> > 
> > Because there's no guarantee that the driver actually supports all
> > "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

So, what? When we encounter such devices and decide that we actually
want a different driver for them (instead of enhancing the existing one)
we'll give them their own compatible string. It's not like "adi,adxl348"
will automatically match with "adi,adxl34x", is it?

> 
> That's one of the reasons. Another one is that the adxl34x driver won't match 
> DT nodes that list the "adi,adxl34x" compatible value in positions other than 
> the first.

Will it match anything else in the position other than 1st (i.e.
if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
Why "adi,adxl34x" is special?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 21:06                   ` Dmitry Torokhov
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:06 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Wolfram Sang

On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> > On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > > I still do not understand what we are trying to fix here. Why is
> > > "adi,adxl34x" compatible string no good anymore? If we start using exact
> > > models and the physical device does not match do we abort probe? What is
> > > the problem that we are solving here?
> > 
> > Because there's no guarantee that the driver actually supports all
> > "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

So, what? When we encounter such devices and decide that we actually
want a different driver for them (instead of enhancing the existing one)
we'll give them their own compatible string. It's not like "adi,adxl348"
will automatically match with "adi,adxl34x", is it?

> 
> That's one of the reasons. Another one is that the adxl34x driver won't match 
> DT nodes that list the "adi,adxl34x" compatible value in positions other than 
> the first.

Will it match anything else in the position other than 1st (i.e.
if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
Why "adi,adxl34x" is special?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 21:06                   ` Dmitry Torokhov
@ 2015-01-15 21:34                     ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 21:34 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote:
> On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> > On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> > > On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > > > I still do not understand what we are trying to fix here. Why is
> > > > "adi,adxl34x" compatible string no good anymore? If we start using
> > > > exact models and the physical device does not match do we abort probe?
> > > > What is the problem that we are solving here?
> > > 
> > > Because there's no guarantee that the driver actually supports all
> > > "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.
> 
> So, what? When we encounter such devices and decide that we actually
> want a different driver for them (instead of enhancing the existing one)
> we'll give them their own compatible string. It's not like "adi,adxl348"
> will automatically match with "adi,adxl34x", is it?

Please remember that compatible strings shouldn't be decided based on a 
particular operating system implementation.

> > That's one of the reasons. Another one is that the adxl34x driver won't
> > match DT nodes that list the "adi,adxl34x" compatible value in positions
> > other than the first.
> 
> Will it match anything else in the position other than 1st (i.e.
> if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
> Why "adi,adxl34x" is special?

The problem on the driver side is that the automatic I2C DT compatible to 
device name matching implementation only tries to match with the first 
compatible string without looking at the other ones. The driver thus needs to 
be enhanced with an explicit OF match table to be able to match on DT nodes 
that specify "adi,adxl345" or "adi,adxl346" as the first compatible entry.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 21:34                     ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 21:34 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote:
> On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> > On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> > > On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > > > I still do not understand what we are trying to fix here. Why is
> > > > "adi,adxl34x" compatible string no good anymore? If we start using
> > > > exact models and the physical device does not match do we abort probe?
> > > > What is the problem that we are solving here?
> > > 
> > > Because there's no guarantee that the driver actually supports all
> > > "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.
> 
> So, what? When we encounter such devices and decide that we actually
> want a different driver for them (instead of enhancing the existing one)
> we'll give them their own compatible string. It's not like "adi,adxl348"
> will automatically match with "adi,adxl34x", is it?

Please remember that compatible strings shouldn't be decided based on a 
particular operating system implementation.

> > That's one of the reasons. Another one is that the adxl34x driver won't
> > match DT nodes that list the "adi,adxl34x" compatible value in positions
> > other than the first.
> 
> Will it match anything else in the position other than 1st (i.e.
> if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
> Why "adi,adxl34x" is special?

The problem on the driver side is that the automatic I2C DT compatible to 
device name matching implementation only tries to match with the first 
compatible string without looking at the other ones. The driver thus needs to 
be enhanced with an explicit OF match table to be able to match on DT nodes 
that specify "adi,adxl345" or "adi,adxl346" as the first compatible entry.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 21:34                     ` Laurent Pinchart
@ 2015-01-15 21:50                       ` Dmitry Torokhov
  -1 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:50 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 11:34:00PM +0200, Laurent Pinchart wrote:
> On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote:
> > On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> > > On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> > > > On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > > > > I still do not understand what we are trying to fix here. Why is
> > > > > "adi,adxl34x" compatible string no good anymore? If we start using
> > > > > exact models and the physical device does not match do we abort probe?
> > > > > What is the problem that we are solving here?
> > > > 
> > > > Because there's no guarantee that the driver actually supports all
> > > > "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.
> > 
> > So, what? When we encounter such devices and decide that we actually
> > want a different driver for them (instead of enhancing the existing one)
> > we'll give them their own compatible string. It's not like "adi,adxl348"
> > will automatically match with "adi,adxl34x", is it?
> 
> Please remember that compatible strings shouldn't be decided based on a 
> particular operating system implementation.

In this case we can never have generic compatible strings of whatever
since in 10 years there might appear an OS that handles them
differently. Let's be a bit realistic here as well.

> 
> > > That's one of the reasons. Another one is that the adxl34x driver won't
> > > match DT nodes that list the "adi,adxl34x" compatible value in positions
> > > other than the first.
> > 
> > Will it match anything else in the position other than 1st (i.e.
> > if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
> > Why "adi,adxl34x" is special?
> 
> The problem on the driver side is that the automatic I2C DT compatible to 
> device name matching implementation only tries to match with the first 
> compatible string without looking at the other ones. The driver thus needs to 
> be enhanced with an explicit OF match table to be able to match on DT nodes 
> that specify "adi,adxl345" or "adi,adxl346" as the first compatible entry.

Why don't we enhance I2C core instead to do proper matching?

-- 
Dmitry

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 21:50                       ` Dmitry Torokhov
  0 siblings, 0 replies; 54+ messages in thread
From: Dmitry Torokhov @ 2015-01-15 21:50 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

On Thu, Jan 15, 2015 at 11:34:00PM +0200, Laurent Pinchart wrote:
> On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote:
> > On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> > > On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> > > > On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> > > > > I still do not understand what we are trying to fix here. Why is
> > > > > "adi,adxl34x" compatible string no good anymore? If we start using
> > > > > exact models and the physical device does not match do we abort probe?
> > > > > What is the problem that we are solving here?
> > > > 
> > > > Because there's no guarantee that the driver actually supports all
> > > > "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.
> > 
> > So, what? When we encounter such devices and decide that we actually
> > want a different driver for them (instead of enhancing the existing one)
> > we'll give them their own compatible string. It's not like "adi,adxl348"
> > will automatically match with "adi,adxl34x", is it?
> 
> Please remember that compatible strings shouldn't be decided based on a 
> particular operating system implementation.

In this case we can never have generic compatible strings of whatever
since in 10 years there might appear an OS that handles them
differently. Let's be a bit realistic here as well.

> 
> > > That's one of the reasons. Another one is that the adxl34x driver won't
> > > match DT nodes that list the "adi,adxl34x" compatible value in positions
> > > other than the first.
> > 
> > Will it match anything else in the position other than 1st (i.e.
> > if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
> > Why "adi,adxl34x" is special?
> 
> The problem on the driver side is that the automatic I2C DT compatible to 
> device name matching implementation only tries to match with the first 
> compatible string without looking at the other ones. The driver thus needs to 
> be enhanced with an explicit OF match table to be able to match on DT nodes 
> that specify "adi,adxl345" or "adi,adxl346" as the first compatible entry.

Why don't we enhance I2C core instead to do proper matching?

-- 
Dmitry

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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 20:34                 ` Laurent Pinchart
@ 2015-01-15 22:05                   ` Sergei Shtylyov
  -1 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2015-01-15 22:05 UTC (permalink / raw)
  To: Laurent Pinchart, Geert Uytterhoeven
  Cc: Dmitry Torokhov, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

Hello.

On 01/15/2015 11:34 PM, Laurent Pinchart wrote:

>>> I still do not understand what we are trying to fix here. Why is
>>> "adi,adxl34x" compatible string no good anymore? If we start using exact
>>> models and the physical device does not match do we abort probe? What is
>>> the problem that we are solving here?

>> Because there's no guarantee that the driver actually supports all
>> "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

> That's one of the reasons. Another one is that the adxl34x driver won't match
> DT nodes that list the "adi,adxl34x" compatible value in positions other than
> the first.

    Let's also not forget that wildcards in the "compatible" prop are not 
really allowed.

WBR, Sergei


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 22:05                   ` Sergei Shtylyov
  0 siblings, 0 replies; 54+ messages in thread
From: Sergei Shtylyov @ 2015-01-15 22:05 UTC (permalink / raw)
  To: Laurent Pinchart, Geert Uytterhoeven
  Cc: Dmitry Torokhov, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

Hello.

On 01/15/2015 11:34 PM, Laurent Pinchart wrote:

>>> I still do not understand what we are trying to fix here. Why is
>>> "adi,adxl34x" compatible string no good anymore? If we start using exact
>>> models and the physical device does not match do we abort probe? What is
>>> the problem that we are solving here?

>> Because there's no guarantee that the driver actually supports all
>> "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.

> That's one of the reasons. Another one is that the adxl34x driver won't match
> DT nodes that list the "adi,adxl34x" compatible value in positions other than
> the first.

    Let's also not forget that wildcards in the "compatible" prop are not 
really allowed.

WBR, Sergei


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
  2015-01-15 21:50                       ` Dmitry Torokhov
@ 2015-01-15 22:09                         ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 22:09 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

Hi Dmitry,

On Thursday 15 January 2015 13:50:53 Dmitry Torokhov wrote:
> On Thu, Jan 15, 2015 at 11:34:00PM +0200, Laurent Pinchart wrote:
> > On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote:
> >> On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> >>> On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> >>>> On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> >>>>> I still do not understand what we are trying to fix here. Why is
> >>>>> "adi,adxl34x" compatible string no good anymore? If we start using
> >>>>> exact models and the physical device does not match do we abort
> >>>>> probe? What is the problem that we are solving here?
> >>>> 
> >>>> Because there's no guarantee that the driver actually supports all
> >>>> "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.
> >> 
> >> So, what? When we encounter such devices and decide that we actually
> >> want a different driver for them (instead of enhancing the existing one)
> >> we'll give them their own compatible string. It's not like "adi,adxl348"
> >> will automatically match with "adi,adxl34x", is it?
> > 
> > Please remember that compatible strings shouldn't be decided based on a
> > particular operating system implementation.
> 
> In this case we can never have generic compatible strings of whatever
> since in 10 years there might appear an OS that handles them
> differently. Let's be a bit realistic here as well.

I don't agree with you. The generic compatible strings should express 
compatibility with a hardware interface to the system, not compatibility with 
particular drivers on particular operating systems. We can thus have generic 
compatible strings without taking the OS into account.

> >>> That's one of the reasons. Another one is that the adxl34x driver
> >>> won't match DT nodes that list the "adi,adxl34x" compatible value in
> >>> positions other than the first.
> >> 
> >> Will it match anything else in the position other than 1st (i.e.
> >> if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
> > > Why "adi,adxl34x" is special?
> > 
> > The problem on the driver side is that the automatic I2C DT compatible to
> > device name matching implementation only tries to match with the first
> > compatible string without looking at the other ones. The driver thus needs
> > to be enhanced with an explicit OF match table to be able to match on DT
> > nodes that specify "adi,adxl345" or "adi,adxl346" as the first compatible
> > entry.
>
> Why don't we enhance I2C core instead to do proper matching?

That would also be possible, but I believe that patch 1/2 is still the right 
thing to do, in which case patch 2/2 is required anyway.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 2/2] input: adxl34x: Add OF match support
@ 2015-01-15 22:09                         ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-01-15 22:09 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list, Wolfram Sang

Hi Dmitry,

On Thursday 15 January 2015 13:50:53 Dmitry Torokhov wrote:
> On Thu, Jan 15, 2015 at 11:34:00PM +0200, Laurent Pinchart wrote:
> > On Thursday 15 January 2015 13:06:32 Dmitry Torokhov wrote:
> >> On Thu, Jan 15, 2015 at 10:34:29PM +0200, Laurent Pinchart wrote:
> >>> On Thursday 15 January 2015 21:00:37 Geert Uytterhoeven wrote:
> >>>> On Thu, Jan 15, 2015 at 7:54 PM, Dmitry Torokhov wrote:
> >>>>> I still do not understand what we are trying to fix here. Why is
> >>>>> "adi,adxl34x" compatible string no good anymore? If we start using
> >>>>> exact models and the physical device does not match do we abort
> >>>>> probe? What is the problem that we are solving here?
> >>>> 
> >>>> Because there's no guarantee that the driver actually supports all
> >>>> "adi,adxl34<x>" with <x> = 0..9, some of which don't exist yet.
> >> 
> >> So, what? When we encounter such devices and decide that we actually
> >> want a different driver for them (instead of enhancing the existing one)
> >> we'll give them their own compatible string. It's not like "adi,adxl348"
> >> will automatically match with "adi,adxl34x", is it?
> > 
> > Please remember that compatible strings shouldn't be decided based on a
> > particular operating system implementation.
> 
> In this case we can never have generic compatible strings of whatever
> since in 10 years there might appear an OS that handles them
> differently. Let's be a bit realistic here as well.

I don't agree with you. The generic compatible strings should express 
compatibility with a hardware interface to the system, not compatibility with 
particular drivers on particular operating systems. We can thus have generic 
compatible strings without taking the OS into account.

> >>> That's one of the reasons. Another one is that the adxl34x driver
> >>> won't match DT nodes that list the "adi,adxl34x" compatible value in
> >>> positions other than the first.
> >> 
> >> Will it match anything else in the position other than 1st (i.e.
> >> if device has compatible sting like "adi,adxl345-1", "adi,adxl345")?
> > > Why "adi,adxl34x" is special?
> > 
> > The problem on the driver side is that the automatic I2C DT compatible to
> > device name matching implementation only tries to match with the first
> > compatible string without looking at the other ones. The driver thus needs
> > to be enhanced with an explicit OF match table to be able to match on DT
> > nodes that specify "adi,adxl345" or "adi,adxl346" as the first compatible
> > entry.
>
> Why don't we enhance I2C core instead to do proper matching?

That would also be possible, but I believe that patch 1/2 is still the right 
thing to do, in which case patch 2/2 is required anyway.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-15 17:51               ` Geert Uytterhoeven
@ 2015-01-26 12:09                 ` Wolfram Sang
  -1 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-26 12:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, linux-input, Linux I2C, Linux-sh list

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


> >> If you drop adi,adxl346, checkpatch will start complaining if it encounters
> >> it in a .dts.
> >
> > Boah, this is annoying. That means we need an 346 entry even if it is
> > not different from 345 (which is fine by me).
> 
> To be clear: you need the entry in the documentation. It can be omitted
> from the driver if it's not (yet) used for matching.

Well, I don't really like that behaviour, but I don't like
i2c/trivial-devices.txt as well, so I'll just apply the patch and live
with it :)


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-26 12:09                 ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-26 12:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, linux-input, Linux I2C, Linux-sh list

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


> >> If you drop adi,adxl346, checkpatch will start complaining if it encounters
> >> it in a .dts.
> >
> > Boah, this is annoying. That means we need an 346 entry even if it is
> > not different from 345 (which is fine by me).
> 
> To be clear: you need the entry in the documentation. It can be omitted
> from the driver if it's not (yet) used for matching.

Well, I don't really like that behaviour, but I don't like
i2c/trivial-devices.txt as well, so I'll just apply the patch and live
with it :)


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
       [not found]   ` <1421333655-31029-2-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
@ 2015-01-26 12:12       ` Wolfram Sang
  2015-01-26 12:12       ` Wolfram Sang
  1 sibling, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-26 12:12 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven

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

On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote:
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

On the other hand, I'll just ack this patch, so Dmitry can take both of
them. D'accord?

Acked-by: Wolfram Sang <wsa@the-dreams.de>


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-01-26 12:12       ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-01-26 12:12 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven

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

On Thu, Jan 15, 2015 at 04:54:14PM +0200, Laurent Pinchart wrote:
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnhk3lzF8UVTdg@public.gmane.orgm>

On the other hand, I'll just ack this patch, so Dmitry can take both of
them. D'accord?

Acked-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-01-26 12:09                 ` Wolfram Sang
@ 2015-02-26 14:27                   ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-02-26 14:27 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list

Hi Wolfram,

On Monday 26 January 2015 13:09:47 Wolfram Sang wrote:
> > >> If you drop adi,adxl346, checkpatch will start complaining if it
> > >> encounters
> > >> it in a .dts.
> > > 
> > > Boah, this is annoying. That means we need an 346 entry even if it is
> > > not different from 345 (which is fine by me).
> > 
> > To be clear: you need the entry in the documentation. It can be omitted
> > from the driver if it's not (yet) used for matching.
> 
> Well, I don't really like that behaviour, but I don't like
> i2c/trivial-devices.txt as well, so I'll just apply the patch and live
> with it :)

What happened to this patch ?

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-02-26 14:27                   ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-02-26 14:27 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list

Hi Wolfram,

On Monday 26 January 2015 13:09:47 Wolfram Sang wrote:
> > >> If you drop adi,adxl346, checkpatch will start complaining if it
> > >> encounters
> > >> it in a .dts.
> > > 
> > > Boah, this is annoying. That means we need an 346 entry even if it is
> > > not different from 345 (which is fine by me).
> > 
> > To be clear: you need the entry in the documentation. It can be omitted
> > from the driver if it's not (yet) used for matching.
> 
> Well, I don't really like that behaviour, but I don't like
> i2c/trivial-devices.txt as well, so I'll just apply the patch and live
> with it :)

What happened to this patch ?

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-02-26 14:27                   ` Laurent Pinchart
@ 2015-03-02  6:40                     ` Wolfram Sang
  -1 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-03-02  6:40 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list

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

On Thu, Feb 26, 2015 at 04:27:49PM +0200, Laurent Pinchart wrote:
> Hi Wolfram,
> 
> On Monday 26 January 2015 13:09:47 Wolfram Sang wrote:
> > > >> If you drop adi,adxl346, checkpatch will start complaining if it
> > > >> encounters
> > > >> it in a .dts.
> > > > 
> > > > Boah, this is annoying. That means we need an 346 entry even if it is
> > > > not different from 345 (which is fine by me).
> > > 
> > > To be clear: you need the entry in the documentation. It can be omitted
> > > from the driver if it's not (yet) used for matching.
> > 
> > Well, I don't really like that behaviour, but I don't like
> > i2c/trivial-devices.txt as well, so I'll just apply the patch and live
> > with it :)
> 
> What happened to this patch ?

My idea was that Dmitry takes them both because they are related:

http://article.gmane.org/gmane.linux.drivers.i2c/21763


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-03-02  6:40                     ` Wolfram Sang
  0 siblings, 0 replies; 54+ messages in thread
From: Wolfram Sang @ 2015-03-02  6:40 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart, linux-input, Linux I2C,
	Linux-sh list

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

On Thu, Feb 26, 2015 at 04:27:49PM +0200, Laurent Pinchart wrote:
> Hi Wolfram,
> 
> On Monday 26 January 2015 13:09:47 Wolfram Sang wrote:
> > > >> If you drop adi,adxl346, checkpatch will start complaining if it
> > > >> encounters
> > > >> it in a .dts.
> > > > 
> > > > Boah, this is annoying. That means we need an 346 entry even if it is
> > > > not different from 345 (which is fine by me).
> > > 
> > > To be clear: you need the entry in the documentation. It can be omitted
> > > from the driver if it's not (yet) used for matching.
> > 
> > Well, I don't really like that behaviour, but I don't like
> > i2c/trivial-devices.txt as well, so I'll just apply the patch and live
> > with it :)
> 
> What happened to this patch ?

My idea was that Dmitry takes them both because they are related:

http://article.gmane.org/gmane.linux.drivers.i2c/21763


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

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-03-02  6:40                     ` Wolfram Sang
@ 2015-03-02 22:52                       ` Laurent Pinchart
  -1 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-03-02 22:52 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Dmitry Torokhov

(CC'ing Dmitry)

On Monday 02 March 2015 07:40:49 Wolfram Sang wrote:
> On Thu, Feb 26, 2015 at 04:27:49PM +0200, Laurent Pinchart wrote:
> > On Monday 26 January 2015 13:09:47 Wolfram Sang wrote:
> >>>>> If you drop adi,adxl346, checkpatch will start complaining if it
> >>>>> encounters it in a .dts.
> >>>> 
> >>>> Boah, this is annoying. That means we need an 346 entry even if it
> >>>> is not different from 345 (which is fine by me).
> >>>
> >>> To be clear: you need the entry in the documentation. It can be
> >>> omitted from the driver if it's not (yet) used for matching.
> >> 
> >> Well, I don't really like that behaviour, but I don't like
> >> i2c/trivial-devices.txt as well, so I'll just apply the patch and live
> >> with it :)
> > 
> > What happened to this patch ?
> 
> My idea was that Dmitry takes them both because they are related:
> 
> http://article.gmane.org/gmane.linux.drivers.i2c/21763

I'm fine with that. Dmitry ?

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-03-02 22:52                       ` Laurent Pinchart
  0 siblings, 0 replies; 54+ messages in thread
From: Laurent Pinchart @ 2015-03-02 22:52 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Laurent Pinchart,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Linux I2C, Linux-sh list,
	Dmitry Torokhov

(CC'ing Dmitry)

On Monday 02 March 2015 07:40:49 Wolfram Sang wrote:
> On Thu, Feb 26, 2015 at 04:27:49PM +0200, Laurent Pinchart wrote:
> > On Monday 26 January 2015 13:09:47 Wolfram Sang wrote:
> >>>>> If you drop adi,adxl346, checkpatch will start complaining if it
> >>>>> encounters it in a .dts.
> >>>> 
> >>>> Boah, this is annoying. That means we need an 346 entry even if it
> >>>> is not different from 345 (which is fine by me).
> >>>
> >>> To be clear: you need the entry in the documentation. It can be
> >>> omitted from the driver if it's not (yet) used for matching.
> >> 
> >> Well, I don't really like that behaviour, but I don't like
> >> i2c/trivial-devices.txt as well, so I'll just apply the patch and live
> >> with it :)
> > 
> > What happened to this patch ?
> 
> My idea was that Dmitry takes them both because they are related:
> 
> http://article.gmane.org/gmane.linux.drivers.i2c/21763

I'm fine with that. Dmitry ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-05-21 11:42   ` Geert Uytterhoeven
@ 2015-05-22  1:32     ` Simon Horman
  -1 siblings, 0 replies; 54+ messages in thread
From: Simon Horman @ 2015-05-22  1:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Dmitry Torokhov, Laurent Pinchart, Wolfram Sang, linux-input,
	linux-i2c, devicetree, linux-sh

On Thu, May 21, 2015 at 01:42:25PM +0200, Geert Uytterhoeven wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Add Acked-by.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


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

* Re: [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-05-22  1:32     ` Simon Horman
  0 siblings, 0 replies; 54+ messages in thread
From: Simon Horman @ 2015-05-22  1:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Dmitry Torokhov, Laurent Pinchart, Wolfram Sang, linux-input,
	linux-i2c, devicetree, linux-sh

On Thu, May 21, 2015 at 01:42:25PM +0200, Geert Uytterhoeven wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> DT nodes should use the more specific adi,adxl345 and adi,adxl346
> compatible values instead. As the ADXL346 is backward-compatible with
> the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
> in that order.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Add Acked-by.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


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

* [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
  2015-05-21 11:42 [PATCH v2 0/2] Fix OF match for adxl34x driver Geert Uytterhoeven
@ 2015-05-21 11:42   ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-05-21 11:42 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Laurent Pinchart, Wolfram Sang, linux-input, linux-i2c,
	devicetree, linux-sh, Geert Uytterhoeven

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

DT nodes should use the more specific adi,adxl345 and adi,adxl346
compatible values instead. As the ADXL346 is backward-compatible with
the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
in that order.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Add Acked-by.

 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index ad0c4ac916dd75ed..00f8652e193a940c 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -19,8 +19,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
 adi,adt7476		+/-1C TDM Extended Temp Range I.C
 adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
-adi,adxl346		Three-Axis Digital Accelerometer
-adi,adxl34x		Three-Axis Digital Accelerometer
+adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,24c00		i2c serial eeprom  (24cxx)
 atmel,24c01		i2c serial eeprom  (24cxx)
-- 
1.9.1


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

* [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string
@ 2015-05-21 11:42   ` Geert Uytterhoeven
  0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2015-05-21 11:42 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Laurent Pinchart, Wolfram Sang, linux-input, linux-i2c,
	devicetree, linux-sh, Geert Uytterhoeven

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

DT nodes should use the more specific adi,adxl345 and adi,adxl346
compatible values instead. As the ADXL346 is backward-compatible with
the ADXL345, ADXL346 nodes must list both adi,adxl346 and adi,adxl345,
in that order.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Add Acked-by.

 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index ad0c4ac916dd75ed..00f8652e193a940c 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -19,8 +19,7 @@ adi,adt7475		+/-1C TDM Extended Temp Range I.C
 adi,adt7476		+/-1C TDM Extended Temp Range I.C
 adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
-adi,adxl346		Three-Axis Digital Accelerometer
-adi,adxl34x		Three-Axis Digital Accelerometer
+adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,24c00		i2c serial eeprom  (24cxx)
 atmel,24c01		i2c serial eeprom  (24cxx)
-- 
1.9.1


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

end of thread, other threads:[~2015-05-22  1:32 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-15 14:54 [PATCH v2 0/2] Fix OF match for adxl34x driver Laurent Pinchart
2015-01-15 14:54 ` Laurent Pinchart
2015-01-15 14:54 ` [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string Laurent Pinchart
2015-01-15 14:54   ` Laurent Pinchart
2015-01-15 17:49   ` Geert Uytterhoeven
2015-01-15 17:49     ` Geert Uytterhoeven
     [not found]   ` <1421333655-31029-2-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2015-01-15 17:02     ` Wolfram Sang
2015-01-15 17:02       ` Wolfram Sang
2015-01-15 17:27       ` Dmitry Torokhov
2015-01-15 17:27         ` Dmitry Torokhov
2015-01-15 17:32       ` Geert Uytterhoeven
2015-01-15 17:32         ` Geert Uytterhoeven
     [not found]         ` <CAMuHMdW7ETcFGSPiE9BWA2dAE93477fzoyF-+_EaiPSDT9WMWA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-01-15 17:43           ` Wolfram Sang
2015-01-15 17:43             ` Wolfram Sang
2015-01-15 17:51             ` Geert Uytterhoeven
2015-01-15 17:51               ` Geert Uytterhoeven
2015-01-26 12:09               ` Wolfram Sang
2015-01-26 12:09                 ` Wolfram Sang
2015-02-26 14:27                 ` Laurent Pinchart
2015-02-26 14:27                   ` Laurent Pinchart
2015-03-02  6:40                   ` Wolfram Sang
2015-03-02  6:40                     ` Wolfram Sang
2015-03-02 22:52                     ` Laurent Pinchart
2015-03-02 22:52                       ` Laurent Pinchart
2015-01-15 20:51             ` Laurent Pinchart
2015-01-15 20:51               ` Laurent Pinchart
2015-01-26 12:12     ` Wolfram Sang
2015-01-26 12:12       ` Wolfram Sang
2015-01-15 14:54 ` [PATCH v2 2/2] input: adxl34x: Add OF match support Laurent Pinchart
2015-01-15 14:54   ` Laurent Pinchart
2015-01-15 16:55   ` Wolfram Sang
2015-01-15 16:55     ` Wolfram Sang
     [not found]   ` <1421333655-31029-3-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2015-01-15 17:45     ` Geert Uytterhoeven
2015-01-15 17:45       ` Geert Uytterhoeven
     [not found]       ` <CAMuHMdWmGi6qKKt5YJm1i7FqDceqKosxsSRJRd8zqXO9jMzyoQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-01-15 18:54         ` Dmitry Torokhov
2015-01-15 18:54           ` Dmitry Torokhov
2015-01-15 20:00           ` Geert Uytterhoeven
2015-01-15 20:00             ` Geert Uytterhoeven
     [not found]             ` <CAMuHMdXrRtgAahaEUh8x61E-koE25VL-6LOJrkjx4_fohKQtwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-01-15 20:34               ` Laurent Pinchart
2015-01-15 20:34                 ` Laurent Pinchart
2015-01-15 21:06                 ` Dmitry Torokhov
2015-01-15 21:06                   ` Dmitry Torokhov
2015-01-15 21:34                   ` Laurent Pinchart
2015-01-15 21:34                     ` Laurent Pinchart
2015-01-15 21:50                     ` Dmitry Torokhov
2015-01-15 21:50                       ` Dmitry Torokhov
2015-01-15 22:09                       ` Laurent Pinchart
2015-01-15 22:09                         ` Laurent Pinchart
2015-01-15 22:05                 ` Sergei Shtylyov
2015-01-15 22:05                   ` Sergei Shtylyov
2015-05-21 11:42 [PATCH v2 0/2] Fix OF match for adxl34x driver Geert Uytterhoeven
2015-05-21 11:42 ` [PATCH v2 1/2] DT: i2c: Deprecate adi,adxl34x compatible string Geert Uytterhoeven
2015-05-21 11:42   ` Geert Uytterhoeven
2015-05-22  1:32   ` Simon Horman
2015-05-22  1:32     ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.