[PATCH v2 1/6] GPIO: xilinx: Simplify driver probe function

[PATCH v2 1/6] GPIO: xilinx: Simplify driver probe function

[Michal Simek]

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

Simplification is done by using OF helper function
which increase readability of code and remove
(if (var) var = be32_to_cpup;) assignment.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v2:
- New patch in this series

 drivers/gpio/gpio-xilinx.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index 9ae7aa8..2aad534 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -170,24 +170,20 @@ static int xgpio_of_probe(struct device_node *np)
 		return -ENOMEM;

 	/* Update GPIO state shadow register with default value */
-	tree_info = of_get_property(np, "xlnx,dout-default", NULL);
-	if (tree_info)
-		chip->gpio_state = be32_to_cpup(tree_info);
+	of_property_read_u32(np, "xlnx,dout-default", &chip->gpio_state);
+
+	/* By default, all pins are inputs */
+	chip->gpio_dir = 0xFFFFFFFF;

 	/* Update GPIO direction shadow register with default value */
-	chip->gpio_dir = 0xFFFFFFFF; /* By default, all pins are inputs */
-	tree_info = of_get_property(np, "xlnx,tri-default", NULL);
-	if (tree_info)
-		chip->gpio_dir = be32_to_cpup(tree_info);
+	of_property_read_u32(np, "xlnx,tri-default", &chip->gpio_dir);
+
+	/* By default assume full GPIO controller */
+	chip->mmchip.gc.ngpio = 32;

 	/* Check device node and parent device node for device width */
-	chip->mmchip.gc.ngpio = 32; /* By default assume full GPIO controller */
-	tree_info = of_get_property(np, "xlnx,gpio-width", NULL);
-	if (!tree_info)
-		tree_info = of_get_property(np->parent,
-					    "xlnx,gpio-width", NULL);
-	if (tree_info)
-		chip->mmchip.gc.ngpio = be32_to_cpup(tree_info);
+	of_property_read_u32(np, "xlnx,gpio-width",
+			      (u32 *)&chip->mmchip.gc.ngpio);

 	spin_lock_init(&chip->gpio_lock);

--
1.8.2.3


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

[PATCH v2 2/6] GPIO: xilinx: Add support for dual channel

[Michal Simek]

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

Supporting the second channel in the driver.
Offset is 0x8 and both channnels share the same
IRQ.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>

---
Changes in v2:
- Use kernel doc format - suggested by Linus Walleij
- Do not use __raw_readl/__raw_writel IO in this patch
- Use of_property_read_u32 helper function
- Use BIT()
- Change patch subject

 drivers/gpio/gpio-xilinx.c | 103 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 91 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
index 2aad534..626eaa8 100644
--- a/drivers/gpio/gpio-xilinx.c
+++ b/drivers/gpio/gpio-xilinx.c
@@ -1,7 +1,7 @@
 /*
- * Xilinx gpio driver
+ * Xilinx gpio driver for xps/axi_gpio IP.
  *
- * Copyright 2008 Xilinx, Inc.
+ * Copyright 2008 - 2013 Xilinx, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2
@@ -12,6 +12,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */

+#include <linux/bitops.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/module.h>
@@ -26,11 +27,26 @@
 #define XGPIO_DATA_OFFSET   (0x0)	/* Data register  */
 #define XGPIO_TRI_OFFSET    (0x4)	/* I/O direction register  */

