[PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates

[PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates

[Marek Belisko]

This series add support for using EOC gpio line of bmp085 to detect when conversion
is finished. First patch add that functionality. Second patch add DT bindings for gpio
and irq parameters. Third patch add missing platform data which are already in DT bindings
to have possibility to used them on non-DT archs.

Marek Belisko (3):
  misc: bmp085: Clean up and enable use of interrupt for completion.
  misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  misc: bmp085: Add missing platform data.

 Documentation/devicetree/bindings/misc/bmp085.txt |   8 ++
 drivers/misc/bmp085.c                             | 138 +++++++++++++++++++---
 include/linux/i2c/bmp085.h                        |  24 ++++
 3 files changed, 155 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/i2c/bmp085.h

-- 
1.8.1.2

[PATCH 1/3] misc: bmp085: Clean up and enable use of interrupt for completion.

[Marek Belisko]

- pass GPIO or IRQ to driver and have it initialize the GPIO and
  the IRQ
- finish waiting early if interrupt fires
- clean up GPIO and IRQ on exit.

Signed-off-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
Signed-off-by: Marek Belisko <marek-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
---
 drivers/misc/bmp085.c      | 68 +++++++++++++++++++++++++++++++++++++++++-----
 include/linux/i2c/bmp085.h | 17 ++++++++++++
 2 files changed, 78 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/i2c/bmp085.h

diff --git a/drivers/misc/bmp085.c b/drivers/misc/bmp085.c
index 2704d88..1510a7b 100644
--- a/drivers/misc/bmp085.c
+++ b/drivers/misc/bmp085.c
@@ -49,9 +49,12 @@
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/slab.h>
-#include <linux/delay.h>
 #include <linux/of.h>
 #include "bmp085.h"
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/gpio.h>
+#include <linux/i2c/bmp085.h>
 
 #define BMP085_CHIP_ID			0x55
 #define BMP085_CALIBRATION_DATA_START	0xAA
@@ -84,8 +87,20 @@ struct bmp085_data {
 	unsigned long last_temp_measurement;
 	u8	chip_id;
 	s32	b6; /* calculated temperature correction coefficient */
+	int	irq;
+	int	gpio;
+	struct	completion done;
 };
 
+static irqreturn_t bmp085_eoc_isr(int irq, void *devid)
+{
+	struct bmp085_data *data = devid;
+
+	complete(&data->done);
+
+	return IRQ_HANDLED;
+}
+
 static s32 bmp085_read_calibration_data(struct bmp085_data *data)
 {
 	u16 tmp[BMP085_CALIBRATION_DATA_LENGTH];
@@ -116,6 +131,9 @@ static s32 bmp085_update_raw_temperature(struct bmp085_data *data)
 	s32 status;
 
 	mutex_lock(&data->lock);
+
+	init_completion(&data->done);
+
 	status = regmap_write(data->regmap, BMP085_CTRL_REG,
 			      BMP085_TEMP_MEASUREMENT);
 	if (status < 0) {
@@ -123,7 +141,8 @@ static s32 bmp085_update_raw_temperature(struct bmp085_data *data)
 			"Error while requesting temperature measurement.\n");
 		goto exit;
 	}
-	msleep(BMP085_TEMP_CONVERSION_TIME);
+	wait_for_completion_timeout(&data->done, 1 + msecs_to_jiffies(
+					    BMP085_TEMP_CONVERSION_TIME));
 
 	status = regmap_bulk_read(data->regmap, BMP085_CONVERSION_REGISTER_MSB,
 				 &tmp, sizeof(tmp));
@@ -147,6 +166,9 @@ static s32 bmp085_update_raw_pressure(struct bmp085_data *data)
 	s32 status;
 
 	mutex_lock(&data->lock);
+
+	init_completion(&data->done);
+
 	status = regmap_write(data->regmap, BMP085_CTRL_REG,
 			BMP085_PRESSURE_MEASUREMENT +
 			(data->oversampling_setting << 6));
@@ -157,8 +179,8 @@ static s32 bmp085_update_raw_pressure(struct bmp085_data *data)
 	}
 
 	/* wait for the end of conversion */
-	msleep(2+(3 << data->oversampling_setting));
-
+	wait_for_completion_timeout(&data->done, 1 + msecs_to_jiffies(
+					2+(3 << data->oversampling_setting)));
 	/* copy data into a u32 (4 bytes), but skip the first byte. */
 	status = regmap_bulk_read(data->regmap, BMP085_CONVERSION_REGISTER_MSB,
 				 ((u8 *)&tmp)+1, 3);
@@ -423,6 +445,7 @@ EXPORT_SYMBOL_GPL(bmp085_regmap_config);
 int bmp085_probe(struct device *dev, struct regmap *regmap)
 {
 	struct bmp085_data *data;
+	struct bmp085_platform_data *pdata = dev->platform_data;
 	int err = 0;
 
 	data = kzalloc(sizeof(struct bmp085_data), GFP_KERNEL);
@@ -435,26 +458,54 @@ int bmp085_probe(struct device *dev, struct regmap *regmap)
 	data->dev = dev;
 	data->regmap = regmap;
 
+	if (pdata && gpio_is_valid(pdata->gpio)) {
+		err = devm_gpio_request(dev, pdata->gpio, "bmp085_eoc_irq");
+		if (err)
+			goto exit_free;
+		err = gpio_direction_input(pdata->gpio);
+		if (err)
+			goto exit_free;
+		data->irq = gpio_to_irq(pdata->gpio);
+		data->gpio = pdata->gpio;
+	} else {
+		if (pdata)
+			data->irq = pdata->irq;
+		else
+			data->irq = 0;
+		data->gpio = -EINVAL;
+	}
+	if (data->irq > 0) {
+		err = request_any_context_irq(data->irq, bmp085_eoc_isr,
+					      IRQF_TRIGGER_RISING, "bmp085",
+					      data);
+		if (err < 0)
+			goto exit_free;
+	} else
+		data->irq = 0;
+
 	/* Initialize the BMP085 chip */
 	err = bmp085_init_client(data);
 	if (err < 0)
-		goto exit_free;
+		goto exit_free_irq;
 
 	err = bmp085_detect(dev);
 	if (err < 0) {
 		dev_err(dev, "%s: chip_id failed!\n", BMP085_NAME);
-		goto exit_free;
+		goto exit_free_irq;
 	}
 
 	/* Register sysfs hooks */
 	err = sysfs_create_group(&dev->kobj, &bmp085_attr_group);
 	if (err)
-		goto exit_free;
+		goto exit_free_irq;
 
 	dev_info(dev, "Successfully initialized %s!\n", BMP085_NAME);
 
 	return 0;
 
+exit_free_irq:
+	if (data->irq)
+		free_irq(data->irq, data);
 exit_free:
 	kfree(data);
 exit:
@@ -466,6 +517,9 @@ int bmp085_remove(struct device *dev)
 {
 	struct bmp085_data *data = dev_get_drvdata(dev);
 
+	if (data->irq)
+		free_irq(data->irq, data);
+
 	sysfs_remove_group(&data->dev->kobj, &bmp085_attr_group);
 	kfree(data);
 
diff --git a/include/linux/i2c/bmp085.h b/include/linux/i2c/bmp085.h
new file mode 100644
index 0000000..b66cb98
--- /dev/null
+++ b/include/linux/i2c/bmp085.h
@@ -0,0 +1,17 @@
+#ifndef __LINUX_I2C_BMP085_H
+#define __LINUX_I2C_BMP085_H
+
+/* linux/i2c/bmp085.h */
+
+/*
+ * bmp085 platform data
+ * @gpio: if is set it is the End Of Conversion line which is high when
+ * conversion is finished
+ * @irq: if gpio < 0 and irq > 0, then it is an interrupt with no gpio
+ */
+struct bmp085_platform_data {
+	int	gpio;
+	int	irq;
+};
+
+#endif
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Marek Belisko]

Signed-off-by: Marek Belisko <marek-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
---
 Documentation/devicetree/bindings/misc/bmp085.txt |  8 ++++
 drivers/misc/bmp085.c                             | 53 ++++++++++++++++++++---
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/misc/bmp085.txt b/Documentation/devicetree/bindings/misc/bmp085.txt
index 91dfda2..c6033d5 100644
--- a/Documentation/devicetree/bindings/misc/bmp085.txt
+++ b/Documentation/devicetree/bindings/misc/bmp085.txt
@@ -8,6 +8,9 @@ Optional properties:
 - temp-measurement-period: temperature measurement period (milliseconds)
 - default-oversampling: default oversampling value to be used at startup,
   value range is 0-3 with rising sensitivity.
+- gpio: if device is using EOC irq line gpio can be specified to setup interrupt
+  handling
+- irq: interrupt with no gpio
 
 Example:
 
@@ -17,4 +20,9 @@ pressure@77 {
 	chip-id = <10>;
 	temp-measurement-period = <100>;
 	default-oversampling = <2>;
+	gpio = <&gpio0 17 0>;
+
+	OR
+
+	irq = <75>;
 };
diff --git a/drivers/misc/bmp085.c b/drivers/misc/bmp085.c
index 1510a7b..9792ce2 100644
--- a/drivers/misc/bmp085.c
+++ b/drivers/misc/bmp085.c
@@ -50,6 +50,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include "bmp085.h"
 #include <linux/interrupt.h>
 #include <linux/completion.h>
@@ -396,7 +397,8 @@ int bmp085_detect(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(bmp085_detect);
 
-static void bmp085_get_of_properties(struct bmp085_data *data)
+static void bmp085_get_of_properties(struct bmp085_data *data,
+			struct bmp085_platform_data *pdata)
 {
 #ifdef CONFIG_OF
 	struct device_node *np = data->dev->of_node;
@@ -413,12 +415,18 @@ static void bmp085_get_of_properties(struct bmp085_data *data)
 
 	if (!of_property_read_u32(np, "default-oversampling", &prop))
 		data->oversampling_setting = prop & 0xff;
+
+	pdata->gpio = of_get_named_gpio(np, "gpio", 0);
+	of_property_read_u32(np, "irq", &pdata->irq);
 #endif
 }
 
-static int bmp085_init_client(struct bmp085_data *data)
+static int bmp085_init_client(struct device *dev, struct bmp085_data *data)
 {
 	int status = bmp085_read_calibration_data(data);
+	struct bmp085_platform_data *pdata = dev->platform_data;
+	struct device_node *node = dev->of_node;
+	int err;
 
 	if (status < 0)
 		return status;
@@ -429,11 +437,46 @@ static int bmp085_init_client(struct bmp085_data *data)
 	data->temp_measurement_period = 1*HZ;
 	data->oversampling_setting = 3;
 
-	bmp085_get_of_properties(data);
+	/* parse DT to get platform data */
+	if (node && !pdata) {
+		pdata = devm_kzalloc(dev, sizeof(struct bmp085_platform_data),
+					GFP_KERNEL);
+		if (!pdata)
+			return -ENOMEM;
+	}
+
+	bmp085_get_of_properties(data, pdata);
+
+	if (gpio_is_valid(pdata->gpio)) {
+		err = devm_gpio_request(dev, pdata->gpio, "bmp085_eoc_irq");
+		if (err)
+			goto exit_free;
+		err = gpio_direction_input(pdata->gpio);
+		if (err)
+			goto exit_free;
+		data->irq = gpio_to_irq(pdata->gpio);
+		data->gpio = pdata->gpio;
+	} else {
+		if (pdata->irq > 0)
+			data->irq = pdata->irq;
+		else
+			data->irq = 0;
+		data->gpio = -EINVAL;
+	}
+	if (data->irq > 0) {
+		err = request_any_context_irq(data->irq, bmp085_eoc_isr,
+					      IRQF_TRIGGER_RISING, "bmp085",
+					      data);
+		if (err < 0)
+			goto exit_free;
+	}
 
 	mutex_init(&data->lock);
 
 	return 0;
+
+exit_free:
+	return err;
 }
 
 struct regmap_config bmp085_regmap_config = {
@@ -445,7 +488,6 @@ EXPORT_SYMBOL_GPL(bmp085_regmap_config);
 int bmp085_probe(struct device *dev, struct regmap *regmap)
 {
 	struct bmp085_data *data;
-	struct bmp085_platform_data *pdata = dev->platform_data;
 	int err = 0;
 
 	data = kzalloc(sizeof(struct bmp085_data), GFP_KERNEL);
@@ -484,7 +526,7 @@ int bmp085_probe(struct device *dev, struct regmap *regmap)
 		data->irq = 0;
 
 	/* Initialize the BMP085 chip */
-	err = bmp085_init_client(data);
+	err = bmp085_init_client(dev, data);
 	if (err < 0)
 		goto exit_free_irq;
 
@@ -506,7 +548,6 @@ int bmp085_probe(struct device *dev, struct regmap *regmap)
 exit_free_irq:
 	if (data->irq)
 		free_irq(data->irq, data);
-exit_free:
 	kfree(data);
 exit:
 	return err;
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] misc: bmp085: Add missing platform data.

[Marek Belisko]

DT bindings contains more parameters to set so add them to platform data also
to have possibility to use on arch where DT isn't available yet.

Signed-off-by: Marek Belisko <marek@goldelico.com>
---
 drivers/misc/bmp085.c      | 21 +++++++++++++++++----
 include/linux/i2c/bmp085.h |  7 +++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/bmp085.c b/drivers/misc/bmp085.c
index 9792ce2..530b4a1 100644
--- a/drivers/misc/bmp085.c
+++ b/drivers/misc/bmp085.c
@@ -408,13 +408,15 @@ static void bmp085_get_of_properties(struct bmp085_data *data,
 		return;
 
 	if (!of_property_read_u32(np, "chip-id", &prop))
-		data->chip_id = prop & 0xff;
+		pdata->chip_id = prop & 0xff;
 
 	if (!of_property_read_u32(np, "temp-measurement-period", &prop))
-		data->temp_measurement_period = (prop/100)*HZ;
+		pdata->temp_measurement_period = (prop/100)*HZ;
 
 	if (!of_property_read_u32(np, "default-oversampling", &prop))
-		data->oversampling_setting = prop & 0xff;
+		pdata->default_oversampling = prop & 0xff;
+	else
+		pdata->default_oversampling = -1;
 
 	pdata->gpio = of_get_named_gpio(np, "gpio", 0);
 	of_property_read_u32(np, "irq", &pdata->irq);
@@ -443,9 +445,20 @@ static int bmp085_init_client(struct device *dev, struct bmp085_data *data)
 					GFP_KERNEL);
 		if (!pdata)
 			return -ENOMEM;
+
+		bmp085_get_of_properties(data, pdata);
 	}
 
-	bmp085_get_of_properties(data, pdata);
+	if (pdata->chip_id)
+		data->chip_id = pdata->chip_id;
+
+	if (pdata->temp_measurement_period > 0)
+		data->temp_measurement_period =
+			(pdata->temp_measurement_period/100)*HZ;
+
+	if (pdata->default_oversampling >= 0 &&
+		pdata->default_oversampling <= 3)
+		data->oversampling_setting = pdata->default_oversampling;
 
 	if (gpio_is_valid(pdata->gpio)) {
 		err = devm_gpio_request(dev, pdata->gpio, "bmp085_eoc_irq");
diff --git a/include/linux/i2c/bmp085.h b/include/linux/i2c/bmp085.h
index b66cb98..addb972 100644
--- a/include/linux/i2c/bmp085.h
+++ b/include/linux/i2c/bmp085.h
@@ -5,11 +5,18 @@
 
 /*
  * bmp085 platform data
+ * @chip_id: configurable chip id for non-default chip revisions
+ * @temp_measurement_period: in milliseconds
+ * @default_oversampling: used at startup, range is 0-3 with rising sensitivity
+ * set it to -1 when don't want to change default value (3)
  * @gpio: if is set it is the End Of Conversion line which is high when
  * conversion is finished
  * @irq: if gpio < 0 and irq > 0, then it is an interrupt with no gpio
  */
 struct bmp085_platform_data {
+	int	chip_id;
+	int	temp_measurement_period;
+	int	default_oversampling;
 	int	gpio;
 	int	irq;
 };
-- 
1.8.1.2

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Arnd Bergmann]

On Thursday 14 November 2013, Marek Belisko wrote:
> +       if (node && !pdata) {
> +               pdata = devm_kzalloc(dev, sizeof(struct bmp085_platform_data),
> +                                       GFP_KERNEL);
> +               if (!pdata)
> +                       return -ENOMEM;
> +       }
> +

I generally recommend against allocating platform data from inside the driver,
as this requires more code and more memory than just adding fields to the
driver-specific data structure and copying over the fields from either
DT or the supplied platform data, depending on which one is used.

	Arnd

Re: [PATCH 3/3] misc: bmp085: Add missing platform data.

[Arnd Bergmann]

On Thursday 14 November 2013, Marek Belisko wrote:
> DT bindings contains more parameters to set so add them to platform data also
> to have possibility to use on arch where DT isn't available yet.
> 
> Signed-off-by: Marek Belisko <marek@goldelico.com>

Can you give an example of a platform that uses this chip and cannot
yet use DT in the mainline kernel? 
If it's only for out-of-tree platforms, I'd prefer to leave this
patch out of tree as well and put the burden on whoever maintains
a non-DT platform in a private kernel.


> diff --git a/include/linux/i2c/bmp085.h b/include/linux/i2c/bmp085.h
> index b66cb98..addb972 100644
> --- a/include/linux/i2c/bmp085.h
> +++ b/include/linux/i2c/bmp085.h

Shouldn't this be in include/linux/platform_data? 

	Arnd

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Mark Rutland]

On Thu, Nov 14, 2013 at 09:46:48PM +0000, Marek Belisko wrote:
> Signed-off-by: Marek Belisko <marek@goldelico.com>
> ---
>  Documentation/devicetree/bindings/misc/bmp085.txt |  8 ++++
>  drivers/misc/bmp085.c                             | 53 ++++++++++++++++++++---
>  2 files changed, 55 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/misc/bmp085.txt b/Documentation/devicetree/bindings/misc/bmp085.txt
> index 91dfda2..c6033d5 100644
> --- a/Documentation/devicetree/bindings/misc/bmp085.txt
> +++ b/Documentation/devicetree/bindings/misc/bmp085.txt
> @@ -8,6 +8,9 @@ Optional properties:
>  - temp-measurement-period: temperature measurement period (milliseconds)
>  - default-oversampling: default oversampling value to be used at startup,
>    value range is 0-3 with rising sensitivity.
> +- gpio: if device is using EOC irq line gpio can be specified to setup interrupt
> +  handling
> +- irq: interrupt with no gpio
>  
>  Example:
>  
> @@ -17,4 +20,9 @@ pressure@77 {
>  	chip-id = <10>;
>  	temp-measurement-period = <100>;
>  	default-oversampling = <2>;
> +	gpio = <&gpio0 17 0>;
> +
> +	OR
> +
> +	irq = <75>;

There's some contention over the description of gpio-based IRQs in DT.
>From the point of view of the device there is a logical IRQ output; the
fact that this happens to be wired up to a GPIO pin that can happen to
generate interrupts isn't anything to do with the device itself. There
are plenty of device we have now whose interrupt lines could be wired to
GPIOs. I see no reason to extend their bindings to support explicit
GPIOs for IRQs, and I see no reason the driver should have to handle
this.

It would be far nicer for the device binding to just have the interrupts
property, and for the gpio controller to act as an interrupt-controller,
with the appropriate pin management.

Thanks,
Mark.

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Arnd Bergmann]

On Friday 15 November 2013, Mark Rutland wrote:
> There's some contention over the description of gpio-based IRQs in DT.
> From the point of view of the device there is a logical IRQ output; the
> fact that this happens to be wired up to a GPIO pin that can happen to
> generate interrupts isn't anything to do with the device itself. There
> are plenty of device we have now whose interrupt lines could be wired to
> GPIOs. I see no reason to extend their bindings to support explicit
> GPIOs for IRQs, and I see no reason the driver should have to handle
> this.
> 
> It would be far nicer for the device binding to just have the interrupts
> property, and for the gpio controller to act as an interrupt-controller,
> with the appropriate pin management.

Yes, agreed. I missed this point in my review: the GPIO is used only
as an interrupt pin here, so there is no reason to know the GPIO number.

	Arnd

Re: [PATCH 3/3] misc: bmp085: Add missing platform data.

[Dr. H. Nikolaus Schaller]

Am 15.11.2013 um 14:58 schrieb Arnd Bergmann:

> On Thursday 14 November 2013, Marek Belisko wrote:
>> DT bindings contains more parameters to set so add them to platform data also
>> to have possibility to use on arch where DT isn't available yet.
>> 
>> Signed-off-by: Marek Belisko <marek@goldelico.com>
> 
> Can you give an example of a platform that uses this chip and cannot
> yet use DT in the mainline kernel? 

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap3-gta04.dts

exists but is far from being complete, because the transition to DT is really complex.

We still need some (private) board file for 3.13 and hope to have everything ready
for 3.14. But that depends on speed of acceptance of other drivers because
all DT things are still constantly moving.

So we will have to mix DT+board file for a while.

> If it's only for out-of-tree platforms, I'd prefer to leave this
> patch out of tree as well and put the burden on whoever maintains
> a non-DT platform in a private kernel.
> 
> 
>> diff --git a/include/linux/i2c/bmp085.h b/include/linux/i2c/bmp085.h
>> index b66cb98..addb972 100644
>> --- a/include/linux/i2c/bmp085.h
>> +++ b/include/linux/i2c/bmp085.h
> 
> Shouldn't this be in include/linux/platform_data? 
> 
> 	Arnd

-- hns

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Belisko Marek]

Hi Mark,

On Fri, Nov 15, 2013 at 4:30 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Nov 14, 2013 at 09:46:48PM +0000, Marek Belisko wrote:
>> Signed-off-by: Marek Belisko <marek@goldelico.com>
>> ---
>>  Documentation/devicetree/bindings/misc/bmp085.txt |  8 ++++
>>  drivers/misc/bmp085.c                             | 53 ++++++++++++++++++++---
>>  2 files changed, 55 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/misc/bmp085.txt b/Documentation/devicetree/bindings/misc/bmp085.txt
>> index 91dfda2..c6033d5 100644
>> --- a/Documentation/devicetree/bindings/misc/bmp085.txt
>> +++ b/Documentation/devicetree/bindings/misc/bmp085.txt
>> @@ -8,6 +8,9 @@ Optional properties:
>>  - temp-measurement-period: temperature measurement period (milliseconds)
>>  - default-oversampling: default oversampling value to be used at startup,
>>    value range is 0-3 with rising sensitivity.
>> +- gpio: if device is using EOC irq line gpio can be specified to setup interrupt
>> +  handling
>> +- irq: interrupt with no gpio
>>
>>  Example:
>>
>> @@ -17,4 +20,9 @@ pressure@77 {
>>       chip-id = <10>;
>>       temp-measurement-period = <100>;
>>       default-oversampling = <2>;
>> +     gpio = <&gpio0 17 0>;
>> +
>> +     OR
>> +
>> +     irq = <75>;
>
> There's some contention over the description of gpio-based IRQs in DT.
> From the point of view of the device there is a logical IRQ output; the
> fact that this happens to be wired up to a GPIO pin that can happen to
> generate interrupts isn't anything to do with the device itself. There
> are plenty of device we have now whose interrupt lines could be wired to
> GPIOs. I see no reason to extend their bindings to support explicit
> GPIOs for IRQs, and I see no reason the driver should have to handle
> this.
>
> It would be far nicer for the device binding to just have the interrupts
> property, and for the gpio controller to act as an interrupt-controller,
> with the appropriate pin management.
OK. Can you please give me some example in current bindings tree?
Something like in:
Documentation/devicetree/bindings/fb/mxsfb.txt ? So if I understand
correctly we define only property
interrupts = <xxx> which will be exact interrupt number for cpu gpio
connected to bmp085 irq line.
>
> Thanks,
> Mark.

Thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Arnd Bergmann]

On Monday 18 November 2013, Belisko Marek wrote:

> > It would be far nicer for the device binding to just have the interrupts
> > property, and for the gpio controller to act as an interrupt-controller,
> > with the appropriate pin management.
> OK. Can you please give me some example in current bindings tree?
> Something like in:
> Documentation/devicetree/bindings/fb/mxsfb.txt ? So if I understand
> correctly we define only property
> interrupts = <xxx> which will be exact interrupt number for cpu gpio
> connected to bmp085 irq line.
> >

See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for an example
of a device whose interrupt line is connected to a gpio controller. The
key here is to set the "interrupt-parent" to the gpio node and have
the irq specifier define an interrupt local to that node.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Gerhard Sittig]

