linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates
@ 2013-11-14 21:46 Marek Belisko
  2013-11-14 21:46 ` [PATCH 1/3] misc: bmp085: Clean up and enable use of interrupt for completion Marek Belisko
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Marek Belisko @ 2013-11-14 21:46 UTC (permalink / raw)
  To: arnd, gregkh
  Cc: neilb, hns, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, devicetree, linux-doc, linux-kernel,
	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


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

* [PATCH 1/3] misc: bmp085: Clean up and enable use of interrupt for completion.
  2013-11-14 21:46 [PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates Marek Belisko
@ 2013-11-14 21:46 ` Marek Belisko
  2013-11-14 21:46 ` [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq Marek Belisko
  2013-11-14 21:46 ` [PATCH 3/3] misc: bmp085: Add missing platform data Marek Belisko
  2 siblings, 0 replies; 11+ messages in thread
From: Marek Belisko @ 2013-11-14 21:46 UTC (permalink / raw)
  To: arnd, gregkh
  Cc: neilb, hns, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, devicetree, linux-doc, linux-kernel,
	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@suse.de>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Marek Belisko <marek@goldelico.com>
---
 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


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

* [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  2013-11-14 21:46 [PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates Marek Belisko
  2013-11-14 21:46 ` [PATCH 1/3] misc: bmp085: Clean up and enable use of interrupt for completion Marek Belisko
@ 2013-11-14 21:46 ` Marek Belisko
  2013-11-15 13:56   ` Arnd Bergmann
  2013-11-15 15:30   ` Mark Rutland
  2013-11-14 21:46 ` [PATCH 3/3] misc: bmp085: Add missing platform data Marek Belisko
  2 siblings, 2 replies; 11+ messages in thread
From: Marek Belisko @ 2013-11-14 21:46 UTC (permalink / raw)
  To: arnd, gregkh
  Cc: neilb, hns, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, devicetree, linux-doc, linux-kernel,
	Marek Belisko

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>;
 };
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


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

* [PATCH 3/3] misc: bmp085: Add missing platform data.
  2013-11-14 21:46 [PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates Marek Belisko
  2013-11-14 21:46 ` [PATCH 1/3] misc: bmp085: Clean up and enable use of interrupt for completion Marek Belisko
  2013-11-14 21:46 ` [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq Marek Belisko
@ 2013-11-14 21:46 ` Marek Belisko
  2013-11-15 13:58   ` Arnd Bergmann
  2 siblings, 1 reply; 11+ messages in thread
From: Marek Belisko @ 2013-11-14 21:46 UTC (permalink / raw)
  To: arnd, gregkh
  Cc: neilb, hns, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, devicetree, linux-doc, linux-kernel,
	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


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

* Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  2013-11-14 21:46 ` [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq Marek Belisko
@ 2013-11-15 13:56   ` Arnd Bergmann
  2013-11-15 15:30   ` Mark Rutland
  1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2013-11-15 13:56 UTC (permalink / raw)
  To: Marek Belisko
  Cc: gregkh, neilb, hns, rob.herring, pawel.moll, mark.rutland,
	swarren, ijc+devicetree, rob, devicetree, linux-doc,
	linux-kernel

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

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

* Re: [PATCH 3/3] misc: bmp085: Add missing platform data.
  2013-11-14 21:46 ` [PATCH 3/3] misc: bmp085: Add missing platform data Marek Belisko
@ 2013-11-15 13:58   ` Arnd Bergmann
  2013-11-16  8:23     ` Dr. H. Nikolaus Schaller
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2013-11-15 13:58 UTC (permalink / raw)
  To: Marek Belisko
  Cc: gregkh, neilb, hns, rob.herring, pawel.moll, mark.rutland,
	swarren, ijc+devicetree, rob, devicetree, linux-doc,
	linux-kernel

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

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

* Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  2013-11-14 21:46 ` [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq Marek Belisko
  2013-11-15 13:56   ` Arnd Bergmann
@ 2013-11-15 15:30   ` Mark Rutland
  2013-11-15 18:47     ` Arnd Bergmann
  2013-11-18  8:58     ` Belisko Marek
  1 sibling, 2 replies; 11+ messages in thread
From: Mark Rutland @ 2013-11-15 15:30 UTC (permalink / raw)
  To: Marek Belisko
  Cc: arnd, gregkh, neilb, hns, rob.herring, Pawel Moll, swarren,
	ijc+devicetree, rob, devicetree, linux-doc, linux-kernel

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.

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

* Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  2013-11-15 15:30   ` Mark Rutland
@ 2013-11-15 18:47     ` Arnd Bergmann
  2013-11-18  8:58     ` Belisko Marek
  1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2013-11-15 18:47 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Marek Belisko, gregkh, neilb, hns, rob.herring, Pawel Moll,
	swarren, ijc+devicetree, rob, devicetree, linux-doc,
	linux-kernel

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

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

* Re: [PATCH 3/3] misc: bmp085: Add missing platform data.
  2013-11-15 13:58   ` Arnd Bergmann
@ 2013-11-16  8:23     ` Dr. H. Nikolaus Schaller
  0 siblings, 0 replies; 11+ messages in thread
From: Dr. H. Nikolaus Schaller @ 2013-11-16  8:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Marek Belisko, gregkh, neilb, rob.herring, pawel.moll,
	mark.rutland, swarren, ijc+devicetree, rob, devicetree,
	linux-doc, linux-kernel


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

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

* Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  2013-11-15 15:30   ` Mark Rutland
  2013-11-15 18:47     ` Arnd Bergmann
@ 2013-11-18  8:58     ` Belisko Marek
  2013-11-18 11:38       ` Arnd Bergmann
  1 sibling, 1 reply; 11+ messages in thread
From: Belisko Marek @ 2013-11-18  8:58 UTC (permalink / raw)
  To: Mark Rutland
  Cc: arnd, gregkh, neilb, hns, rob.herring, Pawel Moll, swarren,
	ijc+devicetree, rob, devicetree, linux-doc, linux-kernel

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

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

* Re: [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq.
  2013-11-18  8:58     ` Belisko Marek
@ 2013-11-18 11:38       ` Arnd Bergmann
  0 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2013-11-18 11:38 UTC (permalink / raw)
  To: Belisko Marek
  Cc: Mark Rutland, gregkh, neilb, hns, rob.herring, Pawel Moll,
	swarren, ijc+devicetree, rob, devicetree, linux-doc,
	linux-kernel

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

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

end of thread, other threads:[~2013-11-18 11:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-14 21:46 [PATCH 0/3] Add EOC handling for bmp085 + DT and platform data updates Marek Belisko
2013-11-14 21:46 ` [PATCH 1/3] misc: bmp085: Clean up and enable use of interrupt for completion Marek Belisko
2013-11-14 21:46 ` [PATCH 2/3] misc: bmp085: Add DT bindings for EOC gpio line and direct irq Marek Belisko
2013-11-15 13:56   ` Arnd Bergmann
2013-11-15 15:30   ` Mark Rutland
2013-11-15 18:47     ` Arnd Bergmann
2013-11-18  8:58     ` Belisko Marek
2013-11-18 11:38       ` Arnd Bergmann
2013-11-14 21:46 ` [PATCH 3/3] misc: bmp085: Add missing platform data Marek Belisko
2013-11-15 13:58   ` Arnd Bergmann
2013-11-16  8:23     ` Dr. H. Nikolaus Schaller

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