+#define XGPIO_CHANNEL_OFFSET	0x8
+
+/* Read/Write access to the GPIO registers */
+#define xgpio_readreg(offset)		in_be32(offset)
+#define xgpio_writereg(offset, val)	out_be32(offset, val)
+
+/**
+ * struct xgpio_instance - Stores information about GPIO device
+ * struct of_mm_gpio_chip mmchip: OF GPIO chip for memory mapped banks
+ * gpio_state: GPIO state shadow register
+ * gpio_dir: GPIO direction shadow register
+ * offset: GPIO channel offset
+ * gpio_lock: Lock used for synchronization
+ */
 struct xgpio_instance {
 	struct of_mm_gpio_chip mmchip;
-	u32 gpio_state;		/* GPIO state shadow register */
-	u32 gpio_dir;		/* GPIO direction shadow register */
-	spinlock_t gpio_lock;	/* Lock used for synchronization */
+	u32 gpio_state;
+	u32 gpio_dir;
+	u32 offset;
+	spinlock_t gpio_lock;
 };

 /**
@@ -44,8 +60,12 @@ struct xgpio_instance {
 static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct xgpio_instance *chip =
+	    container_of(mm_gc, struct xgpio_instance, mmchip);

-	return (in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) >> gpio) & 1;
+	void __iomem *regs = mm_gc->regs + chip->offset;
+
+	return !!(xgpio_readreg(regs + XGPIO_DATA_OFFSET) & BIT(gpio));
 }

 /**
@@ -63,6 +83,7 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	void __iomem *regs = mm_gc->regs;

 	spin_lock_irqsave(&chip->gpio_lock, flags);

@@ -71,7 +92,9 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 		chip->gpio_state |= 1 << gpio;
 	else
 		chip->gpio_state &= ~(1 << gpio);
-	out_be32(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
+
+	xgpio_writereg(regs + chip->offset + XGPIO_DATA_OFFSET,
+							 chip->gpio_state);

 	spin_unlock_irqrestore(&chip->gpio_lock, flags);
 }
@@ -91,12 +114,13 @@ static int xgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	void __iomem *regs = mm_gc->regs;

 	spin_lock_irqsave(&chip->gpio_lock, flags);

 	/* Set the GPIO bit in shadow register and set direction as input */
 	chip->gpio_dir |= (1 << gpio);
-	out_be32(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	xgpio_writereg(regs + chip->offset + XGPIO_TRI_OFFSET, chip->gpio_dir);

 	spin_unlock_irqrestore(&chip->gpio_lock, flags);

@@ -119,6 +143,7 @@ static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);
+	void __iomem *regs = mm_gc->regs;

 	spin_lock_irqsave(&chip->gpio_lock, flags);

@@ -127,11 +152,12 @@ static int xgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 		chip->gpio_state |= 1 << gpio;
 	else
 		chip->gpio_state &= ~(1 << gpio);
-	out_be32(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
+	xgpio_writereg(regs + chip->offset + XGPIO_DATA_OFFSET,
+		       chip->gpio_state);

 	/* Clear the GPIO bit in shadow register and set direction as output */
 	chip->gpio_dir &= (~(1 << gpio));
-	out_be32(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	xgpio_writereg(regs + chip->offset + XGPIO_TRI_OFFSET, chip->gpio_dir);

 	spin_unlock_irqrestore(&chip->gpio_lock, flags);

@@ -147,8 +173,10 @@ static void xgpio_save_regs(struct of_mm_gpio_chip *mm_gc)
 	struct xgpio_instance *chip =
 	    container_of(mm_gc, struct xgpio_instance, mmchip);

-	out_be32(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
-	out_be32(mm_gc->regs + XGPIO_TRI_OFFSET, chip->gpio_dir);
+	xgpio_writereg(mm_gc->regs + chip->offset + XGPIO_DATA_OFFSET,
+							chip->gpio_state);
+	xgpio_writereg(mm_gc->regs + chip->offset + XGPIO_TRI_OFFSET,
+							 chip->gpio_dir);
 }

 /**
@@ -202,6 +230,57 @@ static int xgpio_of_probe(struct device_node *np)
 		       np->full_name, status);
 		return status;
 	}
+
+	pr_info("XGpio: %s: registered, base is %d\n", np->full_name,
+							chip->mmchip.gc.base);
+
+	tree_info = of_get_property(np, "xlnx,is-dual", NULL);
+	if (tree_info && be32_to_cpup(tree_info)) {
+		chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+		if (!chip)
+			return -ENOMEM;
+
+		/* Add dual channel offset */
+		chip->offset = XGPIO_CHANNEL_OFFSET;
+
+		/* Update GPIO state shadow register with default value */
+		of_property_read_u32(np, "xlnx,dout-default-2",
+				     &chip->gpio_state);
+
+		/* By default, all pins are inputs */
+		chip->gpio_dir = 0xFFFFFFFF;
+
+		/* Update GPIO direction shadow register with default value */
+		of_property_read_u32(np, "xlnx,tri-default-2", &chip->gpio_dir);
+
+		/* By default assume full GPIO controller */
+		chip->mmchip.gc.ngpio = 32;
+
+		/* Check device node and parent device node for device width */
+		of_property_read_u32(np, "xlnx,gpio2-width",
+				     (u32 *)&chip->mmchip.gc.ngpio);
+
+		spin_lock_init(&chip->gpio_lock);
+
+		chip->mmchip.gc.direction_input = xgpio_dir_in;
+		chip->mmchip.gc.direction_output = xgpio_dir_out;
+		chip->mmchip.gc.get = xgpio_get;
+		chip->mmchip.gc.set = xgpio_set;
+
+		chip->mmchip.save_regs = xgpio_save_regs;
+
+		/* Call the OF gpio helper to setup and register the GPIO dev */
+		status = of_mm_gpiochip_add(np, &chip->mmchip);
+		if (status) {
+			kfree(chip);
+			pr_err("%s: error in probe function with status %d\n",
+			np->full_name, status);
+			return status;
+		}
+		pr_info("XGpio: %s: dual channel registered, base is %d\n",
+					np->full_name, chip->mmchip.gc.base);
+	}
+
 	return 0;
 }

