[RFC/PATCH 0/2] R-Car GPIO DT bindings

[RFC/PATCH 0/2] R-Car GPIO DT bindings

[Laurent Pinchart]

Hello,

Here's an RFC that adds DT bindings to the gpio-rcar driver. The patches have
been compile-tested only as I don't have access to the hardware right now, but
I'd like to already receive feedback on the bindings.

Laurent Pinchart (2):
  gpio-rcar: Make the platform data gpio_base field signed
  gpio-rcar: Add DT support

 .../devicetree/bindings/gpio/renesas,gpio-rcar.txt | 48 +++++++++++++++++
 drivers/gpio/gpio-rcar.c                           | 63 ++++++++++++++++++----
 include/linux/platform_data/gpio-rcar.h            |  2 +-
 3 files changed, 102 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt

-- 
Regards,

Laurent Pinchart

[RFC/PATCH 1/2] gpio-rcar: Make the platform data gpio_base field signed

[Laurent Pinchart]

The gpio_base field is used to specify the desired GPIO base for the
GPIO controller. The GPIO core can automatically allocate a GPIO number
range when the base is set to -1. To make this possible, make the field
signed.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/linux/platform_data/gpio-rcar.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/platform_data/gpio-rcar.h b/include/linux/platform_data/gpio-rcar.h
index cc472f6..fb2c718 100644
--- a/include/linux/platform_data/gpio-rcar.h
+++ b/include/linux/platform_data/gpio-rcar.h
@@ -17,7 +17,7 @@
 #define __GPIO_RCAR_H__
 
 struct gpio_rcar_config {
-	unsigned int gpio_base;
+	int gpio_base;
 	unsigned int irq_base;
 	unsigned int number_of_pins;
 	const char *pctl_name;
-- 
1.8.1.5

[RFC/PATCH 2/2] gpio-rcar: Add DT support

[Laurent Pinchart]

Add DT bindings for the gpio-rcar driver and read the device
configuration from the DT node at probe time if available.

Cc: devicetree-discuss@lists.ozlabs.org
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/gpio/renesas,gpio-rcar.txt | 48 +++++++++++++++++
 drivers/gpio/gpio-rcar.c                           | 63 ++++++++++++++++++----
 2 files changed, 101 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt

diff --git a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
new file mode 100644
index 0000000..a2f4cd1
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
@@ -0,0 +1,48 @@
+* Renesas R-Car GPIO Controller
+
+Required Properties:
+
+  - compatible: should be one of the following.
+    - "renesas,gpio-r8a7778": for R8A7778 (R-Mobile M1) compatible GPIO controller.
+    - "renesas,gpio-r8a7779": for R8A7779 (R-Car H1) compatible GPIO controller.
+    - "renesas,gpio-r8a7790": for R8A7790 (R-Car H2) compatible GPIO controller.
+    - "renesas,gpio-rcar": for generic R-Car GPIO controller.
+
+  - reg: Base address and length of each memory resource used by the GPIO
+    controller hardware module.
+
+  - interrupt-parent: phandle of the parent interrupt controller.
+  - interrupts: Interrupt specifier for the controllers interrupt.
+
+  - gpio-controller: Marks the device node as a gpio controller.
+  - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+    cell is used to specify optional parameters as bit flags. Only the GPIO
+    active low flag (bit 0) is currently supported.
+
+Optional Properties:
+
+  - #gpio-lines: Number of GPIOs handled by this controller (defaults to 32).
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: R8A7779 (R-Car H1) GPIO controller nodes
+
+	gpio0: gpio@ffc40000 {
+		compatible = "renesas,gpio-r8a7779", "renesas,gpio-rcar";
+		reg = <0xffc40000 0x2c>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 141 0x4>;
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+	...
+	gpio6: gpio@ffc46000 {
+		compatible = "renesas,gpio-r8a7779", "renesas,gpio-rcar";
+		reg = <0xffc46000 0x2c>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 147 0x4>;
+		#gpio-cells = <2>;
+		gpio-controller;
+		#gpio-lines = <9>;
+	};
diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 0f3d647..a499ea6 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -50,6 +50,8 @@ struct gpio_rcar_priv {
 #define EDGLEVEL 0x24
 #define FILONOFF 0x28
 
+#define RCAR_MAX_GPIO_PER_BANK		32
+
 static inline u32 gpio_rcar_read(struct gpio_rcar_priv *p, int offs)
 {
 	return ioread32(p->base + offs);
@@ -258,9 +260,37 @@ static struct irq_domain_ops gpio_rcar_irq_domain_ops = {
 	.map	= gpio_rcar_irq_domain_map,
 };
 
+static void gpio_rcar_parse_pdata(struct gpio_rcar_priv *p)
+{
+	struct gpio_rcar_config *pdata = p->pdev->dev.platform_data;
+#ifdef CONFIG_OF
+	struct device_node *np = p->pdev->dev.of_node;
+	u32 gpio_lines;
+	int ret;
+#endif
+
+	if (pdata)
+		p->config = *pdata;
+#ifdef CONFIG_OF
+	else if (np) {
+		ret = of_property_read_u32(np, "#gpio-lines", &gpio_lines);
+		p->config.number_of_pins = ret == 0 ? gpio_lines
+					 : RCAR_MAX_GPIO_PER_BANK;
+		p->config.gpio_base = -1;
+	}
+#endif
+
+	if (p->config.number_of_pins == 0 ||
+	    p->config.number_of_pins > RCAR_MAX_GPIO_PER_BANK) {
+		dev_warn(&p->pdev->dev,
+			 "Invalid number of gpio lines %u, using %u\n",
+			 p->config.number_of_pins, RCAR_MAX_GPIO_PER_BANK);
+		p->config.number_of_pins = RCAR_MAX_GPIO_PER_BANK;
+	}
+}
+
 static int gpio_rcar_probe(struct platform_device *pdev)
 {
-	struct gpio_rcar_config *pdata = pdev->dev.platform_data;
 	struct gpio_rcar_priv *p;
 	struct resource *io, *irq;
 	struct gpio_chip *gpio_chip;
@@ -275,14 +305,14 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 		goto err0;
 	}
 
-	/* deal with driver instance configuration */
-	if (pdata)
-		p->config = *pdata;
-
 	p->pdev = pdev;
-	platform_set_drvdata(pdev, p);
 	spin_lock_init(&p->lock);
 
+	/* Get device configuration from DT node or platform data. */
+	gpio_rcar_parse_pdata(p);
+
+	platform_set_drvdata(pdev, p);
+
 	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 
@@ -355,10 +385,12 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 				 p->config.irq_base, ret);
 	}
 
-	ret = gpiochip_add_pin_range(gpio_chip, p->config.pctl_name, 0,
-				     gpio_chip->base, gpio_chip->ngpio);
-	if (ret < 0)
-		dev_warn(&pdev->dev, "failed to add pin range\n");
+	if (p->config.pctl_name) {
+		ret = gpiochip_add_pin_range(gpio_chip, p->config.pctl_name, 0,
+					     gpio_chip->base, gpio_chip->ngpio);
+		if (ret < 0)
+			dev_warn(&pdev->dev, "failed to add pin range\n");
+	}
 
 	return 0;
 
@@ -381,11 +413,22 @@ static int gpio_rcar_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_rcar_of_table[] = {
+	{
+		.compatible = "renesas,gpio-rcar",
+	},
+};
+
+MODULE_DEVICE_TABLE(of, gpio_rcar_of_table);
+#endif
+
 static struct platform_driver gpio_rcar_device_driver = {
 	.probe		= gpio_rcar_probe,
 	.remove		= gpio_rcar_remove,
 	.driver		= {
 		.name	= "gpio_rcar",
+		.of_match_table = of_match_ptr(gpio_rcar_of_table),
 	}
 };
 
-- 
1.8.1.5

Re: [RFC/PATCH 1/2] gpio-rcar: Make the platform data gpio_base field signed

[Simon Horman]

On Fri, May 10, 2013 at 04:48:36PM +0200, Laurent Pinchart wrote:
> The gpio_base field is used to specify the desired GPIO base for the
> GPIO controller. The GPIO core can automatically allocate a GPIO number
> range when the base is set to -1. To make this possible, make the field
> signed.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, queued-up for v3.11 in the gpio-car branch.

> ---
>  include/linux/platform_data/gpio-rcar.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/platform_data/gpio-rcar.h b/include/linux/platform_data/gpio-rcar.h
> index cc472f6..fb2c718 100644
> --- a/include/linux/platform_data/gpio-rcar.h
> +++ b/include/linux/platform_data/gpio-rcar.h
> @@ -17,7 +17,7 @@
>  #define __GPIO_RCAR_H__
>  
>  struct gpio_rcar_config {
> -	unsigned int gpio_base;
> +	int gpio_base;
>  	unsigned int irq_base;
>  	unsigned int number_of_pins;
>  	const char *pctl_name;
> -- 
> 1.8.1.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC/PATCH 2/2] gpio-rcar: Add DT support

[Guennadi Liakhovetski]

Hi Laurent

On Fri, 10 May 2013, Laurent Pinchart wrote:

> Add DT bindings for the gpio-rcar driver and read the device
> configuration from the DT node at probe time if available.
> 
> Cc: devicetree-discuss@lists.ozlabs.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/gpio/renesas,gpio-rcar.txt | 48 +++++++++++++++++
>  drivers/gpio/gpio-rcar.c                           | 63 ++++++++++++++++++----
>  2 files changed, 101 insertions(+), 10 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt

Testing this patch reveals, that (at least) one thing is missing here:

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index a499ea6..e233778 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -342,6 +342,7 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 	gpio_chip->owner = THIS_MODULE;
 	gpio_chip->base = p->config.gpio_base;
 	gpio_chip->ngpio = p->config.number_of_pins;
+	gpio_chip->dev = &pdev->dev;
 
 	irq_chip = &p->irq_chip;
 	irq_chip->name = name;

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [RFC/PATCH 2/2] gpio-rcar: Add DT support

[Laurent Pinchart]

Hi Guennadi,

On Friday 17 May 2013 14:22:22 Guennadi Liakhovetski wrote:
> On Fri, 10 May 2013, Laurent Pinchart wrote:
> > Add DT bindings for the gpio-rcar driver and read the device
> > configuration from the DT node at probe time if available.
> > 
> > Cc: devicetree-discuss@lists.ozlabs.org
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  .../devicetree/bindings/gpio/renesas,gpio-rcar.txt | 48 +++++++++++++++++
> >  drivers/gpio/gpio-rcar.c                           | 63 +++++++++++++----
> >  2 files changed, 101 insertions(+), 10 deletions(-)
> >  create mode 100644
> >  Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> Testing this patch reveals, that (at least) one thing is missing here:
> 
> diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
> index a499ea6..e233778 100644
> --- a/drivers/gpio/gpio-rcar.c
> +++ b/drivers/gpio/gpio-rcar.c
> @@ -342,6 +342,7 @@ static int gpio_rcar_probe(struct platform_device *pdev)
> gpio_chip->owner = THIS_MODULE;
>  	gpio_chip->base = p->config.gpio_base;
>  	gpio_chip->ngpio = p->config.number_of_pins;
> +	gpio_chip->dev = &pdev->dev;
> 
>  	irq_chip = &p->irq_chip;
>  	irq_chip->name = name;

Indeed, thank you. I wonder how I managed not to notice this. I'll integrate 
the modification for v2.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/2] gpio-rcar: Make the platform data gpio_base field signed

[Linus Walleij]

On Fri, May 10, 2013 at 4:48 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> The gpio_base field is used to specify the desired GPIO base for the
> GPIO controller. The GPIO core can automatically allocate a GPIO number
> range when the base is set to -1. To make this possible, make the field
> signed.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij