[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Maxime Ripard]

Hi everyone,

This is an attempt at adding the DT probing to the st_sensors
accelerometers and gyroscopes.

It's somewhat hackish, because the core actively checks for the value
of the device at probe time, and DT probing generates a quite
different name.

I first tried to turn the function st_sensors_check_device_support the
other way around, with the "leaf" providing the sensor structure, and
the function checking that it's consistent with the WAI stored in the
sensor. However, it turned to be a quite intrusive change, so I'm
sending the easiest one, to try to get some feedback :)

Thanks!
Maxime

Maxime Ripard (3):
  iio: accel: Add device tree probing for STMicro accelerometers
  iio: accel: Add device tree probing for STMicro gyroscopes
  ARM: cfa10057: Add the accelerometer and gyroscope to the device tree

 .../devicetree/bindings/iio/accel/st_accel_i2c.txt | 22 +++++++++++++++
 .../devicetree/bindings/iio/gyro/st_gyro_i2c.txt   | 20 +++++++++++++
 arch/arm/boot/dts/imx28-cfa10057.dts               | 13 +++++++++
 drivers/iio/accel/st_accel_i2c.c                   | 33 ++++++++++++++++++++++
 drivers/iio/gyro/st_gyro_i2c.c                     | 31 ++++++++++++++++++++
 5 files changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
 create mode 100644 Documentation/devicetree/bindings/iio/gyro/st_gyro_i2c.txt

-- 
1.8.4.2

[RFC PATCH 1/3] iio: accel: Add device tree probing for STMicro accelerometers

[Maxime Ripard]

Add the compatibles supported by the st_sensors library. This uses kind
of a hack, since the st_sensors core will actively check at probe time
that the device name matches the one reported when using old style i2c
probing, and that this name will be different with device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/iio/accel/st_accel_i2c.txt | 22 +++++++++++++++
 drivers/iio/accel/st_accel_i2c.c                   | 33 ++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt

diff --git a/Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt b/Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
new file mode 100644
index 000000000000..4a1efdf1775f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
@@ -0,0 +1,22 @@
+* ST Micro accelerometer sensors 
+
+Required properties:
+  - compatible : should be either:
+	* "st,lis3dh"
+	* "st,lis331dlh"
+	* "st,lsm303dl-accel"
+	* "st,lsm303dlh-accel"
+	* "st,lsm303dlhc-accel"
+	* "st,lsm303dlm-accel"
+	* "st,lsm330-accel"
+	* "st,lsm330d-accel"
+	* "st,lsm330dl-accel"
+	* "st,lsm330dlc-accel"
+  - reg : the I2C address of the sensor
+
+Example:
+
+accel: accel at 19 {
+	compatible = "st,lsm330dlc-accel";
+	reg = <0x19>;
+};
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
index d7bedbdfc81d..de2bf76378d8 100644
--- a/drivers/iio/accel/st_accel_i2c.c
+++ b/drivers/iio/accel/st_accel_i2c.c
@@ -13,11 +13,27 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
+#include <linux/of_device.h>
 
 #include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_i2c.h>
 #include "st_accel.h"
 