--
1.8.2.3


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

[PATCH v2 6/6] DT: Add documentation for gpio-xilinx

[Michal Simek]

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

Describe gpio-xilinx binding.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v2:
- Extend description

 .../devicetree/bindings/gpio/gpio-xilinx.txt       | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xilinx.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt b/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt
new file mode 100644
index 0000000..63bf4be
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xilinx.txt
@@ -0,0 +1,48 @@
+Xilinx plb/axi GPIO controller
+
+Dual channel GPIO controller with configurable number of pins
+(from 1 to 32 per channel). Every pin can be configured as
+input/output/tristate. Both channels share the same global IRQ but
+local interrupts can be enabled on channel basis.
+
+Required properties:
+- compatible : Should be "xlnx,xps-gpio-1.00.a"
+- reg : Address and length of the register set for the device
+- #gpio-cells : Should be two. The first cell is the pin number and the
+  second cell is used to specify optional parameters (currently unused).
+- gpio-controller : Marks the device node as a GPIO controller.
+
+Optional properties:
+- interrupts : Interrupt mapping for GPIO IRQ.
+- interrupt-parent : Phandle for the interrupt controller that
+  services interrupts for this device.
+- xlnx,all-inputs : if n-th bit is setup, GPIO-n is input
+- xlnx,dout-default : if n-th bit is 1, GPIO-n default value is 1
+- xlnx,gpio-width : gpio width
+- xlnx,tri-default : if n-th bit is 1, GPIO-n is in tristate mode
+- xlnx,is-dual : if 1, controller also uses the second channel
+- xlnx,all-inputs-2 : as above but for the second channel
+- xlnx,dout-default-2 : as above but the second channel
+- xlnx,gpio2-width : as above but for the second channel
+- xlnx,tri-default-2 : as above but for the second channel
+
+
+Example:
+gpio: gpio@40000000 {
+	#gpio-cells = <2>;
+	compatible = "xlnx,xps-gpio-1.00.a";
+	gpio-controller ;
+	interrupt-parent = <&microblaze_0_intc>;
+	interrupts = < 6 2 >;
+	reg = < 0x40000000 0x10000 >;
+	xlnx,all-inputs = <0x0>;
+	xlnx,all-inputs-2 = <0x0>;
+	xlnx,dout-default = <0x0>;
+	xlnx,dout-default-2 = <0x0>;
+	xlnx,gpio-width = <0x2>;
+	xlnx,gpio2-width = <0x2>;
+	xlnx,interrupt-present = <0x1>;
+	xlnx,is-dual = <0x1>;
+	xlnx,tri-default = <0xffffffff>;
+	xlnx,tri-default-2 = <0xffffffff>;
+} ;
--
1.8.2.3


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

Re: [PATCH v2 1/6] GPIO: xilinx: Simplify driver probe function

[Michal Simek]

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

Hi Linus,

can you please look at this patchset?

Thanks,
Michal