[ trimmed Cc: list for DT ]
[ is the bindings/gpio/8xxx_gpio.txt document incomplete? ]

On Mon, Nov 18, 2013 at 12:38 +0100, Arnd Bergmann wrote:
> 
> See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for an
> example of a device whose interrupt line is connected to a gpio
> controller. The key here is to set the "interrupt-parent" to
> the gpio node and have the irq specifier define an interrupt
> local to that node.

I was wondering whether the gpio-controller nodes lack both the
'interrupt-controller' boolean flag as well as the
'#interrupt-cells = <2>' property.  The former may be optional,
I'm not certain there.  But the latter should be a hard
requirement without which the 'funkyfpga' references cannot get
parsed I'm afraid.

Am I missing something?  Did I get the names right?  Shall I send
a patch?  Is the code working without those specs since it
assumes knowledge that was not specified in the device tree?  And
would this be against the idea that the binding should be
"complete" and independent of who is interpreting the data?  Or
is the binding document just incomplete?


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Arnd Bergmann]

On Monday 18 November 2013, Gerhard Sittig wrote:
> [ trimmed Cc: list for DT ]
> [ is the bindings/gpio/8xxx_gpio.txt document incomplete? ]
> 
> On Mon, Nov 18, 2013 at 12:38 +0100, Arnd Bergmann wrote:
> > 
> > See Documentation/devicetree/bindings/gpio/8xxx_gpio.txt for an
> > example of a device whose interrupt line is connected to a gpio
> > controller. The key here is to set the "interrupt-parent" to
> > the gpio node and have the irq specifier define an interrupt
> > local to that node.
> 
> I was wondering whether the gpio-controller nodes lack both the
> 'interrupt-controller' boolean flag as well as the
> '#interrupt-cells = <2>' property.  The former may be optional,
> I'm not certain there.  But the latter should be a hard
> requirement without which the 'funkyfpga' references cannot get
> parsed I'm afraid.