+static const struct of_device_id st_accel_of_table[] = {
+	{ .compatible = "st,lis3dh", .data = LIS3DH_ACCEL_DEV_NAME },
+	{ .compatible = "st,lis331dlh", .data = LIS331DLH_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm303dl-accel", .data = LSM303DL_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm303dlh-accel", .data = LSM303DLH_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm303dlhc-accel", .data = LSM303DLHC_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm303dlm-accel", .data = LSM303DLM_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm330-accel", .data = LSM330_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm330d-accel", .data = LSM330D_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm330dl-accel", .data = LSM330DL_ACCEL_DEV_NAME },
+	{ .compatible = "st,lsm330dlc-accel", .data = LSM330DLC_ACCEL_DEV_NAME },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_accel_of_table);
+
 static int st_accel_i2c_probe(struct i2c_client *client,
 						const struct i2c_device_id *id)
 {
@@ -34,6 +50,22 @@ static int st_accel_i2c_probe(struct i2c_client *client,
 
 	st_sensors_i2c_configure(indio_dev, client, adata);
 
+	/*
+	 * If we are probed through DT, st_sensors_i2c_configure will
+	 * fill the indio_dev->name string with the client->name,
+	 * which is the compatible without the vendor prefix.  Since
+	 * compatibles separators are usually "-", and that the
+	 * convention in this driver is using "_", we obviously have a
+	 * problem when the st-sensors core checks that the two
+	 * strings matches. We need to set again the indio_dev->name
+	 * string to the real value used by the core later on.
+	 */
+	if (client->dev.of_node) {
+		const struct of_device_id *device;
+		device = of_match_device(st_accel_of_table, &client->dev);
+		indio_dev->name = device->data;
+	}
+
 	err = st_accel_common_probe(indio_dev, client->dev.platform_data);
 	if (err < 0)
 		return err;
@@ -67,6 +99,7 @@ static struct i2c_driver st_accel_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "st-accel-i2c",
+		.of_match_table = of_match_ptr(st_accel_of_table),
 	},
 	.probe = st_accel_i2c_probe,
 	.remove = st_accel_i2c_remove,
-- 
1.8.4.2

[RFC PATCH 1/3] iio: accel: Add device tree probing for STMicro accelerometers

[Jonathan Cameron]

On 11/19/13 15:50, Maxime Ripard wrote:
> Add the compatibles supported by the st_sensors library. This uses kind
> of a hack, since the st_sensors core will actively check at probe time
> that the device name matches the one reported when using old style i2c
> probing, and that this name will be different with device tree.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Note that this is unnecessary give core i2c matching, until we have some
more elements in the binding.  I'd certainly expect the interrupts
on the relevant devices to be in the binding...
> ---
>  .../devicetree/bindings/iio/accel/st_accel_i2c.txt | 22 +++++++++++++++
>  drivers/iio/accel/st_accel_i2c.c                   | 33 ++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt b/Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
> new file mode 100644
> index 000000000000..4a1efdf1775f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
> @@ -0,0 +1,22 @@
> +* ST Micro accelerometer sensors 
> +
> +Required properties:
> +  - compatible : should be either:
> +	* "st,lis3dh"
> +	* "st,lis331dlh"
> +	* "st,lsm303dl-accel"
> +	* "st,lsm303dlh-accel"
> +	* "st,lsm303dlhc-accel"
> +	* "st,lsm303dlm-accel"
> +	* "st,lsm330-accel"
> +	* "st,lsm330d-accel"
> +	* "st,lsm330dl-accel"
> +	* "st,lsm330dlc-accel"
> +  - reg : the I2C address of the sensor
> +
> +Example:
> +
> +accel: accel at 19 {
> +	compatible = "st,lsm330dlc-accel";
> +	reg = <0x19>;
> +};
> diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> index d7bedbdfc81d..de2bf76378d8 100644
> --- a/drivers/iio/accel/st_accel_i2c.c
> +++ b/drivers/iio/accel/st_accel_i2c.c
> @@ -13,11 +13,27 @@
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
>  #include <linux/iio/iio.h>
> +#include <linux/of_device.h>
>  
>  #include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_i2c.h>
>  #include "st_accel.h"
>  
> +static const struct of_device_id st_accel_of_table[] = {
> +	{ .compatible = "st,lis3dh", .data = LIS3DH_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lis331dlh", .data = LIS331DLH_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm303dl-accel", .data = LSM303DL_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm303dlh-accel", .data = LSM303DLH_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm303dlhc-accel", .data = LSM303DLHC_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm303dlm-accel", .data = LSM303DLM_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm330-accel", .data = LSM330_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm330d-accel", .data = LSM330D_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm330dl-accel", .data = LSM330DL_ACCEL_DEV_NAME },
> +	{ .compatible = "st,lsm330dlc-accel", .data = LSM330DLC_ACCEL_DEV_NAME },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_accel_of_table);
> +
>  static int st_accel_i2c_probe(struct i2c_client *client,
>  						const struct i2c_device_id *id)
>  {
> @@ -34,6 +50,22 @@ static int st_accel_i2c_probe(struct i2c_client *client,
>  
>  	st_sensors_i2c_configure(indio_dev, client, adata);
>  
> +	/*
> +	 * If we are probed through DT, st_sensors_i2c_configure will
> +	 * fill the indio_dev->name string with the client->name,
> +	 * which is the compatible without the vendor prefix.  Since
> +	 * compatibles separators are usually "-", and that the
> +	 * convention in this driver is using "_", we obviously have a
> +	 * problem when the st-sensors core checks that the two
> +	 * strings matches. We need to set again the indio_dev->name
> +	 * string to the real value used by the core later on.
> +	 */
> +	if (client->dev.of_node) {
> +		const struct of_device_id *device;
> +		device = of_match_device(st_accel_of_table, &client->dev);
> +		indio_dev->name = device->data;
> +	}
> +
>  	err = st_accel_common_probe(indio_dev, client->dev.platform_data);
>  	if (err < 0)
>  		return err;
> @@ -67,6 +99,7 @@ static struct i2c_driver st_accel_driver = {
>  	.driver = {
>  		.owner = THIS_MODULE,
>  		.name = "st-accel-i2c",
> +		.of_match_table = of_match_ptr(st_accel_of_table),
>  	},
>  	.probe = st_accel_i2c_probe,
>  	.remove = st_accel_i2c_remove,
> 

[RFC PATCH 1/3] iio: accel: Add device tree probing for STMicro accelerometers

[Maxime Ripard]

On Sat, Nov 30, 2013 at 12:04:20PM +0000, Jonathan Cameron wrote:
> On 11/19/13 15:50, Maxime Ripard wrote:
> > Add the compatibles supported by the st_sensors library. This uses kind
> > of a hack, since the st_sensors core will actively check at probe time
> > that the device name matches the one reported when using old style i2c
> > probing, and that this name will be different with device tree.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> Note that this is unnecessary give core i2c matching, until we have some
> more elements in the binding.  I'd certainly expect the interrupts
> on the relevant devices to be in the binding...

This interrupt DT thing has been stalled for monthes, I don't have any
hardware to test the interrupts, maybe we can just move forward and
wait for someone that have the need for the interrupts to do the work?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131130/6d5ab51c/attachment.sig>

[RFC PATCH 2/3] iio: accel: Add device tree probing for STMicro gyroscopes

[Maxime Ripard]

Add the compatibles supported by the st_sensors library. This uses kind
of a hack, since the st_sensors core will actively check at probe time
that the device name matches the one reported when using old style i2c
probing, and that this name will be different with device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/iio/gyro/st_gyro_i2c.txt   | 20 ++++++++++++++
 drivers/iio/gyro/st_gyro_i2c.c                     | 31 ++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/gyro/st_gyro_i2c.txt

diff --git a/Documentation/devicetree/bindings/iio/gyro/st_gyro_i2c.txt b/Documentation/devicetree/bindings/iio/gyro/st_gyro_i2c.txt
new file mode 100644
index 000000000000..39e434c80771
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/gyro/st_gyro_i2c.txt
@@ -0,0 +1,20 @@
+* ST Micro gyroscopes sensors 
+
+Required properties:
+  - compatible : should be either:
+	* "st,l3g4200d"
+	* "st,lsm330-gyro"
+	* "st,lsm330d-gyro"
+	* "st,lsm330dl-gyro"
+	* "st,lsm330dlc-gyro"
+	* "st,l3gd20"
+	* "st,l3gd20h"
+	* "st,l3g4is-ui"
+  - reg : the I2C address of the sensor
+
+Example:
+
+gyro: gyro at 6a {
+	compatible = "st,lsm330dlc-gyro";
+	reg = <0x6a>;
+};
diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
index 16b8b8d70bf1..f82c1d9c4be9 100644
--- a/drivers/iio/gyro/st_gyro_i2c.c
+++ b/drivers/iio/gyro/st_gyro_i2c.c
@@ -13,11 +13,25 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
+#include <linux/of_device.h>
 
 #include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_i2c.h>
 #include "st_gyro.h"
 
+static const struct of_device_id st_gyro_of_table[] = {
+	{ .compatible = "st,l3g4200d", .data = L3G4200D_GYRO_DEV_NAME },
+	{ .compatible = "st,lsm330-gyro", .data = LSM330_GYRO_DEV_NAME },
+	{ .compatible = "st,lsm330d-gyro", .data = LSM330D_GYRO_DEV_NAME },
+	{ .compatible = "st,lsm330dl-gyro", .data = LSM330DL_GYRO_DEV_NAME },
+	{ .compatible = "st,lsm330dlc-gyro", .data = LSM330DLC_GYRO_DEV_NAME },
+	{ .compatible = "st,l3gd20", .data = L3GD20_GYRO_DEV_NAME },
+	{ .compatible = "st,l3gd20h", .data = L3GD20H_GYRO_DEV_NAME },
+	{ .compatible = "st,l3g4is-ui", .data = L3G4IS_GYRO_DEV_NAME },
+	{},
+};
+MODULE_DEVICE_TABLE(of, st_gyro_of_table);
+
 static int st_gyro_i2c_probe(struct i2c_client *client,
 						const struct i2c_device_id *id)
 {
@@ -34,6 +48,22 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
 
 	st_sensors_i2c_configure(indio_dev, client, gdata);
 
+	/*
+	 * If we are probed through DT, st_sensors_i2c_configure will
+	 * fill the indio_dev->name string with the client->name,
+	 * which is the compatible without the vendor prefix.  Since
+	 * compatibles separators are usually "-", and that the
+	 * convention in this driver is using "_", we obviously have a
+	 * problem when the st-sensors core checks that the two
+	 * strings matches. We need to set again the indio_dev->name
+	 * string to the real value used by the core later on.
+	 */
+	if (client->dev.of_node) {
+		const struct of_device_id *device;
+		device = of_match_device(st_gyro_of_table, &client->dev);
+		indio_dev->name = device->data;
+	}
+
 	err = st_gyro_common_probe(indio_dev,
 				(struct st_sensors_platform_data *)&gyro_pdata);
 	if (err < 0)
@@ -66,6 +96,7 @@ static struct i2c_driver st_gyro_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "st-gyro-i2c",
+		.of_match_table = of_match_ptr(st_gyro_of_table),
 	},
 	.probe = st_gyro_i2c_probe,
 	.remove = st_gyro_i2c_remove,
-- 
1.8.4.2

[RFC PATCH 3/3] ARM: cfa10057: Add the accelerometer and gyroscope to the device tree

[Maxime Ripard]

The CFA-10057 has an embedded ST Micro LSM330 DLC accelerometer and
gyroscope wired on the the i2c0 bus. Add those to the DT.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10057.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/imx28-cfa10057.dts b/arch/arm/boot/dts/imx28-cfa10057.dts
index 0333c0532f28..fc289465c30f 100644
--- a/arch/arm/boot/dts/imx28-cfa10057.dts
+++ b/arch/arm/boot/dts/imx28-cfa10057.dts
@@ -119,6 +119,19 @@
 				status = "okay";
 			};
 
+			i2c0: i2c at 80058000 {
+				accel: accel at 19 {
+					compatible = "st,lsm330dlc-accel";
+					reg = <0x19>;
+				};
+
+				gyro: gyro at 6a {
+					compatible = "st,lsm330dlc-gyro";
+					reg = <0x6a>;
+				};
+			};
+
+
 			i2c1: i2c at 8005a000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&i2c1_pins_a>;
-- 
1.8.4.2

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Denis CIOCCA]

Hi Maxime, everyone,

only one point: it's possible to use the same names with DT? (using _ 
instead of -)
Why only for i2c?

Thanks,
Denis

> Hi everyone,
>
> This is an attempt at adding the DT probing to the st_sensors
> accelerometers and gyroscopes.
>
> It's somewhat hackish, because the core actively checks for the value
> of the device at probe time, and DT probing generates a quite
> different name.
>
> I first tried to turn the function st_sensors_check_device_support the
> other way around, with the "leaf" providing the sensor structure, and
> the function checking that it's consistent with the WAI stored in the
> sensor. However, it turned to be a quite intrusive change, so I'm
> sending the easiest one, to try to get some feedback :)
>
> Thanks!
> Maxime
>
> Maxime Ripard (3):
>    iio: accel: Add device tree probing for STMicro accelerometers
>    iio: accel: Add device tree probing for STMicro gyroscopes
>    ARM: cfa10057: Add the accelerometer and gyroscope to the device tree
>
>   .../devicetree/bindings/iio/accel/st_accel_i2c.txt | 22 +++++++++++++++
>   .../devicetree/bindings/iio/gyro/st_gyro_i2c.txt   | 20 +++++++++++++
>   arch/arm/boot/dts/imx28-cfa10057.dts               | 13 +++++++++
>   drivers/iio/accel/st_accel_i2c.c                   | 33 ++++++++++++++++++++++
>   drivers/iio/gyro/st_gyro_i2c.c                     | 31 ++++++++++++++++++++
>   5 files changed, 119 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iio/accel/st_accel_i2c.txt
>   create mode 100644 Documentation/devicetree/bindings/iio/gyro/st_gyro_i2c.txt
>

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Maxime Ripard]