On 06/03/2013 02:31 PM, Michal Simek wrote:
> Simplification is done by using OF helper function
> which increase readability of code and remove
> (if (var) var = be32_to_cpup;) assignment.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> Changes in v2:
> - New patch in this series
> 
>  drivers/gpio/gpio-xilinx.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index 9ae7aa8..2aad534 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -170,24 +170,20 @@ static int xgpio_of_probe(struct device_node *np)
>  		return -ENOMEM;
> 
>  	/* Update GPIO state shadow register with default value */
> -	tree_info = of_get_property(np, "xlnx,dout-default", NULL);
> -	if (tree_info)
> -		chip->gpio_state = be32_to_cpup(tree_info);
> +	of_property_read_u32(np, "xlnx,dout-default", &chip->gpio_state);
> +
> +	/* By default, all pins are inputs */
> +	chip->gpio_dir = 0xFFFFFFFF;
> 
>  	/* Update GPIO direction shadow register with default value */
> -	chip->gpio_dir = 0xFFFFFFFF; /* By default, all pins are inputs */
> -	tree_info = of_get_property(np, "xlnx,tri-default", NULL);
> -	if (tree_info)
> -		chip->gpio_dir = be32_to_cpup(tree_info);
> +	of_property_read_u32(np, "xlnx,tri-default", &chip->gpio_dir);
> +
> +	/* By default assume full GPIO controller */
> +	chip->mmchip.gc.ngpio = 32;
> 
>  	/* Check device node and parent device node for device width */
> -	chip->mmchip.gc.ngpio = 32; /* By default assume full GPIO controller */
> -	tree_info = of_get_property(np, "xlnx,gpio-width", NULL);
> -	if (!tree_info)
> -		tree_info = of_get_property(np->parent,
> -					    "xlnx,gpio-width", NULL);
> -	if (tree_info)
> -		chip->mmchip.gc.ngpio = be32_to_cpup(tree_info);
> +	of_property_read_u32(np, "xlnx,gpio-width",
> +			      (u32 *)&chip->mmchip.gc.ngpio);
> 
>  	spin_lock_init(&chip->gpio_lock);
> 
> --
> 1.8.2.3
> 


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 1/6] GPIO: xilinx: Simplify driver probe function

[Michal Simek]

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

Hi Linus,

can you please look at this?

Thanks,
Michal


On 06/03/2013 02:31 PM, Michal Simek wrote:
> Simplification is done by using OF helper function
> which increase readability of code and remove
> (if (var) var = be32_to_cpup;) assignment.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> Changes in v2:
> - New patch in this series
> 
>  drivers/gpio/gpio-xilinx.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index 9ae7aa8..2aad534 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -170,24 +170,20 @@ static int xgpio_of_probe(struct device_node *np)
>  		return -ENOMEM;
> 
>  	/* Update GPIO state shadow register with default value */
> -	tree_info = of_get_property(np, "xlnx,dout-default", NULL);
> -	if (tree_info)
> -		chip->gpio_state = be32_to_cpup(tree_info);
> +	of_property_read_u32(np, "xlnx,dout-default", &chip->gpio_state);
> +
> +	/* By default, all pins are inputs */
> +	chip->gpio_dir = 0xFFFFFFFF;
> 
>  	/* Update GPIO direction shadow register with default value */
> -	chip->gpio_dir = 0xFFFFFFFF; /* By default, all pins are inputs */
> -	tree_info = of_get_property(np, "xlnx,tri-default", NULL);
> -	if (tree_info)
> -		chip->gpio_dir = be32_to_cpup(tree_info);
> +	of_property_read_u32(np, "xlnx,tri-default", &chip->gpio_dir);
> +
> +	/* By default assume full GPIO controller */
> +	chip->mmchip.gc.ngpio = 32;
> 
>  	/* Check device node and parent device node for device width */
> -	chip->mmchip.gc.ngpio = 32; /* By default assume full GPIO controller */
> -	tree_info = of_get_property(np, "xlnx,gpio-width", NULL);
> -	if (!tree_info)
> -		tree_info = of_get_property(np->parent,
> -					    "xlnx,gpio-width", NULL);
> -	if (tree_info)
> -		chip->mmchip.gc.ngpio = be32_to_cpup(tree_info);
> +	of_property_read_u32(np, "xlnx,gpio-width",
> +			      (u32 *)&chip->mmchip.gc.ngpio);
> 
>  	spin_lock_init(&chip->gpio_lock);
> 
> --
> 1.8.2.3
> 


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 1/6] GPIO: xilinx: Simplify driver probe function