You are absolutely right, I picked an example that was actually
wrong.

> Am I missing something?  Did I get the names right?  Shall I send
> a patch?  Is the code working without those specs since it
> assumes knowledge that was not specified in the device tree?  And
> would this be against the idea that the binding should be
> "complete" and independent of who is interpreting the data?  Or
> is the binding document just incomplete?

I think the problem is that this particular controller is never used
in "interrupts" properties, so nobody noticed the mistake. It probably
still works for gpio references and drivers using gpio_to_irq on those
numbers. It would be nice if you can send a patch to add those as
optional properties to the binding and the example. We can't really
make them mandatory properties now because that would make all existing
dts files with this controller invalid.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Gerhard Sittig]

re-format and re-word the device tree binding documentation for MPC8xxx
and compatibles, reference the common document for interrupt controllers
and remove outdated duplicate SoC specific information

Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Rob Herring@ <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>
---

This change should improve the "flow" of the 8xxx GPIO binding document,
starting with an overview, then discussing specific properties, and then
collecting examples at the end.  Re-formatting should improve readability
of enumerations.

I was not certain whether the kinds of changes to the document warrant a
split into several patches, separating whitespace/reformatting and the
content update.  For the moment I went with a single patch to not
overcomplicate matters.

Putting the interrupt-parent before the interrupts specifiers is a thing
of personal taste -- I'm a fan of hierarchy, finding the specific detail
first and then the addendum "oh, by the way, it's that other parent"
keeps confusing me as much as top-posting in email does. :)