Hi Denis,

On Thu, Nov 21, 2013 at 12:41:24PM +0100, Denis CIOCCA wrote:
> only one point: it's possible to use the same names with DT? (using _ 
> instead of -)

Yes, it is, but only for i2c as far as I'm aware, and usually the DT
compatibles are with - as a separator (I looked into the ePAPR, but
couldn't get any explanations or requirements on this, even though
it's used in all of their examples)

> Why only for i2c?

Because I was only interested in i2c :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131121/88174551/attachment.sig>

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Jonathan Cameron]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/21/13 13:14, Maxime Ripard wrote:
> Hi Denis,
> 
> On Thu, Nov 21, 2013 at 12:41:24PM +0100, Denis CIOCCA wrote:
>> only one point: it's possible to use the same names with DT? (using _ instead of -)
> 
> Yes, it is, but only for i2c as far as I'm aware, and usually the DT compatibles are with - as a separator (I
> looked into the ePAPR, but couldn't get any explanations or requirements on this, even though it's used in all of
> their examples)
In other discussions, where the defacto i2c device tree bindings have
been followed, the conclusion has been that to change to a - from _
would result in userspace ABI changes, so whilst no one wants _
the discussion has concluded we can't really avoid it.


> 
>> Why only for i2c?
> 
> Because I was only interested in i2c :)
> 
> Maxime
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSkmkBAAoJEFSFNJnE9BaI9mgP/A0TrsoGuLmu1Zc0R2Y2QkY/
wE4hrQrB0uGwD6TE+dXCuJVmHLqA7KviGN3l4sJPgo8kGAEsAItow8XoOPZFE8sU
w9hXVU+0UXOiMmQIf/5bTvVpBV7oL6agYpQQkb8ZIHKZCj5gd588sc+Ag90Uc+lK
MiI1QsvLbS8O1tZui+4NlkaioUQs5rs0OvD9N1n+aCfSdgsZk1NMR97Xf5Hwpmbj
ypTokNVAClZd8dyrq9KHXfR6+qPwI9Pr/yA5nqZvpV++SiqlFw5nWAR/URPp/rEE
czT8ZiC2SR5qRTVb+jRCj4hY8QmLIfeZEqrsZab9CkRkEmeMgQ8FhpBe+a1Zk1YS
ZojKQS4rJzjuJukuq4vs/R8jPhwAIxguzzC7rA/UNtyYUvqTrIcXBRzFxRp6BHtv
PO2viO773gvni3C6qzFtLBcYfJUBD6VNi440yHDxMSZY5gJl3auHdIzpTxN30aQG
P4l347YqfsvrEpiG7gN2B5ScQ3WwGXBpfomke1dkuDElZZ7vgV/MAE7Hjh/uKCfg
0lLzoBWdO72faznHCm8tOOPAQcSqXAm0F4BHQENeVirieZN2mS4EhS0xmN1fCdOy
5PeN424FSGw6LUgAZd8R45/usPzzlDKTQ2rf0nBZIRdxYmyocg2ZUCINI6OxJThn
OJkKZAsuvBdhsqSSAx+C
=xwmo
-----END PGP SIGNATURE-----

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Denis CIOCCA]

