[PATCH 1/2] gpio: pl061: get gpio base from alias id

[PATCH 1/2] gpio: pl061: get gpio base from alias id

[Haojian Zhuang]

If gpio base number isn't specified, the gpio base will be find from
the end of gpio number. In order to keep with schematics, use alias
to get the ID of gpio chip.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 .../devicetree/bindings/gpio/gpio-pl061.txt        | 31 ++++++++++++++++++++++
 drivers/gpio/gpio-pl061.c                          | 14 +++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pl061.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pl061.txt b/Documentation/devicetree/bindings/gpio/gpio-pl061.txt
new file mode 100644
index 0000000..164b5ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pl061.txt
@@ -0,0 +1,31 @@
+PL061 GPIO controller bindings
+
+Required properties:
+- compatible:
+  - "arm,pl061", "arm,primecell".
+- #gpio-cells : Should be two.
+  - first cell is the gpio pin number
+  - second cell is used to specify the gpio polarity:
+      0 = active high
+      1 = active low
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupt-controller : Marks the device node as an interrupt controller.
+- #interrupt-cells : Should be two.
+  - first cell is the hw irq number
+  - second cell is used to specify the interrupt type:
+      0 = default, unspecified type
+      1 = rising edge triggered
+      2 = falling edge triggered
+      4 = high level triggered
+      8 = low level triggered
+
+Example:
+	gpio0: gpio@fc806000 {
+		compatible = "arm,pl061", "arm,primecell";
+		reg = <0xfc806000 0x1000>;
+		interrupts = <0 64 0x4>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+	};
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index b0f4752..14f3ab5 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -236,6 +236,18 @@ static struct irq_chip pl061_irqchip = {
 	.irq_set_type	= pl061_irq_type,
 };
 
+/* Parse gpio base from DT */
+static int pl061_parse_gpio_base(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	int ret, id;
+
+	id = of_alias_get_id(np, "gpio");
+	if (id < 0)
+		return id;
+	return (id * PL061_GPIO_NR);
+}
+
 static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 {
 	struct device *dev = &adev->dev;
@@ -255,7 +267,7 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 			return -ENODEV;
 		}
 	} else {
-		chip->gc.base = -1;
+		chip->gc.base = pl061_parse_gpio_base(dev);
 		irq_base = 0;
 	}
 
-- 
1.9.1

[PATCH 2/2] ARM: dts: add gpio alias in hi3620 dts file

[Haojian Zhuang]

Use gpio alias to identify the index of gpio chip. Then we can keep the
same gpio number as schematics. Otherwise, gpio number is countered from
bottom to top.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/boot/dts/hi3620.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/hi3620.dtsi b/arch/arm/boot/dts/hi3620.dtsi
index 6836795..2aaae09 100644
--- a/arch/arm/boot/dts/hi3620.dtsi
+++ b/arch/arm/boot/dts/hi3620.dtsi
@@ -21,6 +21,28 @@
 		serial2 = &uart2;
 		serial3 = &uart3;
 		serial4 = &uart4;
+		gpio0 = &gpio0;
+		gpio1 = &gpio1;
+		gpio2 = &gpio2;
+		gpio3 = &gpio3;
+		gpio4 = &gpio4;
+		gpio5 = &gpio5;
+		gpio6 = &gpio6;
+		gpio7 = &gpio7;
+		gpio8 = &gpio8;
+		gpio9 = &gpio9;
+		gpio10 = &gpio10;
+		gpio11 = &gpio11;
+		gpio12 = &gpio12;
+		gpio13 = &gpio13;
+		gpio14 = &gpio14;
+		gpio15 = &gpio15;
+		gpio16 = &gpio16;
+		gpio17 = &gpio17;
+		gpio18 = &gpio18;
+		gpio19 = &gpio19;
+		gpio20 = &gpio20;
+		gpio21 = &gpio21;
 	};
 
 	pclk: clk {
-- 
1.9.1

Re: [PATCH 1/2] gpio: pl061: get gpio base from alias id

[Baruch Siach]

Hi Haojian Zhuang,

On Tue, Apr 22, 2014 at 09:35:42AM +0800, Haojian Zhuang wrote:
> If gpio base number isn't specified, the gpio base will be find from
> the end of gpio number. In order to keep with schematics, use alias
> to get the ID of gpio chip.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  .../devicetree/bindings/gpio/gpio-pl061.txt        | 31 ++++++++++++++++++++++

Binding documentation should be done in a separate patch.

>  drivers/gpio/gpio-pl061.c                          | 14 +++++++++-
>  2 files changed, 44 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pl061.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pl061.txt b/Documentation/devicetree/bindings/gpio/gpio-pl061.txt
> new file mode 100644
> index 0000000..164b5ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pl061.txt
> @@ -0,0 +1,31 @@
> +PL061 GPIO controller bindings
> +
> +Required properties:
> +- compatible:
> +  - "arm,pl061", "arm,primecell".
> +- #gpio-cells : Should be two.
> +  - first cell is the gpio pin number
> +  - second cell is used to specify the gpio polarity:
> +      0 = active high
> +      1 = active low
> +- gpio-controller : Marks the device node as a GPIO controller.
> +- interrupt-controller : Marks the device node as an interrupt controller.
> +- #interrupt-cells : Should be two.
> +  - first cell is the hw irq number
> +  - second cell is used to specify the interrupt type:
> +      0 = default, unspecified type
> +      1 = rising edge triggered
> +      2 = falling edge triggered
> +      4 = high level triggered
> +      8 = low level triggered
> +
> +Example:
> +	gpio0: gpio@fc806000 {
> +		compatible = "arm,pl061", "arm,primecell";
> +		reg = <0xfc806000 0x1000>;
> +		interrupts = <0 64 0x4>;

The "interrupts" property should be mentioned above.

> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +	};

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Re: [PATCH 1/2] gpio: pl061: get gpio base from alias id

[Linus Walleij]

On Tue, Apr 22, 2014 at 3:35 AM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> If gpio base number isn't specified, the gpio base will be find from
> the end of gpio number. In order to keep with schematics, use alias
> to get the ID of gpio chip.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

The idea with GPIO numbers is not that these shall correspond
to schematics or data sheets or anything like that. It's a completely
Linux-internal number and has nothing to do with anything else.

The same is true for IRQ numbers.

The long term goal is to get rid of both GPIO and IRQ numbers
and deal only with descriptors in the kernel.

> +++ b/Documentation/devicetree/bindings/gpio/gpio-pl061.txt

A patch to add the device tree bindings is *very* welcome.

>         } else {
> -               chip->gc.base = -1;
> +               chip->gc.base = pl061_parse_gpio_base(dev);
>                 irq_base = 0;
>         }