Feel free to tell me when I got terminology wrong.  This is the only
real issue that may remain after this change IMO.

 .../devicetree/bindings/gpio/8xxx_gpio.txt         |   59 ++++++++++++--------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
index b0019eb5330e..7788e8df3a15 100644
--- a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
@@ -5,16 +5,41 @@ This is for the non-QE/CPM/GUTs GPIO controllers as found on
 
 Every GPIO controller node must have #gpio-cells property defined,
 this information will be used to translate gpio-specifiers.
+See bindings/gpio/gpio.txt for details of how to specify GPIO
+information for devices.
+
+The GPIO module usually is connected to the SoC's internal interrupt
+controller, see bindings/interrupt-controller/interrupts.txt (the
+interrupt client nodes section) for details how to specify this GPIO
+module's interrupt.
+
+The GPIO module may serve as another interrupt controller (cascaded to
+the SoC's internal interrupt controller).  See the interrupt controller
+nodes section in bindings/interrupt-controller/interrupts.txt for
+details.
 
 Required properties:
-- compatible : "fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio" for
-  83xx, "fsl,mpc8572-gpio" for 85xx and "fsl,mpc8610-gpio" for 86xx.
-- #gpio-cells : Should be two. The first cell is the pin number and the
-  second cell is used to specify optional parameters (currently unused).
- - interrupts : Interrupt mapping for GPIO IRQ.
- - interrupt-parent : Phandle for the interrupt controller that
-   services interrupts for this device.
-- gpio-controller : Marks the port as GPIO controller.
+- compatible:		"fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio"
+			for 83xx, "fsl,mpc8572-gpio" for 85xx and
+			"fsl,mpc8610-gpio" for 86xx.
+- #gpio-cells:		Should be two. The first cell is the pin number
+			and the second cell is used to specify optional
+			parameters (currently unused).
+- interrupt-parent:	Phandle for the interrupt controller that
+			services interrupts for this device.
+- interrupts:		Interrupt mapping for GPIO IRQ.
+- gpio-controller:	Marks the port as GPIO controller.
+
+Optional properties:
+- interrupt-controller:	Empty boolean property which marks the GPIO
+			module as an IRQ controller.
+- #interrupt-cells:	Number of integer cells required to specify an
+			interrupt within this interrupt controller.  The
+			first cell defines the pin number, the second
+			cell defines additional flags (trigger type,
+			trigger polarity).  Note that the specific set
+			of trigger conditions supported by the GPIO
+			module depends on the actual SoC.
 
 Example of gpio-controller nodes for a MPC8347 SoC:
 
@@ -36,25 +61,11 @@ Example of gpio-controller nodes for a MPC8347 SoC:
 		gpio-controller;
 	};
 