Hi Jonathan,
>> Yes, it is, but only for i2c as far as I'm aware, and usually the DT compatibles are with - as a separator (I
>> looked into the ePAPR, but couldn't get any explanations or requirements on this, even though it's used in all of
>> their examples)
> In other discussions, where the defacto i2c device tree bindings have
> been followed, the conclusion has been that to change to a - from _
> would result in userspace ABI changes, so whilst no one wants _
> the discussion has concluded we can't really avoid it.
Ok, I understood the point. Thanks,

Denis

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Maxime Ripard]

Hi Jonathan,

On Sun, Nov 24, 2013 at 09:00:49PM +0000, Jonathan Cameron wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 11/21/13 13:14, Maxime Ripard wrote:
> > Hi Denis,
> > 
> > On Thu, Nov 21, 2013 at 12:41:24PM +0100, Denis CIOCCA wrote:
> > > only one point: it's possible to use the same names with DT?
> > > (using _ instead of -)
> > 
> > Yes, it is, but only for i2c as far as I'm aware, and usually the
> > DT compatibles are with - as a separator (I looked into the ePAPR,
> > but couldn't get any explanations or requirements on this, even
> > though it's used in all of their examples)
> 
> In other discussions, where the defacto i2c device tree bindings have
> been followed, the conclusion has been that to change to a - from _
> would result in userspace ABI changes, so whilst no one wants _
> the discussion has concluded we can't really avoid it.

What kind of userspace ABI changes are we talking about?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131125/175727d9/attachment.sig>

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Jonathan Cameron]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/25/13 09:40, Maxime Ripard wrote:
> Hi Jonathan,
> 
> On Sun, Nov 24, 2013 at 09:00:49PM +0000, Jonathan Cameron wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 11/21/13 13:14, Maxime Ripard wrote:
>>> Hi Denis,
>>> 
>>> On Thu, Nov 21, 2013 at 12:41:24PM +0100, Denis CIOCCA wrote:
>>>> only one point: it's possible to use the same names with DT? (using _ instead of -)
>>> 
>>> Yes, it is, but only for i2c as far as I'm aware, and usually the DT compatibles are with - as a separator (I
>>> looked into the ePAPR, but couldn't get any explanations or requirements on this, even though it's used in all
>>> of their examples)
>> 
>> In other discussions, where the defacto i2c device tree bindings have been followed, the conclusion has been that
>> to change to a - from _ would result in userspace ABI changes, so whilst no one wants _ the discussion has
>> concluded we can't really avoid it.
> 
> What kind of userspace ABI changes are we talking about?
IIRC:

