All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] gpio: pca953x updates for DT
@ 2014-07-29  7:24 ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel, Markus Pargmann

Hi,

here are some updates for the pca953x driver for handling DT. The series
includes some cleanup of deprecated DT properties, adds documentation and
handling of reset gpio for the chip.

Best regards,

Markus


Markus Pargmann (4):
  gpio: pca953x: Drop deprecated DT bindings
  gpio: pca953x: Add DT binding documentation
  gpio: pca953x: Add pca9506 as DT compatible
  gpio: pca953x: Add DT binding for reset gpio

 .../devicetree/bindings/gpio/gpio-pca953x.txt      | 45 +++++++++++++
 drivers/gpio/gpio-pca953x.c                        | 73 +++++++---------------
 2 files changed, 67 insertions(+), 51 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt

-- 
2.0.1


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

* [PATCH 0/4] gpio: pca953x updates for DT
@ 2014-07-29  7:24 ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

here are some updates for the pca953x driver for handling DT. The series
includes some cleanup of deprecated DT properties, adds documentation and
handling of reset gpio for the chip.

Best regards,

Markus


Markus Pargmann (4):
  gpio: pca953x: Drop deprecated DT bindings
  gpio: pca953x: Add DT binding documentation
  gpio: pca953x: Add pca9506 as DT compatible
  gpio: pca953x: Add DT binding for reset gpio

 .../devicetree/bindings/gpio/gpio-pca953x.txt      | 45 +++++++++++++
 drivers/gpio/gpio-pca953x.c                        | 73 +++++++---------------
 2 files changed, 67 insertions(+), 51 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt

-- 
2.0.1

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

* [PATCH 1/4] gpio: pca953x: Drop deprecated DT bindings
  2014-07-29  7:24 ` Markus Pargmann
@ 2014-07-29  7:24   ` Markus Pargmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel, Markus Pargmann

Drop deprecated DT bindings and use automaticly assigned gpio and irq
bases.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/gpio/gpio-pca953x.c | 54 +++------------------------------------------
 1 file changed, 3 insertions(+), 51 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e721a37c3473..fda3eae835d0 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -520,7 +520,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 	struct i2c_client *client = chip->client;
 	int ret, i, offset = 0;
 
-	if (irq_base != -1
+	if (client->irq && irq_base != -1
 			&& (id->driver_data & PCA_INT)) {
 
 		switch (chip->chip_type) {
@@ -586,50 +586,6 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 }
 #endif
 
-/*
- * Handlers for alternative sources of platform_data
- */
-#ifdef CONFIG_OF_GPIO
-/*
- * Translate OpenFirmware node properties into platform_data
- * WARNING: This is DEPRECATED and will be removed eventually!
- */
-static void
-pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, u32 *invert)
-{
-	struct device_node *node;
-	const __be32 *val;
-	int size;
-
-	*gpio_base = -1;
-
-	node = client->dev.of_node;
-	if (node == NULL)
-		return;
-
-	val = of_get_property(node, "linux,gpio-base", &size);
-	WARN(val, "%s: device-tree property 'linux,gpio-base' is deprecated!", __func__);
-	if (val) {
-		if (size != sizeof(*val))
-			dev_warn(&client->dev, "%s: wrong linux,gpio-base\n",
-				 node->full_name);
-		else
-			*gpio_base = be32_to_cpup(val);
-	}
-
-	val = of_get_property(node, "polarity", NULL);
-	WARN(val, "%s: device-tree property 'polarity' is deprecated!", __func__);
-	if (val)
-		*invert = *val;
-}
-#else
-static void
-pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, u32 *invert)
-{
-	*gpio_base = -1;
-}
-#endif
-
 static int device_pca953x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
@@ -704,12 +660,8 @@ static int pca953x_probe(struct i2c_client *client,
 		invert = pdata->invert;
 		chip->names = pdata->names;
 	} else {
-		pca953x_get_alt_pdata(client, &chip->gpio_start, &invert);
-#ifdef CONFIG_OF_GPIO
-		/* If I2C node has no interrupts property, disable GPIO interrupts */
-		if (of_find_property(client->dev.of_node, "interrupts", NULL) == NULL)
-			irq_base = -1;
-#endif
+		chip->gpio_start = -1;
+		irq_base = 0;
 	}
 
 	chip->client = client;
-- 
2.0.1


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

* [PATCH 1/4] gpio: pca953x: Drop deprecated DT bindings
@ 2014-07-29  7:24   ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

Drop deprecated DT bindings and use automaticly assigned gpio and irq
bases.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/gpio/gpio-pca953x.c | 54 +++------------------------------------------
 1 file changed, 3 insertions(+), 51 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e721a37c3473..fda3eae835d0 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -520,7 +520,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 	struct i2c_client *client = chip->client;
 	int ret, i, offset = 0;
 