-See booting-without-of.txt for details of how to specify GPIO
-information for devices.
-
-To use GPIO pins as interrupt sources for peripherals, specify the
-GPIO controller as the interrupt parent and define GPIO number +
-trigger mode using the interrupts property, which is defined like
-this:
-
-interrupts = <number trigger>, where:
- - number: GPIO pin (0..31)
- - trigger: trigger mode:
-	2 = trigger on falling edge
-	3 = trigger on both edges
-
-Example of device using this is:
+Example of a peripheral using the GPIO module as an IRQ controller:
 
 	funkyfpga@0 {
 		compatible = "funky-fpga";
 		...
-		interrupts = <4 3>;
 		interrupt-parent = <&gpio1>;
+		interrupts = <4 3>;
 	};
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.

[Gerhard Sittig]

On Mon, Nov 18, 2013 at 23:28 +0100, Arnd Bergmann wrote:
> 
> [ ... incomplete 8xxx gpio binding document ... ]
> 
> You are absolutely right, I picked an example that was actually
> wrong.
> 
> [ ... ]
> 
> I think the problem is that this particular controller is never
> used in "interrupts" properties, so nobody noticed the mistake.
> It probably still works for gpio references and drivers using
> gpio_to_irq on those numbers.

Actually that very GPIO module _is_ being used as an IRQ
controller as well.  The MPC5121 is one of those (not explicitly
listed) compatibles.  The mpc5121.dtsi correctly declares the
GPIO module as both a GPIO as well as an IRQ controller, and the
ac14xx.dts board does use GPIO lines as IRQs.