i2c has a generic binding that matches to the name bit of the i2c_device_id
array.  That is then exported in sysfs.  There are quite a lot of instances
of underscores out there in these names.  Thus unforutnately they can't
be changed without possibly breaking userspace.  Typically those same names
are also output by IIO though obviously we could keep that the same whilst
changing the dt binding.

Also the i2c binding allows binding after dropping the vendor prefix which
is even more 'interesting'. See of_modialias_node in drivers/of/base.c

I'd therefore argue in favour of just leaving the underscores in existing
drivers as a nasty bit of legacy and doing our best to not introduce any
new ones!

Jonathan

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSmdOzAAoJEFSFNJnE9BaIilUP/RPwSswdYi9wWtN0tBEFfZvo
JO0bybjw342LE2xt3DXd7fo04v1+LspWxatw6kht1mJ/aU6O4HwNHQpPO7Md5pDm
4HPoPSREaDbYEOqhVSjXaBUOXIOYSe392KtqKJMRSxRs3XiIeqYPfojqtIE2KF7a
W3EEjud5wrWuOLKtdQWjRt+UpDnewImR+qORtRUoA7NY2YIHvdarOQFlfDqLXSRK
GvE49O8jCtQvcjI1rsybMRXQkUF3GE8hLyATaf/SbvgVeILbHSl6OvaN7O7nNJ+b
DGGrzAzmKwj56G47imQ7z2v/D9ExWNwr1DZ2CMPt+yXQ15OBXDnAmWGXTr45/dUl
G8PjaZA9GKsp0dE713b0RN/eJnsoFsJd8bDCeXh9IpVblmkQzW3HsvI8p2Uj8xga
5d/txeXytLNvHj+3OiiVTaKA3Y9/dFNq9dwI+QAMbLY87T4ft70z533liTZRnMWG
2O8x2oHYnoFRH3qmpg6jy3/qCFe6wTTXy1yMFQ0Ds/bJ93vhrv7SiXXsxX3CNl9X
KI8t74zx51GMxBV5pMixgX9aBH7Wrn+WSXmUKkMkm7JGbeicZEjk259EjAluuohD
HHh0cehjCuVPY6PTUhkXTfCFDfpFtwOkXsZb6fOMS2wDKK4BLwb3SPwyn+ID7XJs
u/RQzwNNjvdsR2hmBQOw
=lpwt
-----END PGP SIGNATURE-----

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Jean Delvare]