-	if (irq_base != -1
+	if (client->irq && irq_base != -1
 			&& (id->driver_data & PCA_INT)) {
 
 		switch (chip->chip_type) {
@@ -586,50 +586,6 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 }
 #endif
 
-/*
- * Handlers for alternative sources of platform_data
- */
-#ifdef CONFIG_OF_GPIO
-/*
- * Translate OpenFirmware node properties into platform_data
- * WARNING: This is DEPRECATED and will be removed eventually!
- */
-static void
-pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, u32 *invert)
-{
-	struct device_node *node;
-	const __be32 *val;
-	int size;
-
-	*gpio_base = -1;
-
-	node = client->dev.of_node;
-	if (node == NULL)
-		return;
-
-	val = of_get_property(node, "linux,gpio-base", &size);
-	WARN(val, "%s: device-tree property 'linux,gpio-base' is deprecated!", __func__);
-	if (val) {
-		if (size != sizeof(*val))
-			dev_warn(&client->dev, "%s: wrong linux,gpio-base\n",
-				 node->full_name);
-		else
-			*gpio_base = be32_to_cpup(val);
-	}
-
-	val = of_get_property(node, "polarity", NULL);
-	WARN(val, "%s: device-tree property 'polarity' is deprecated!", __func__);
-	if (val)
-		*invert = *val;
-}
-#else
-static void
-pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, u32 *invert)
-{
-	*gpio_base = -1;
-}
-#endif
-
 static int device_pca953x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
@@ -704,12 +660,8 @@ static int pca953x_probe(struct i2c_client *client,
 		invert = pdata->invert;
 		chip->names = pdata->names;
 	} else {
-		pca953x_get_alt_pdata(client, &chip->gpio_start, &invert);
-#ifdef CONFIG_OF_GPIO
-		/* If I2C node has no interrupts property, disable GPIO interrupts */
-		if (of_find_property(client->dev.of_node, "interrupts", NULL) == NULL)
-			irq_base = -1;
-#endif
+		chip->gpio_start = -1;
+		irq_base = 0;
 	}
 
 	chip->client = client;
-- 
2.0.1

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

* [PATCH 2/4] gpio: pca953x: Add DT binding documentation
  2014-07-29  7:24 ` Markus Pargmann
@ 2014-07-29  7:24   ` Markus Pargmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel, Markus Pargmann

Add a devicetree binding documentation for the pca953x driver.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../devicetree/bindings/gpio/gpio-pca953x.txt      | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
new file mode 100644
index 000000000000..b9a42f294dd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -0,0 +1,39 @@
+* NXP PCA953x I2C GPIO multiplexer
+
+Required properties:
+ - compatible: Has to contain one of the following:
+	nxp,pca9505
+	nxp,pca9534
+	nxp,pca9535
+	nxp,pca9536
+	nxp,pca9537
+	nxp,pca9538
+	nxp,pca9539
+	nxp,pca9554
+	nxp,pca9555
+	nxp,pca9556
+	nxp,pca9557
+	nxp,pca9574
+	nxp,pca9575
+	nxp,pca9698
+	maxim,max7310
+	maxim,max7312
+	maxim,max7313
+	maxim,max7315
+	ti,pca6107
+	ti,tca6408
+	ti,tca6416
+	ti,tca6424
+	exar,xra1202
+
+Example:
+
+
+	gpio@20 {
+		compatible = "nxp,pca9505";
+		reg = <0x20>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pca9505>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+	};
-- 
2.0.1


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

* [PATCH 2/4] gpio: pca953x: Add DT binding documentation
@ 2014-07-29  7:24   ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

Add a devicetree binding documentation for the pca953x driver.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../devicetree/bindings/gpio/gpio-pca953x.txt      | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
new file mode 100644
index 000000000000..b9a42f294dd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -0,0 +1,39 @@
+* NXP PCA953x I2C GPIO multiplexer
+
+Required properties:
+ - compatible: Has to contain one of the following:
+	nxp,pca9505
+	nxp,pca9534
+	nxp,pca9535
+	nxp,pca9536
+	nxp,pca9537
+	nxp,pca9538
+	nxp,pca9539
+	nxp,pca9554
+	nxp,pca9555
+	nxp,pca9556
+	nxp,pca9557
+	nxp,pca9574
+	nxp,pca9575
+	nxp,pca9698
+	maxim,max7310
+	maxim,max7312
+	maxim,max7313
+	maxim,max7315
+	ti,pca6107
+	ti,tca6408
+	ti,tca6416
+	ti,tca6424
+	exar,xra1202
+
+Example:
+
+
+	gpio at 20 {
+		compatible = "nxp,pca9505";
+		reg = <0x20>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pca9505>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+	};
-- 
2.0.1

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

