| Message ID | 1398130543-19454-1-git-send-email-haojian.zhuang@linaro.org |
|---|---|
| State | New |
| Headers | show |
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }
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