On Sat, 30 Nov 2013 12:02:02 +0000, Jonathan Cameron wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 11/25/13 09:40, Maxime Ripard wrote:
> > Hi Jonathan,
> > 
> > On Sun, Nov 24, 2013 at 09:00:49PM +0000, Jonathan Cameron wrote:
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >> 
> >> On 11/21/13 13:14, Maxime Ripard wrote:
> >>> Hi Denis,
> >>> 
> >>> On Thu, Nov 21, 2013 at 12:41:24PM +0100, Denis CIOCCA wrote:
> >>>> only one point: it's possible to use the same names with DT? (using _ instead of -)
> >>> 
> >>> Yes, it is, but only for i2c as far as I'm aware, and usually the DT compatibles are with - as a separator (I
> >>> looked into the ePAPR, but couldn't get any explanations or requirements on this, even though it's used in all
> >>> of their examples)
> >> 
> >> In other discussions, where the defacto i2c device tree bindings have been followed, the conclusion has been that
> >> to change to a - from _ would result in userspace ABI changes, so whilst no one wants _ the discussion has
> >> concluded we can't really avoid it.
> > 
> > What kind of userspace ABI changes are we talking about?
> IIRC:
> 
> i2c has a generic binding that matches to the name bit of the i2c_device_id
> array.  That is then exported in sysfs.  There are quite a lot of instances
> of underscores out there in these names.  Thus unforutnately they can't
> be changed without possibly breaking userspace.  Typically those same names
> are also output by IIO though obviously we could keep that the same whilst
> changing the dt binding.
> 
> Also the i2c binding allows binding after dropping the vendor prefix which
> is even more 'interesting'. See of_modialias_node in drivers/of/base.c
> 
> I'd therefore argue in favour of just leaving the underscores in existing
> drivers as a nasty bit of legacy and doing our best to not introduce any
> new ones!