I do not like a patch that changes the dynamic numbering to a
static one, that make things worse for our goals, not better :-(

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: pl061: get gpio base from alias id

[Haojian Zhuang]

On 23 April 2014 21:21, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Apr 22, 2014 at 3:35 AM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>
>> If gpio base number isn't specified, the gpio base will be find from
>> the end of gpio number. In order to keep with schematics, use alias
>> to get the ID of gpio chip.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>
> The idea with GPIO numbers is not that these shall correspond
> to schematics or data sheets or anything like that. It's a completely
> Linux-internal number and has nothing to do with anything else.
>
> The same is true for IRQ numbers.
>
> The long term goal is to get rid of both GPIO and IRQ numbers
> and deal only with descriptors in the kernel.

Go it. But the gpio sysfs interface is using the gpio name with an
internal number. It'll make developer confusion since it's different
from datasheet
or schematics. Is there any plan to remove the confusion?

Regards
Haojian

Re: [PATCH 1/2] gpio: pl061: get gpio base from alias id

[Linus Walleij]

On Fri, Apr 25, 2014 at 2:27 AM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:

> Go it. But the gpio sysfs interface is using the gpio name with an
> internal number. It'll make developer confusion since it's different
> from datasheet
> or schematics. Is there any plan to remove the confusion?

First I am as always very suspicious about the sysfs interface
since it is often used for madness like reimplementing
leds-gpio.c, gpio_keys.c, extcon-gpio.c, gpio-regulator.c
spi-gpio.c, w1-gpio.c or gpio_wdt.c in userspace for no
good reason when the kernel should handle it.

Yet there are exceptions where it's actually warranted.

There is a possibility to set a label on the gpio line,
so this should be used.

C.f.
commit 781f6d710d4482eab05cfaad50060a0ea8c0e4e0
"gpio: generic: Add label to platform data"

Something like this is needed in this and other drivers
needing to label its GPIO lines. It's much like how we name
all pins in a pin controller really.

I don't exactly know how we should achieve the same
for labels to offsets in any given gpio_chip using
some generic device tree labels for example, but I
imagine it could be implemented in a generic fashion?

Describing the names of gpio lines should be quite
obvious hardware description suitable for the device
tree.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: pl061: get gpio base from alias id

[Rob Herring]

On Mon, Apr 21, 2014 at 8:35 PM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:
> If gpio base number isn't specified, the gpio base will be find from
> the end of gpio number. In order to keep with schematics, use alias
> to get the ID of gpio chip.

NAK. This is an abuse aliases.

> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  .../devicetree/bindings/gpio/gpio-pl061.txt        | 31 ++++++++++++++++++++++

Is something wrong with the binding doc already in pl061-gpio.txt?

Rob

Re: [PATCH 1/2] gpio: pl061: get gpio base from alias id

[Haojian Zhuang]

On 24 April 2014 02:52, Rob Herring <robherring2@gmail.com> wrote:
> On Mon, Apr 21, 2014 at 8:35 PM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>> If gpio base number isn't specified, the gpio base will be find from
>> the end of gpio number. In order to keep with schematics, use alias
>> to get the ID of gpio chip.
>
> NAK. This is an abuse aliases.
>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>>  .../devicetree/bindings/gpio/gpio-pl061.txt        | 31 ++++++++++++++++++++++
>
> Is something wrong with the binding doc already in pl061-gpio.txt?
>
Oh. I just copy my old patch to here. I didn't notice there's a new
pl061-gpio.txt. Thanks for reminder.

Regards
Haojian