Which suggests that the issue is just incomplete documentation.
And that an incomplete binding document does not prevent correct
use of the binding. :)  And more importantly that the use of DT
is so intuitive that people get things right without looking at
the docs. :-]


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Rob Herring]

On Wed, Nov 20, 2013 at 3:34 PM, Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org> wrote:
> re-format and re-word the device tree binding documentation for MPC8xxx
> and compatibles, reference the common document for interrupt controllers
> and remove outdated duplicate SoC specific information
>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring@ <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>
> ---
>
> This change should improve the "flow" of the 8xxx GPIO binding document,
> starting with an overview, then discussing specific properties, and then
> collecting examples at the end.  Re-formatting should improve readability
> of enumerations.
>
> I was not certain whether the kinds of changes to the document warrant a
> split into several patches, separating whitespace/reformatting and the
> content update.  For the moment I went with a single patch to not
> overcomplicate matters.
>
> Putting the interrupt-parent before the interrupts specifiers is a thing
> of personal taste -- I'm a fan of hierarchy, finding the specific detail
> first and then the addendum "oh, by the way, it's that other parent"
> keeps confusing me as much as top-posting in email does. :)
>
> Feel free to tell me when I got terminology wrong.  This is the only
> real issue that may remain after this change IMO.
>
>  .../devicetree/bindings/gpio/8xxx_gpio.txt         |   59 ++++++++++++--------
>  1 file changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
> index b0019eb5330e..7788e8df3a15 100644
> --- a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
> @@ -5,16 +5,41 @@ This is for the non-QE/CPM/GUTs GPIO controllers as found on
>
>  Every GPIO controller node must have #gpio-cells property defined,
>  this information will be used to translate gpio-specifiers.
> +See bindings/gpio/gpio.txt for details of how to specify GPIO
> +information for devices.
> +
> +The GPIO module usually is connected to the SoC's internal interrupt
> +controller, see bindings/interrupt-controller/interrupts.txt (the
> +interrupt client nodes section) for details how to specify this GPIO
> +module's interrupt.
> +
> +The GPIO module may serve as another interrupt controller (cascaded to
> +the SoC's internal interrupt controller).  See the interrupt controller
> +nodes section in bindings/interrupt-controller/interrupts.txt for
> +details.
>
>  Required properties:
> -- compatible : "fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio" for
> -  83xx, "fsl,mpc8572-gpio" for 85xx and "fsl,mpc8610-gpio" for 86xx.
> -- #gpio-cells : Should be two. The first cell is the pin number and the
> -  second cell is used to specify optional parameters (currently unused).
> - - interrupts : Interrupt mapping for GPIO IRQ.
> - - interrupt-parent : Phandle for the interrupt controller that
> -   services interrupts for this device.
> -- gpio-controller : Marks the port as GPIO controller.
> +- compatible:          "fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio"

Can you use <chip> here. Only because I'm developing some
documentation checking scripts and handle that already.

> +                       for 83xx, "fsl,mpc8572-gpio" for 85xx and

s/and/or/

> +                       "fsl,mpc8610-gpio" for 86xx.
> +- #gpio-cells:         Should be two. The first cell is the pin number
> +                       and the second cell is used to specify optional
> +                       parameters (currently unused).
> +- interrupt-parent:    Phandle for the interrupt controller that
> +                       services interrupts for this device.
> +- interrupts:          Interrupt mapping for GPIO IRQ.
> +- gpio-controller:     Marks the port as GPIO controller.
> +
> +Optional properties:
> +- interrupt-controller:        Empty boolean property which marks the GPIO
> +                       module as an IRQ controller.
> +- #interrupt-cells:    Number of integer cells required to specify an

Should start with "Should be two."

> +                       interrupt within this interrupt controller.  The
> +                       first cell defines the pin number, the second
> +                       cell defines additional flags (trigger type,
> +                       trigger polarity).  Note that the specific set
> +                       of trigger conditions supported by the GPIO
> +                       module depends on the actual SoC.

You mean what values are supported vary, not the meaning of the values right?

>  Example of gpio-controller nodes for a MPC8347 SoC:
>
> @@ -36,25 +61,11 @@ Example of gpio-controller nodes for a MPC8347 SoC:
>                 gpio-controller;
>         };
>
> -See booting-without-of.txt for details of how to specify GPIO
> -information for devices.
> -
> -To use GPIO pins as interrupt sources for peripherals, specify the
> -GPIO controller as the interrupt parent and define GPIO number +
> -trigger mode using the interrupts property, which is defined like
> -this:
> -
> -interrupts = <number trigger>, where:
> - - number: GPIO pin (0..31)
> - - trigger: trigger mode:
> -       2 = trigger on falling edge
> -       3 = trigger on both edges
> -
> -Example of device using this is:
> +Example of a peripheral using the GPIO module as an IRQ controller:
>
>         funkyfpga@0 {
>                 compatible = "funky-fpga";
>                 ...
> -               interrupts = <4 3>;
>                 interrupt-parent = <&gpio1>;
> +               interrupts = <4 3>;
>         };
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Arnd Bergmann]