* [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible
  2014-07-29  7:24 ` Markus Pargmann
@ 2014-07-29  7:24   ` Markus Pargmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel, Markus Pargmann

This driver can also be used for the nxp,pca9506 device.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
 drivers/gpio/gpio-pca953x.c                             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
index b9a42f294dd0..eb65157d47f6 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -3,6 +3,7 @@
 Required properties:
  - compatible: Has to contain one of the following:
 	nxp,pca9505
+	nxp,pca9506
 	nxp,pca9534
 	nxp,pca9535
 	nxp,pca9536
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index fda3eae835d0..df5eb6e6be1e 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -729,6 +729,7 @@ static int pca953x_remove(struct i2c_client *client)
 
 static const struct of_device_id pca953x_dt_ids[] = {
 	{ .compatible = "nxp,pca9505", },
+	{ .compatible = "nxp,pca9506", },
 	{ .compatible = "nxp,pca9534", },
 	{ .compatible = "nxp,pca9535", },
 	{ .compatible = "nxp,pca9536", },
-- 
2.0.1


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

* [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible
@ 2014-07-29  7:24   ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

This driver can also be used for the nxp,pca9506 device.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
 drivers/gpio/gpio-pca953x.c                             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
index b9a42f294dd0..eb65157d47f6 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -3,6 +3,7 @@
 Required properties:
  - compatible: Has to contain one of the following:
 	nxp,pca9505
+	nxp,pca9506
 	nxp,pca9534
 	nxp,pca9535
 	nxp,pca9536
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index fda3eae835d0..df5eb6e6be1e 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -729,6 +729,7 @@ static int pca953x_remove(struct i2c_client *client)
 
 static const struct of_device_id pca953x_dt_ids[] = {
 	{ .compatible = "nxp,pca9505", },
+	{ .compatible = "nxp,pca9506", },
 	{ .compatible = "nxp,pca9534", },
 	{ .compatible = "nxp,pca9535", },
 	{ .compatible = "nxp,pca9536", },
-- 
2.0.1

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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
  2014-07-29  7:24 ` Markus Pargmann
@ 2014-07-29  7:24   ` Markus Pargmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel, Markus Pargmann

The pca953x has a negated reset input. This patch adds a DT binding for
the reset gpio and resets the chip when it is probed. This will reset
the device and leave the gpio in the correct state so reset is not
triggered.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../devicetree/bindings/gpio/gpio-pca953x.txt          |  5 +++++
 drivers/gpio/gpio-pca953x.c                            | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
index eb65157d47f6..57e31414f74d 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -27,6 +27,10 @@ Required properties:
 	ti,tca6424
 	exar,xra1202
 
+Optional properties:
+ - reset-gpios: phandle with arguments identifying the reset gpio. See
+   Documentation/devicetree/bindings/gpio/gpio.txt for more information
+
 Example:
 
 
@@ -37,4 +41,5 @@ Example:
 		pinctrl-0 = <&pinctrl_pca9505>;
 		interrupt-parent = <&gpio3>;
 		interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+		reset-gpios = <&gpio3 24 GPIO_ACTIVE_LOW>;
 	};
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index df5eb6e6be1e..053d8b4702e6 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -11,6 +11,7 @@
  *  the Free Software Foundation; version 2 of the License.
  */
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
@@ -660,8 +661,25 @@ static int pca953x_probe(struct i2c_client *client,
 		invert = pdata->invert;
 		chip->names = pdata->names;
 	} else {
+		struct gpio_desc *reset;
+
 		chip->gpio_start = -1;
 		irq_base = 0;
+
+		reset = devm_gpiod_get(&client->dev, "reset");
+		if (IS_ERR(reset)) {
+			if (PTR_ERR(reset) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			else
+				dev_info(&client->dev, "Did not find/get a gpio for reset (%ld)\n",
+						PTR_ERR(reset));
+		} else {
+			/* Reset the chip if the reset is wired */
+			gpiod_direction_output(reset, 0);
+			udelay(100);
+			gpiod_set_value(reset, 1);
+			udelay(100);
+		}
 	}
 
 	chip->client = client;
-- 
2.0.1


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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
@ 2014-07-29  7:24   ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-29  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

The pca953x has a negated reset input. This patch adds a DT binding for
the reset gpio and resets the chip when it is probed. This will reset
the device and leave the gpio in the correct state so reset is not
triggered.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../devicetree/bindings/gpio/gpio-pca953x.txt          |  5 +++++
 drivers/gpio/gpio-pca953x.c                            | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
index eb65157d47f6..57e31414f74d 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -27,6 +27,10 @@ Required properties:
 	ti,tca6424
 	exar,xra1202
 
+Optional properties:
+ - reset-gpios: phandle with arguments identifying the reset gpio. See
+   Documentation/devicetree/bindings/gpio/gpio.txt for more information
+
 Example:
 
 
@@ -37,4 +41,5 @@ Example:
 		pinctrl-0 = <&pinctrl_pca9505>;
 		interrupt-parent = <&gpio3>;
 		interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+		reset-gpios = <&gpio3 24 GPIO_ACTIVE_LOW>;
 	};
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index df5eb6e6be1e..053d8b4702e6 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -11,6 +11,7 @@
  *  the Free Software Foundation; version 2 of the License.
  */
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/gpio.h>
@@ -660,8 +661,25 @@ static int pca953x_probe(struct i2c_client *client,
 		invert = pdata->invert;
 		chip->names = pdata->names;
 	} else {
+		struct gpio_desc *reset;
+
 		chip->gpio_start = -1;
 		irq_base = 0;
+
+		reset = devm_gpiod_get(&client->dev, "reset");
+		if (IS_ERR(reset)) {
+			if (PTR_ERR(reset) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			else
+				dev_info(&client->dev, "Did not find/get a gpio for reset (%ld)\n",
+						PTR_ERR(reset));
+		} else {
+			/* Reset the chip if the reset is wired */
+			gpiod_direction_output(reset, 0);
+			udelay(100);
+			gpiod_set_value(reset, 1);
+			udelay(100);
+		}
 	}
 
 	chip->client = client;
-- 
2.0.1

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

* Re: [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible
  2014-07-29  7:24   ` Markus Pargmann
@ 2014-07-30 12:52     ` Markus Pargmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-30 12:52 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel

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

On Tue, Jul 29, 2014 at 09:24:45AM +0200, Markus Pargmann wrote:
> This driver can also be used for the nxp,pca9506 device.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
>  drivers/gpio/gpio-pca953x.c                             | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> index b9a42f294dd0..eb65157d47f6 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> @@ -3,6 +3,7 @@
>  Required properties:
>   - compatible: Has to contain one of the following:
>  	nxp,pca9505
> +	nxp,pca9506
>  	nxp,pca9534
>  	nxp,pca9535
>  	nxp,pca9536
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index fda3eae835d0..df5eb6e6be1e 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -729,6 +729,7 @@ static int pca953x_remove(struct i2c_client *client)
>  
>  static const struct of_device_id pca953x_dt_ids[] = {
>  	{ .compatible = "nxp,pca9505", },
> +	{ .compatible = "nxp,pca9506", },

The item for the i2c_device_id table is missing in this patch. I will
send a v2.

Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible
@ 2014-07-30 12:52     ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-30 12:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 09:24:45AM +0200, Markus Pargmann wrote:
> This driver can also be used for the nxp,pca9506 device.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
>  drivers/gpio/gpio-pca953x.c                             | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> index b9a42f294dd0..eb65157d47f6 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> @@ -3,6 +3,7 @@
>  Required properties:
>   - compatible: Has to contain one of the following:
>  	nxp,pca9505
> +	nxp,pca9506
>  	nxp,pca9534
>  	nxp,pca9535
>  	nxp,pca9536
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index fda3eae835d0..df5eb6e6be1e 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -729,6 +729,7 @@ static int pca953x_remove(struct i2c_client *client)
>  
>  static const struct of_device_id pca953x_dt_ids[] = {
>  	{ .compatible = "nxp,pca9505", },
> +	{ .compatible = "nxp,pca9506", },

The item for the i2c_device_id table is missing in this patch. I will
send a v2.

Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
-------------- 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/20140730/1c493698/attachment.sig>

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

* Re: [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible
  2014-07-30 12:52     ` Markus Pargmann
@ 2014-07-30 13:09       ` Markus Pargmann
  -1 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-30 13:09 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, kernel

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

On Wed, Jul 30, 2014 at 02:52:16PM +0200, Markus Pargmann wrote:
> On Tue, Jul 29, 2014 at 09:24:45AM +0200, Markus Pargmann wrote:
> > This driver can also be used for the nxp,pca9506 device.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
> >  drivers/gpio/gpio-pca953x.c                             | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> > index b9a42f294dd0..eb65157d47f6 100644
> > --- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> > @@ -3,6 +3,7 @@
> >  Required properties:
> >   - compatible: Has to contain one of the following:
> >  	nxp,pca9505
> > +	nxp,pca9506
> >  	nxp,pca9534
> >  	nxp,pca9535
> >  	nxp,pca9536
> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> > index fda3eae835d0..df5eb6e6be1e 100644
> > --- a/drivers/gpio/gpio-pca953x.c
> > +++ b/drivers/gpio/gpio-pca953x.c
> > @@ -729,6 +729,7 @@ static int pca953x_remove(struct i2c_client *client)
> >  
> >  static const struct of_device_id pca953x_dt_ids[] = {
> >  	{ .compatible = "nxp,pca9505", },
> > +	{ .compatible = "nxp,pca9506", },
> 
> The item for the i2c_device_id table is missing in this patch. I will
> send a v2.

This patch can actually be dropped. There is no difference between
pca9505 and pca9506 so it doesn't make sense to add pca9506.

Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible
@ 2014-07-30 13:09       ` Markus Pargmann
  0 siblings, 0 replies; 28+ messages in thread
From: Markus Pargmann @ 2014-07-30 13:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 30, 2014 at 02:52:16PM +0200, Markus Pargmann wrote:
> On Tue, Jul 29, 2014 at 09:24:45AM +0200, Markus Pargmann wrote:
> > This driver can also be used for the nxp,pca9506 device.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 1 +
> >  drivers/gpio/gpio-pca953x.c                             | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> > index b9a42f294dd0..eb65157d47f6 100644
> > --- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> > @@ -3,6 +3,7 @@
> >  Required properties:
> >   - compatible: Has to contain one of the following:
> >  	nxp,pca9505
> > +	nxp,pca9506
> >  	nxp,pca9534
> >  	nxp,pca9535
> >  	nxp,pca9536
> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> > index fda3eae835d0..df5eb6e6be1e 100644
> > --- a/drivers/gpio/gpio-pca953x.c
> > +++ b/drivers/gpio/gpio-pca953x.c
> > @@ -729,6 +729,7 @@ static int pca953x_remove(struct i2c_client *client)
> >  
> >  static const struct of_device_id pca953x_dt_ids[] = {
> >  	{ .compatible = "nxp,pca9505", },
> > +	{ .compatible = "nxp,pca9506", },
> 
> The item for the i2c_device_id table is missing in this patch. I will
> send a v2.

This patch can actually be dropped. There is no difference between
pca9505 and pca9506 so it doesn't make sense to add pca9506.

Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
-------------- 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/20140730/2b146029/attachment-0001.sig>

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

* Re: [PATCH 1/4] gpio: pca953x: Drop deprecated DT bindings
  2014-07-29  7:24   ` Markus Pargmann
@ 2014-08-08 13:07     ` Linus Walleij
  -1 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-08 13:07 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, Sascha Hauer

On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> Drop deprecated DT bindings and use automaticly assigned gpio and irq
> bases.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Patch applied.

> -       val = of_get_property(node, "linux,gpio-base", &size);
> -       WARN(val, "%s: device-tree property 'linux,gpio-base' is deprecated!", __func__);

Gah! This should never have been merged.

I even consider this patch a bug fix now.

Yours,
Linus Walleij

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

* [PATCH 1/4] gpio: pca953x: Drop deprecated DT bindings
@ 2014-08-08 13:07     ` Linus Walleij
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-08 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> Drop deprecated DT bindings and use automaticly assigned gpio and irq
> bases.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Patch applied.

> -       val = of_get_property(node, "linux,gpio-base", &size);
> -       WARN(val, "%s: device-tree property 'linux,gpio-base' is deprecated!", __func__);

Gah! This should never have been merged.

I even consider this patch a bug fix now.

Yours,
Linus Walleij

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

* Re: [PATCH 2/4] gpio: pca953x: Add DT binding documentation
  2014-07-29  7:24   ` Markus Pargmann
@ 2014-08-08 13:07     ` Linus Walleij
  -1 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-08 13:07 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, Sascha Hauer

On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> Add a devicetree binding documentation for the pca953x driver.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Patch applied.

Yours,
Linus Walleij

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

* [PATCH 2/4] gpio: pca953x: Add DT binding documentation
@ 2014-08-08 13:07     ` Linus Walleij
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-08 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> Add a devicetree binding documentation for the pca953x driver.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Patch applied.

Yours,
Linus Walleij

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

* Re: [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
  2014-07-29  7:24   ` Markus Pargmann
@ 2014-08-08 13:14     ` Linus Walleij
  -1 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-08 13:14 UTC (permalink / raw)
  To: Markus Pargmann, Houcheng Lin
  Cc: Alexandre Courbot, Toby Smith, Aaron Sierra, linux-gpio,
	linux-arm-kernel, Sascha Hauer

On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> The pca953x has a negated reset input. This patch adds a DT binding for
> the reset gpio and resets the chip when it is probed. This will reset
> the device and leave the gpio in the correct state so reset is not
> triggered.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Why on earth should this be in the GPIO driver?

The driver should be in drivers/reset/reset-gpio.c and you
should provide a separate driver for it.

As it happens, Houcheng Lin has already proposed such a
driver:
http://marc.info/?l=linux-kernel&m=140309916607115&w=2

Please coordinate with Houcheng and use his driver for what
you want to achieve.

Yours,
Linus Walleij

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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
@ 2014-08-08 13:14     ` Linus Walleij
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-08 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> The pca953x has a negated reset input. This patch adds a DT binding for
> the reset gpio and resets the chip when it is probed. This will reset
> the device and leave the gpio in the correct state so reset is not
> triggered.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

Why on earth should this be in the GPIO driver?

The driver should be in drivers/reset/reset-gpio.c and you
should provide a separate driver for it.

As it happens, Houcheng Lin has already proposed such a
driver:
http://marc.info/?l=linux-kernel&m=140309916607115&w=2

Please coordinate with Houcheng and use his driver for what
you want to achieve.

Yours,
Linus Walleij

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

* Re: [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
  2014-08-08 13:14     ` Linus Walleij
@ 2014-08-08 14:11       ` Philipp Zabel
  -1 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2014-08-08 14:11 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Markus Pargmann, Houcheng Lin, Alexandre Courbot, Toby Smith,
	Aaron Sierra, linux-gpio, linux-arm-kernel, Sascha Hauer

Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:
> On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
> 
> > The pca953x has a negated reset input. This patch adds a DT binding for
> > the reset gpio and resets the chip when it is probed. This will reset
> > the device and leave the gpio in the correct state so reset is not
> > triggered.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> 
> Why on earth should this be in the GPIO driver?
>
> The driver should be in drivers/reset/reset-gpio.c and you
> should provide a separate driver for it.

I still think we should keep using the reset-gpios binding for simple
cases like this; I see no reason to add a separate device to the device
tree for a single GPIO.

> As it happens, Houcheng Lin has already proposed such a
> driver:
> http://marc.info/?l=linux-kernel&m=140309916607115&w=2

That is a different issue, as there the device does not appear on the
bus until the reset is released.

Here the I2C device will be probed from the device tree, so the reset
can be released or triggered from the probe function.

regards
Philipp


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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
@ 2014-08-08 14:11       ` Philipp Zabel
  0 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2014-08-08 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:
> On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
> 
> > The pca953x has a negated reset input. This patch adds a DT binding for
> > the reset gpio and resets the chip when it is probed. This will reset
> > the device and leave the gpio in the correct state so reset is not
> > triggered.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> 
> Why on earth should this be in the GPIO driver?
>
> The driver should be in drivers/reset/reset-gpio.c and you
> should provide a separate driver for it.

I still think we should keep using the reset-gpios binding for simple
cases like this; I see no reason to add a separate device to the device
tree for a single GPIO.

> As it happens, Houcheng Lin has already proposed such a
> driver:
> http://marc.info/?l=linux-kernel&m=140309916607115&w=2

That is a different issue, as there the device does not appear on the
bus until the reset is released.

Here the I2C device will be probed from the device tree, so the reset
can be released or triggered from the probe function.

regards
Philipp

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

* Re: [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
  2014-08-08 14:11       ` Philipp Zabel
@ 2014-08-11  8:43         ` Linus Walleij
  -1 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-11  8:43 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Markus Pargmann, Houcheng Lin, Alexandre Courbot, Toby Smith,
	Aaron Sierra, linux-gpio, linux-arm-kernel, Sascha Hauer

On Fri, Aug 8, 2014 at 4:11 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:
>> On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
>>
>> > The pca953x has a negated reset input. This patch adds a DT binding for
>> > the reset gpio and resets the chip when it is probed. This will reset
>> > the device and leave the gpio in the correct state so reset is not
>> > triggered.
>> >
>> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
>>
>> Why on earth should this be in the GPIO driver?
>>
>> The driver should be in drivers/reset/reset-gpio.c and you
>> should provide a separate driver for it.
>
> I still think we should keep using the reset-gpios binding for simple
> cases like this; I see no reason to add a separate device to the device
> tree for a single GPIO.

In any case you have to propose something generic that
happens in drivers/gpio/gpiolib.c at the end of the
gpiochip_add() function, because this will likely appear in
many other systems.

The binding should also be generic in
Documentation/devicetree/bindings/gpio/gpio.txt
not for just this driver.

>> As it happens, Houcheng Lin has already proposed such a
>> driver:
>> http://marc.info/?l=linux-kernel&m=140309916607115&w=2
>
> That is a different issue, as there the device does not appear on the
> bus until the reset is released.

You're just looking at the patch description. Look at what the
driver does.

I wrote this reply to the patch:
http://marc.info/?l=linux-kernel&m=140480593524472&w=2

With a delay of zero, the reset will be released
immediately, by the use of a helper OF node and this driver
from the reset subsystem. It's nice, generic code that solves
a generic problem of deasserting GPIO lines for
some reset.

Yours,
Linus Walleij

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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
@ 2014-08-11  8:43         ` Linus Walleij
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-11  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Aug 8, 2014 at 4:11 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:
>> On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
>>
>> > The pca953x has a negated reset input. This patch adds a DT binding for
>> > the reset gpio and resets the chip when it is probed. This will reset
>> > the device and leave the gpio in the correct state so reset is not
>> > triggered.
>> >
>> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
>>
>> Why on earth should this be in the GPIO driver?
>>
>> The driver should be in drivers/reset/reset-gpio.c and you
>> should provide a separate driver for it.
>
> I still think we should keep using the reset-gpios binding for simple
> cases like this; I see no reason to add a separate device to the device
> tree for a single GPIO.

In any case you have to propose something generic that
happens in drivers/gpio/gpiolib.c at the end of the
gpiochip_add() function, because this will likely appear in
many other systems.

The binding should also be generic in
Documentation/devicetree/bindings/gpio/gpio.txt
not for just this driver.

>> As it happens, Houcheng Lin has already proposed such a
>> driver:
>> http://marc.info/?l=linux-kernel&m=140309916607115&w=2
>
> That is a different issue, as there the device does not appear on the
> bus until the reset is released.

You're just looking at the patch description. Look at what the
driver does.

I wrote this reply to the patch:
http://marc.info/?l=linux-kernel&m=140480593524472&w=2

With a delay of zero, the reset will be released
immediately, by the use of a helper OF node and this driver
from the reset subsystem. It's nice, generic code that solves
a generic problem of deasserting GPIO lines for
some reset.

Yours,
Linus Walleij

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

* Re: [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
  2014-08-11  8:43         ` Linus Walleij
@ 2014-08-14  9:21           ` Philipp Zabel
  -1 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2014-08-14  9:21 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Markus Pargmann, Houcheng Lin, Alexandre Courbot, Toby Smith,
	Aaron Sierra, linux-gpio, linux-arm-kernel, Sascha Hauer

Hi Linus,

Am Montag, den 11.08.2014, 10:43 +0200 schrieb Linus Walleij:
> On Fri, Aug 8, 2014 at 4:11 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:
> >> On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
> >>
> >> > The pca953x has a negated reset input. This patch adds a DT binding for
> >> > the reset gpio and resets the chip when it is probed. This will reset
> >> > the device and leave the gpio in the correct state so reset is not
> >> > triggered.
> >> >
> >> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> >>
> >> Why on earth should this be in the GPIO driver?
> >>
> >> The driver should be in drivers/reset/reset-gpio.c and you
> >> should provide a separate driver for it.
> >
> > I still think we should keep using the reset-gpios binding for simple
> > cases like this; I see no reason to add a separate device to the device
> > tree for a single GPIO.
> 
> In any case you have to propose something generic that
> happens in drivers/gpio/gpiolib.c at the end of the
> gpiochip_add() function, because this will likely appear in
> many other systems.

In general, I'd like to keep drivers in control over how and when the
reset is asserted; mainly because there are variations of reset,
powerdown/enable, and bootstrap pins that have to be taken into account.

On some devices the powerdown needs to be deasserted before the reset
can work, on some devices it might be the other way around, or the
powerdown pin may cause a reset itself, or there are bootstrap pins that
need to be configured a certain way while the reset is issued (but are
used for something else while the device is active). Others occasionally
need to reset the chip while in operation.

If you expect none of those issues for GPIO chips, I don't argue against
doing this for the drivers in gpiochip_add, if it is documented that
this function might reset the chip.
This would work with a separate gpio reset provider driver by just
calling device_reset(chip->dev) at the end of gpiochip_add.
With my previous suggestion, as Maxime points out, I'd still need to
find a way to provide the duration of the reset pulse.

> The binding should also be generic in
> Documentation/devicetree/bindings/gpio/gpio.txt
> not for just this driver.

Ideally it should be the same as all other devices with an external
reset pin.

> >> As it happens, Houcheng Lin has already proposed such a
> >> driver:
> >> http://marc.info/?l=linux-kernel&m=140309916607115&w=2
> >
> > That is a different issue, as there the device does not appear on the
> > bus until the reset is released.
> 
> You're just looking at the patch description. Look at what the
> driver does.
> 
> I wrote this reply to the patch:
> http://marc.info/?l=linux-kernel&m=140480593524472&w=2
> 
> With a delay of zero, the reset will be released
> immediately, by the use of a helper OF node and this driver
> from the reset subsystem. It's nice, generic code that solves
> a generic problem of deasserting GPIO lines for
> some reset.

Yes, it does what you want. But its purpose is different, it's a bit
indirect for the use case at hand, and we'll still find cases where this
won't work (interaction with other pins). As to whether the sub-node
might conflict with existing bindings, I don't know. This is something
that should be taken to devicetree discussion list.

regards
Philipp


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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
@ 2014-08-14  9:21           ` Philipp Zabel
  0 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2014-08-14  9:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Linus,

Am Montag, den 11.08.2014, 10:43 +0200 schrieb Linus Walleij:
> On Fri, Aug 8, 2014 at 4:11 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:
> >> On Tue, Jul 29, 2014 at 9:24 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
> >>
> >> > The pca953x has a negated reset input. This patch adds a DT binding for
> >> > the reset gpio and resets the chip when it is probed. This will reset
> >> > the device and leave the gpio in the correct state so reset is not
> >> > triggered.
> >> >
> >> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> >>
> >> Why on earth should this be in the GPIO driver?
> >>
> >> The driver should be in drivers/reset/reset-gpio.c and you
> >> should provide a separate driver for it.
> >
> > I still think we should keep using the reset-gpios binding for simple
> > cases like this; I see no reason to add a separate device to the device
> > tree for a single GPIO.
> 
> In any case you have to propose something generic that
> happens in drivers/gpio/gpiolib.c at the end of the
> gpiochip_add() function, because this will likely appear in
> many other systems.

In general, I'd like to keep drivers in control over how and when the
reset is asserted; mainly because there are variations of reset,
powerdown/enable, and bootstrap pins that have to be taken into account.

On some devices the powerdown needs to be deasserted before the reset
can work, on some devices it might be the other way around, or the
powerdown pin may cause a reset itself, or there are bootstrap pins that
need to be configured a certain way while the reset is issued (but are
used for something else while the device is active). Others occasionally
need to reset the chip while in operation.

If you expect none of those issues for GPIO chips, I don't argue against
doing this for the drivers in gpiochip_add, if it is documented that
this function might reset the chip.
This would work with a separate gpio reset provider driver by just
calling device_reset(chip->dev) at the end of gpiochip_add.
With my previous suggestion, as Maxime points out, I'd still need to
find a way to provide the duration of the reset pulse.

> The binding should also be generic in
> Documentation/devicetree/bindings/gpio/gpio.txt
> not for just this driver.

Ideally it should be the same as all other devices with an external
reset pin.

> >> As it happens, Houcheng Lin has already proposed such a
> >> driver:
> >> http://marc.info/?l=linux-kernel&m=140309916607115&w=2
> >
> > That is a different issue, as there the device does not appear on the
> > bus until the reset is released.
> 
> You're just looking at the patch description. Look at what the
> driver does.
> 
> I wrote this reply to the patch:
> http://marc.info/?l=linux-kernel&m=140480593524472&w=2
> 
> With a delay of zero, the reset will be released
> immediately, by the use of a helper OF node and this driver
> from the reset subsystem. It's nice, generic code that solves
> a generic problem of deasserting GPIO lines for
> some reset.

Yes, it does what you want. But its purpose is different, it's a bit
indirect for the use case at hand, and we'll still find cases where this
won't work (interaction with other pins). As to whether the sub-node
might conflict with existing bindings, I don't know. This is something
that should be taken to devicetree discussion list.

regards
Philipp

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

* Re: [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
  2014-08-14  9:21           ` Philipp Zabel
@ 2014-08-29  6:27             ` Linus Walleij
  -1 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-29  6:27 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Markus Pargmann, Houcheng Lin, Alexandre Courbot, Toby Smith,
	Aaron Sierra, linux-gpio, linux-arm-kernel, Sascha Hauer

On Thu, Aug 14, 2014 at 11:21 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Montag, den 11.08.2014, 10:43 +0200 schrieb Linus Walleij:
>> On Fri, Aug 8, 2014 at 4:11 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> > Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:

>> >> As it happens, Houcheng Lin has already proposed such a
>> >> driver:
>> >> http://marc.info/?l=linux-kernel&m=140309916607115&w=2
>> >
>> > That is a different issue, as there the device does not appear on the
>> > bus until the reset is released.
>>
>> You're just looking at the patch description. Look at what the
>> driver does.
>>
>> I wrote this reply to the patch:
>> http://marc.info/?l=linux-kernel&m=140480593524472&w=2
>>
>> With a delay of zero, the reset will be released
>> immediately, by the use of a helper OF node and this driver
>> from the reset subsystem. It's nice, generic code that solves
>> a generic problem of deasserting GPIO lines for
>> some reset.
>
> Yes, it does what you want. But its purpose is different, it's a bit
> indirect for the use case at hand,

I think a generic driver is always best, define what you mean
by "indirect" in this context.

> and we'll still find cases where this
> won't work (interaction with other pins).

But that is not what you're trying to solve right now.

> As to whether the sub-node
> might conflict with existing bindings, I don't know. This is something
> that should be taken to devicetree discussion list.

Um? I don't get this.

Yours,
Linus Walleij

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

* [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio
@ 2014-08-29  6:27             ` Linus Walleij
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2014-08-29  6:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 14, 2014 at 11:21 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Montag, den 11.08.2014, 10:43 +0200 schrieb Linus Walleij:
>> On Fri, Aug 8, 2014 at 4:11 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> > Am Freitag, den 08.08.2014, 15:14 +0200 schrieb Linus Walleij:

>> >> As it happens, Houcheng Lin has already proposed such a
>> >> driver:
>> >> http://marc.info/?l=linux-kernel&m=140309916607115&w=2
>> >
>> > That is a different issue, as there the device does not appear on the
>> > bus until the reset is released.
>>
>> You're just looking at the patch description. Look at what the
>> driver does.
>>
>> I wrote this reply to the patch:
>> http://marc.info/?l=linux-kernel&m=140480593524472&w=2
>>
>> With a delay of zero, the reset will be released
>> immediately, by the use of a helper OF node and this driver
>> from the reset subsystem. It's nice, generic code that solves
>> a generic problem of deasserting GPIO lines for
>> some reset.
>
> Yes, it does what you want. But its purpose is different, it's a bit
> indirect for the use case at hand,

I think a generic driver is always best, define what you mean
by "indirect" in this context.

> and we'll still find cases where this
> won't work (interaction with other pins).

But that is not what you're trying to solve right now.

> As to whether the sub-node
> might conflict with existing bindings, I don't know. This is something
> that should be taken to devicetree discussion list.

Um? I don't get this.

Yours,
Linus Walleij

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

end of thread, other threads:[~2014-08-29  6:27 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-29  7:24 [PATCH 0/4] gpio: pca953x updates for DT Markus Pargmann
2014-07-29  7:24 ` Markus Pargmann
2014-07-29  7:24 ` [PATCH 1/4] gpio: pca953x: Drop deprecated DT bindings Markus Pargmann
2014-07-29  7:24   ` Markus Pargmann
2014-08-08 13:07   ` Linus Walleij
2014-08-08 13:07     ` Linus Walleij
2014-07-29  7:24 ` [PATCH 2/4] gpio: pca953x: Add DT binding documentation Markus Pargmann
2014-07-29  7:24   ` Markus Pargmann
2014-08-08 13:07   ` Linus Walleij
2014-08-08 13:07     ` Linus Walleij
2014-07-29  7:24 ` [PATCH 3/4] gpio: pca953x: Add pca9506 as DT compatible Markus Pargmann
2014-07-29  7:24   ` Markus Pargmann
2014-07-30 12:52   ` Markus Pargmann
2014-07-30 12:52     ` Markus Pargmann
2014-07-30 13:09     ` Markus Pargmann
2014-07-30 13:09       ` Markus Pargmann
2014-07-29  7:24 ` [PATCH 4/4] gpio: pca953x: Add DT binding for reset gpio Markus Pargmann
2014-07-29  7:24   ` Markus Pargmann
2014-08-08 13:14   ` Linus Walleij
2014-08-08 13:14     ` Linus Walleij
2014-08-08 14:11     ` Philipp Zabel
2014-08-08 14:11       ` Philipp Zabel
2014-08-11  8:43       ` Linus Walleij
2014-08-11  8:43         ` Linus Walleij
2014-08-14  9:21         ` Philipp Zabel
2014-08-14  9:21           ` Philipp Zabel
2014-08-29  6:27           ` Linus Walleij
2014-08-29  6:27             ` Linus Walleij

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