I don't know what is the problem with underscores, but please note that
hwmon i2c devices are NOT allowed to have dashes in their name because
that would break libsensors.

-- 
Jean Delvare

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Maxime Ripard]

Hi Jean,

On Sat, Nov 30, 2013 at 02:04:02PM +0100, Jean Delvare wrote:
> > i2c has a generic binding that matches to the name bit of the
> > i2c_device_id array.  That is then exported in sysfs.  There are
> > quite a lot of instances of underscores out there in these names.
> > Thus unforutnately they can't be changed without possibly breaking
> > userspace.  Typically those same names are also output by IIO
> > though obviously we could keep that the same whilst changing the
> > dt binding.
> > 
> > Also the i2c binding allows binding after dropping the vendor
> > prefix which is even more 'interesting'. See of_modialias_node in
> > drivers/of/base.c
> > 
> > I'd therefore argue in favour of just leaving the underscores in
> > existing drivers as a nasty bit of legacy and doing our best to
> > not introduce any new ones!
> 
> I don't know what is the problem with underscores, but please note
> that hwmon i2c devices are NOT allowed to have dashes in their name
> because that would break libsensors.

As far as I remember, hwmon doesn't handle the accelerometers or
gyroscopes, does it?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131203/f2dacea7/attachment.sig>

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Jean Delvare]