On Wednesday 20 November 2013, Gerhard Sittig wrote:
> re-format and re-word the device tree binding documentation for MPC8xxx
> and compatibles, reference the common document for interrupt controllers
> and remove outdated duplicate SoC specific information
> 
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring@ <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>

Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

> +Optional properties:
> +- interrupt-controller:	Empty boolean property which marks the GPIO
> +			module as an IRQ controller.
> +- #interrupt-cells:	Number of integer cells required to specify an
> +			interrupt within this interrupt controller.  The
> +			first cell defines the pin number, the second
> +			cell defines additional flags (trigger type,
> +			trigger polarity).  Note that the specific set
> +			of trigger conditions supported by the GPIO
> +			module depends on the actual SoC.

Nitpicking: I think you need to add that #interrupt-cells must be <2>
if present. I'd also suggest adding these two to the example section
of the binding while you're at it.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Gerhard Sittig]

On Wed, Nov 20, 2013 at 17:50 -0600, Rob Herring wrote:
> 
> On Wed, Nov 20, 2013 at 3:34 PM, Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org> wrote:
> > re-format and re-word the device tree binding documentation for MPC8xxx
> > and compatibles, reference the common document for interrupt controllers
> > and remove outdated duplicate SoC specific information
> >
> > [ ... ]
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
> > index b0019eb5330e..7788e8df3a15 100644
> > --- a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
> > +++ b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
> > @@ -5,16 +5,41 @@ This is for the non-QE/CPM/GUTs GPIO controllers as found on
> >
> > [ ... ]
> >
> >  Required properties:
> > -- compatible : "fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio" for
> > -  83xx, "fsl,mpc8572-gpio" for 85xx and "fsl,mpc8610-gpio" for 86xx.
> > -- #gpio-cells : Should be two. The first cell is the pin number and the
> > -  second cell is used to specify optional parameters (currently unused).
> > - - interrupts : Interrupt mapping for GPIO IRQ.
> > - - interrupt-parent : Phandle for the interrupt controller that
> > -   services interrupts for this device.
> > -- gpio-controller : Marks the port as GPIO controller.
> > +- compatible:          "fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio"
> 
> Can you use <chip> here. Only because I'm developing some
> documentation checking scripts and handle that already.

will do

> 
> > +                       for 83xx, "fsl,mpc8572-gpio" for 85xx and
> 
> s/and/or/

will do

> 
> > +                       "fsl,mpc8610-gpio" for 86xx.
> > +- #gpio-cells:         Should be two. The first cell is the pin number
> > +                       and the second cell is used to specify optional
> > +                       parameters (currently unused).
> > +- interrupt-parent:    Phandle for the interrupt controller that
> > +                       services interrupts for this device.
> > +- interrupts:          Interrupt mapping for GPIO IRQ.
> > +- gpio-controller:     Marks the port as GPIO controller.
> > +
> > +Optional properties:
> > +- interrupt-controller:        Empty boolean property which marks the GPIO
> > +                       module as an IRQ controller.
> > +- #interrupt-cells:    Number of integer cells required to specify an
> 
> Should start with "Should be two."

will do

> 
> > +                       interrupt within this interrupt controller.  The
> > +                       first cell defines the pin number, the second
> > +                       cell defines additional flags (trigger type,
> > +                       trigger polarity).  Note that the specific set
> > +                       of trigger conditions supported by the GPIO
> > +                       module depends on the actual SoC.
> 
> You mean what values are supported vary, not the meaning of the values right?

yes, encoding of trigger conditions is specified in the common
document and the values remain the same, but the set of possible
values which apply to a SoC are individual


thank you for the feedback


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Gerhard Sittig]

On Thu, Nov 21, 2013 at 01:49 +0100, Arnd Bergmann wrote:
> 
> On Wednesday 20 November 2013, Gerhard Sittig wrote:
> > re-format and re-word the device tree binding documentation for MPC8xxx
> > and compatibles, reference the common document for interrupt controllers
> > and remove outdated duplicate SoC specific information
> > 
> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > Cc: Rob Herring@ <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > Cc: Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>
> > Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>
> 
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

thank you for the feedback and for the ACK

> > +Optional properties:
> > +- interrupt-controller:	Empty boolean property which marks the GPIO
> > +			module as an IRQ controller.
> > +- #interrupt-cells:	Number of integer cells required to specify an
> > +			interrupt within this interrupt controller.  The
> > +			first cell defines the pin number, the second
> > +			cell defines additional flags (trigger type,
> > +			trigger polarity).  Note that the specific set
> > +			of trigger conditions supported by the GPIO
> > +			module depends on the actual SoC.
> 
> Nitpicking: I think you need to add that #interrupt-cells must be <2>
> if present. I'd also suggest adding these two to the example section
> of the binding while you're at it.