[Linus Walleij]

On Mon, Jun 3, 2013 at 2:31 PM, Michal Simek <michal.simek@xilinx.com> wrote:

> Simplification is done by using OF helper function
> which increase readability of code and remove
> (if (var) var = be32_to_cpup;) assignment.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> Changes in v2:
> - New patch in this series

Patch applied. Nice cleanup!

Yours,
Linus Walleij

Re: [PATCH v2 2/6] GPIO: xilinx: Add support for dual channel

[Linus Walleij]

On Mon, Jun 3, 2013 at 2:31 PM, Michal Simek <michal.simek@xilinx.com> wrote:

> Supporting the second channel in the driver.
> Offset is 0x8 and both channnels share the same
> IRQ.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>
> ---
> Changes in v2:
> - Use kernel doc format - suggested by Linus Walleij
> - Do not use __raw_readl/__raw_writel IO in this patch
> - Use of_property_read_u32 helper function
> - Use BIT()
> - Change patch subject

Patch is looking overall nice and improves the kernel so
applied.

But check this:

> @@ -202,6 +230,57 @@ static int xgpio_of_probe(struct device_node *np)
>                        np->full_name, status);
>                 return status;
>         }
> +
> +       pr_info("XGpio: %s: registered, base is %d\n", np->full_name,
> +                                                       chip->mmchip.gc.base);
> +
> +       tree_info = of_get_property(np, "xlnx,is-dual", NULL);
> +       if (tree_info && be32_to_cpup(tree_info)) {

Doesn't this work:

if (of_property_read_bool(np, "xlnx,is-dual")) {
(...)

?

Yours,
Linus Walleij

Re: [PATCH v2 6/6] DT: Add documentation for gpio-xilinx

[Linus Walleij]

On Mon, Jun 3, 2013 at 2:31 PM, Michal Simek <michal.simek@xilinx.com> wrote:

> Describe gpio-xilinx binding.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> Changes in v2:
> - Extend description

Thanks, patch applied but look into this:

> +Optional properties:
> +- interrupts : Interrupt mapping for GPIO IRQ.
> +- interrupt-parent : Phandle for the interrupt controller that
> +  services interrupts for this device.
> +- xlnx,all-inputs : if n-th bit is setup, GPIO-n is input
> +- xlnx,dout-default : if n-th bit is 1, GPIO-n default value is 1
> +- xlnx,gpio-width : gpio width
> +- xlnx,tri-default : if n-th bit is 1, GPIO-n is in tristate mode
> +- xlnx,is-dual : if 1, controller also uses the second channel

If is present, xlnx,is-dual;

> +Example:
> +gpio: gpio@40000000 {
> +       #gpio-cells = <2>;
> +       compatible = "xlnx,xps-gpio-1.00.a";
> +       gpio-controller ;
> +       interrupt-parent = <&microblaze_0_intc>;
> +       interrupts = < 6 2 >;
> +       reg = < 0x40000000 0x10000 >;
> +       xlnx,all-inputs = <0x0>;
> +       xlnx,all-inputs-2 = <0x0>;
> +       xlnx,dout-default = <0x0>;
> +       xlnx,dout-default-2 = <0x0>;
> +       xlnx,gpio-width = <0x2>;
> +       xlnx,gpio2-width = <0x2>;
> +       xlnx,interrupt-present = <0x1>;
> +       xlnx,is-dual = <0x1>;

xlnx,is-dual;

I'm not giving up on this suggestion.

Test it please...

Yours,
Linus Walleij

Re: [PATCH v2 6/6] DT: Add documentation for gpio-xilinx

[Michal Simek]

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

On 06/17/2013 07:50 AM, Linus Walleij wrote:
> On Mon, Jun 3, 2013 at 2:31 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> 
>> Describe gpio-xilinx binding.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>> Changes in v2:
>> - Extend description
> 
> Thanks, patch applied but look into this:
> 
>> +Optional properties:
>> +- interrupts : Interrupt mapping for GPIO IRQ.
>> +- interrupt-parent : Phandle for the interrupt controller that
>> +  services interrupts for this device.
>> +- xlnx,all-inputs : if n-th bit is setup, GPIO-n is input
>> +- xlnx,dout-default : if n-th bit is 1, GPIO-n default value is 1
>> +- xlnx,gpio-width : gpio width
>> +- xlnx,tri-default : if n-th bit is 1, GPIO-n is in tristate mode
>> +- xlnx,is-dual : if 1, controller also uses the second channel
> 
> If is present, xlnx,is-dual;
> 
>> +Example:
>> +gpio: gpio@40000000 {
>> +       #gpio-cells = <2>;
>> +       compatible = "xlnx,xps-gpio-1.00.a";
>> +       gpio-controller ;
>> +       interrupt-parent = <&microblaze_0_intc>;
>> +       interrupts = < 6 2 >;
>> +       reg = < 0x40000000 0x10000 >;
>> +       xlnx,all-inputs = <0x0>;
>> +       xlnx,all-inputs-2 = <0x0>;
>> +       xlnx,dout-default = <0x0>;
>> +       xlnx,dout-default-2 = <0x0>;
>> +       xlnx,gpio-width = <0x2>;
>> +       xlnx,gpio2-width = <0x2>;
>> +       xlnx,interrupt-present = <0x1>;
>> +       xlnx,is-dual = <0x1>;
> 
> xlnx,is-dual;
> 
> I'm not giving up on this suggestion.

I have commented this in the v1. The point is that all dtses for the last
3-4 years are using this binding and this value is present there for a long
time. That's why I think it is better to keep this binding and do not break
backward compatibility which there is.
Also it reflects how design tool describe hardware.
IP is designed that can be easily extended to more channels which means
that it won't be simple yes/no option.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 6/6] DT: Add documentation for gpio-xilinx

[Linus Walleij]

On Mon, Jun 17, 2013 at 8:21 AM, Michal Simek <monstr@monstr.eu> wrote:
> On 06/17/2013 07:50 AM, Linus Walleij wrote:
>> On Mon, Jun 3, 2013 at 2:31 PM, Michal Simek <michal.simek@xilinx.com> wrote:

>>> +- xlnx,tri-default : if n-th bit is 1, GPIO-n is in tristate mode
>>> +- xlnx,is-dual : if 1, controller also uses the second channel
>>
>> If is present, xlnx,is-dual;
>>
>>> +       xlnx,is-dual = <0x1>;
>>
>> xlnx,is-dual;
>>
>> I'm not giving up on this suggestion.
>
> I have commented this in the v1.

I commented your comment on v1, and said I think you can support
both bindings.

Yours,
Linus Walleij

Re: [PATCH v2 6/6] DT: Add documentation for gpio-xilinx

[Michal Simek]

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

Hi Linus,

On 06/17/2013 11:13 AM, Linus Walleij wrote:
> On Mon, Jun 17, 2013 at 8:21 AM, Michal Simek <monstr@monstr.eu> wrote:
>> On 06/17/2013 07:50 AM, Linus Walleij wrote:
>>> On Mon, Jun 3, 2013 at 2:31 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> 
>>>> +- xlnx,tri-default : if n-th bit is 1, GPIO-n is in tristate mode
>>>> +- xlnx,is-dual : if 1, controller also uses the second channel
>>>
>>> If is present, xlnx,is-dual;
>>>
>>>> +       xlnx,is-dual = <0x1>;
>>>
>>> xlnx,is-dual;
>>>
>>> I'm not giving up on this suggestion.
>>
>> I have commented this in the v1.
> 
> I commented your comment on v1, and said I think you can support
> both bindings.

in 2/6 you have applied that dual support for this driver
and that's why please add this binding description to your repo
because it reflects actual binding for this driver.

As I wrote you I am working on interrupt support for this IP
and in connection to this I will introduce new binding
as we discussed in v1.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 6/6] DT: Add documentation for gpio-xilinx

[Linus Walleij]

On Mon, Jun 24, 2013 at 1:54 PM, Michal Simek <monstr@monstr.eu> wrote:

>> I commented your comment on v1, and said I think you can support
>> both bindings.
>
> in 2/6 you have applied that dual support for this driver
> and that's why please add this binding description to your repo
> because it reflects actual binding for this driver.

I applied this patch ages ago I think?

This discussion is about what I'd like to see as new changes...

Yours,
Linus Walleij