On Tue, 3 Dec 2013 22:29:14 +0100, Maxime Ripard wrote:
> Hi Jean,
> 
> On Sat, Nov 30, 2013 at 02:04:02PM +0100, Jean Delvare wrote:
> > > i2c has a generic binding that matches to the name bit of the
> > > i2c_device_id array.  That is then exported in sysfs.  There are
> > > quite a lot of instances of underscores out there in these names.
> > > Thus unforutnately they can't be changed without possibly breaking
> > > userspace.  Typically those same names are also output by IIO
> > > though obviously we could keep that the same whilst changing the
> > > dt binding.
> > > 
> > > Also the i2c binding allows binding after dropping the vendor
> > > prefix which is even more 'interesting'. See of_modialias_node in
> > > drivers/of/base.c
> > > 
> > > I'd therefore argue in favour of just leaving the underscores in
> > > existing drivers as a nasty bit of legacy and doing our best to
> > > not introduce any new ones!
> > 
> > I don't know what is the problem with underscores, but please note
> > that hwmon i2c devices are NOT allowed to have dashes in their name
> > because that would break libsensors.
> 
> As far as I remember, hwmon doesn't handle the accelerometers or
> gyroscopes, does it?

You remember correctly, it does not and never will.

-- 
Jean Delvare

[RFC PATCH 0/3] DT support for ST micro accelerometers and gyroscopes

[Maxime Ripard]

On Sat, Nov 30, 2013 at 12:02:02PM +0000, Jonathan Cameron wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 11/25/13 09:40, Maxime Ripard wrote:
> > Hi Jonathan,
> > 
> > On Sun, Nov 24, 2013 at 09:00:49PM +0000, Jonathan Cameron wrote:
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >> 
> >> On 11/21/13 13:14, Maxime Ripard wrote:
> >>> Hi Denis,
> >>> 
> >>> On Thu, Nov 21, 2013 at 12:41:24PM +0100, Denis CIOCCA wrote:
> >>>> only one point: it's possible to use the same names with DT? (using _ instead of -)
> >>> 
> >>> Yes, it is, but only for i2c as far as I'm aware, and usually the DT compatibles are with - as a separator (I
> >>> looked into the ePAPR, but couldn't get any explanations or requirements on this, even though it's used in all
> >>> of their examples)
> >> 
> >> In other discussions, where the defacto i2c device tree bindings have been followed, the conclusion has been that
> >> to change to a - from _ would result in userspace ABI changes, so whilst no one wants _ the discussion has
> >> concluded we can't really avoid it.
> > 
> > What kind of userspace ABI changes are we talking about?
> IIRC:
> 
> i2c has a generic binding that matches to the name bit of the i2c_device_id
> array.  That is then exported in sysfs.  There are quite a lot of instances
> of underscores out there in these names.  Thus unforutnately they can't
> be changed without possibly breaking userspace.  Typically those same names
> are also output by IIO though obviously we could keep that the same whilst
> changing the dt binding.
> 
> Also the i2c binding allows binding after dropping the vendor prefix which
> is even more 'interesting'. See of_modialias_node in drivers/of/base.c
> 
> I'd therefore argue in favour of just leaving the underscores in existing
> drivers as a nasty bit of legacy and doing our best to not introduce any
> new ones!

Except I'm not removing anything, just adding new stuff. Therefore,
I'm not breaking anything. The existing users of the underscore stuff
in DT will still work, (if some ever exist, last time I checked, it
wasn't the case) just like they used to.

If we have the occasion of having the thing done like it's done
anywhere else, at no cost, I don't get what the issue is.

Especially since these devices are also usable on other buses than
i2c, which make it quite inconsistent from one bus to another.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131130/6c6f0e85/attachment.sig>