will do

and I assume that your ACK still applies after I have addressed
the feedback from you and from Rob


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Gerhard Sittig]

re-format and re-word the device tree binding documentation for MPC8xxx
and compatibles, reference the common document for interrupt controllers
and remove outdated duplicate SoC specific information

Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Rob Herring@ <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>
---

changes in v2:
- s/CHIP/chip/ in 'compatible', s/and/or/ for the
  list of strings to use in 'compatible'
- "should be two" and "s/specific/available/ set of
  trigger conditions" in '#interrupt-cells'
- IRQ controller properties added to the 'gpio1' example node
  as it is referenced as one from 'funkyfpga'
---
 .../devicetree/bindings/gpio/8xxx_gpio.txt         |   66 ++++++++++++--------
 1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
index b0019eb5330e..798cfc9d3839 100644
--- a/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/8xxx_gpio.txt
@@ -5,16 +5,42 @@ This is for the non-QE/CPM/GUTs GPIO controllers as found on
 
 Every GPIO controller node must have #gpio-cells property defined,
 this information will be used to translate gpio-specifiers.
+See bindings/gpio/gpio.txt for details of how to specify GPIO
+information for devices.
+
+The GPIO module usually is connected to the SoC's internal interrupt
+controller, see bindings/interrupt-controller/interrupts.txt (the
+interrupt client nodes section) for details how to specify this GPIO
+module's interrupt.
+
+The GPIO module may serve as another interrupt controller (cascaded to
+the SoC's internal interrupt controller).  See the interrupt controller
+nodes section in bindings/interrupt-controller/interrupts.txt for
+details.
 
 Required properties:
-- compatible : "fsl,<CHIP>-gpio" followed by "fsl,mpc8349-gpio" for
-  83xx, "fsl,mpc8572-gpio" for 85xx and "fsl,mpc8610-gpio" for 86xx.
-- #gpio-cells : Should be two. The first cell is the pin number and the
-  second cell is used to specify optional parameters (currently unused).
- - interrupts : Interrupt mapping for GPIO IRQ.
- - interrupt-parent : Phandle for the interrupt controller that
-   services interrupts for this device.
-- gpio-controller : Marks the port as GPIO controller.
+- compatible:		"fsl,<chip>-gpio" followed by "fsl,mpc8349-gpio"
+			for 83xx, "fsl,mpc8572-gpio" for 85xx, or
+			"fsl,mpc8610-gpio" for 86xx.
+- #gpio-cells:		Should be two. The first cell is the pin number
+			and the second cell is used to specify optional
+			parameters (currently unused).
+- interrupt-parent:	Phandle for the interrupt controller that
+			services interrupts for this device.
+- interrupts:		Interrupt mapping for GPIO IRQ.
+- gpio-controller:	Marks the port as GPIO controller.
+
+Optional properties:
+- interrupt-controller:	Empty boolean property which marks the GPIO
+			module as an IRQ controller.
+- #interrupt-cells:	Should be two.  Defines the number of integer
+			cells required to specify an interrupt within
+			this interrupt controller.  The first cell
+			defines the pin number, the second cell
+			defines additional flags (trigger type,
+			trigger polarity).  Note that the available
+			set of trigger conditions supported by the
+			GPIO module depends on the actual SoC.
 
 Example of gpio-controller nodes for a MPC8347 SoC:
 
@@ -22,39 +48,27 @@ Example of gpio-controller nodes for a MPC8347 SoC:
 		#gpio-cells = <2>;
 		compatible = "fsl,mpc8347-gpio", "fsl,mpc8349-gpio";
 		reg = <0xc00 0x100>;
-		interrupts = <74 0x8>;
 		interrupt-parent = <&ipic>;
+		interrupts = <74 0x8>;
 		gpio-controller;
+		interrupt-controller;
+		#interrupt-cells = <2>;
 	};
 
 	gpio2: gpio-controller@d00 {
 		#gpio-cells = <2>;
 		compatible = "fsl,mpc8347-gpio", "fsl,mpc8349-gpio";
 		reg = <0xd00 0x100>;
-		interrupts = <75 0x8>;
 		interrupt-parent = <&ipic>;
+		interrupts = <75 0x8>;
 		gpio-controller;
 	};
 
-See booting-without-of.txt for details of how to specify GPIO
-information for devices.
-
-To use GPIO pins as interrupt sources for peripherals, specify the
-GPIO controller as the interrupt parent and define GPIO number +
-trigger mode using the interrupts property, which is defined like
-this:
-
-interrupts = <number trigger>, where:
- - number: GPIO pin (0..31)
- - trigger: trigger mode:
-	2 = trigger on falling edge
-	3 = trigger on both edges
-
-Example of device using this is:
+Example of a peripheral using the GPIO module as an IRQ controller:
 
 	funkyfpga@0 {
 		compatible = "funky-fpga";
 		...
-		interrupts = <4 3>;
 		interrupt-parent = <&gpio1>;
+		interrupts = <4 3>;
 	};
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] dt: binding: reword PowerPC 8xxx GPIO documentation

[Rob Herring]

On Thu, Nov 21, 2013 at 2:41 AM, Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org> wrote:
> re-format and re-word the device tree binding documentation for MPC8xxx
> and compatibles, reference the common document for interrupt controllers
> and remove outdated duplicate SoC specific information
>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring@ <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Pawel Moll <Pawel.Moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>
> ---
>
> changes in v2:
> - s/CHIP/chip/ in 'compatible', s/and/or/ for the
>   list of strings to use in 'compatible'
> - "should be two" and "s/specific/available/ set of
>   trigger conditions" in '#interrupt-cells'
> - IRQ controller properties added to the 'gpio1' example node
>   as it is referenced as one from 'funkyfpga'
> ---
>  .../devicetree/bindings/gpio/8xxx_gpio.txt         |   66 ++++++++++++--------
>  1 file changed, 40 insertions(+), 26 deletions(-)

Applied for 3.13.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html