| Message ID | 1470225957-5692-1-git-send-email-noltari@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
All devicetree binding patches must be sent to the devicetree@vger.kernel.org mailing list so include them on subsequent posts of this patch. On Wed, Aug 3, 2016 at 2:05 PM, Álvaro Fernández Rojas <noltari@gmail.com> wrote: > This patch adds the device tree bindings for the Broadcom's BCM6345 > memory-mapped GPIO controllers. > > The gpios will be supported by gpio-mmio code of the > GPIO generic library. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > --- > v2: add improvements suggested by Jonas Gorski: > - use native-endian instead of big-endian. What is this? Does it come from some other existing binding? I was feeling native endian is the default unless LE or BE is explicitly specified... > +Required properties: > + - compatible: should be "brcm,bcm6345-gpio" > + - reg-names: must contain > + "dat" - data register > + "dirout" - direction (output) register I don't like this and don't understand why you can't just cover all GPIO registers with a single reg property. > + - reg: address + size pairs describing the GPIO register sets; > + order must correspond with the order of entries in reg-names > + - #gpio-cells: must be set to 2. The first cell is the pin number and > + the 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. Reference the standard bindings in gpio.txt for cells and controller. > +Optional properties: > + - native-endian: use native endian memory. That is weird. Explain why this is needed. > + - BCM6345: > + gpio: gpio-controller@fffe0406 { > + compatible = "brcm,bcm6345-gpio"; > + reg-names = "dirout", "dat"; > + reg = <0xfffe0406 2>, <0xfffe040a 2>; Also I do not understand this at all. Why pick out two 16bit registers? Surely the rest of the registers at 0xfffe0400 must be GPIO registers as well? Are they not? If they are, cover them all with a single reg property. If they are not all GPIO registers, use MFD syscon for this and access the constituent parts through that. 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
Hi Linus, El 11/08/2016 a las 13:45, Linus Walleij escribió: > All devicetree binding patches must be sent to the devicetree@vger.kernel.org > mailing list so include them on subsequent posts of this patch. > > On Wed, Aug 3, 2016 at 2:05 PM, Álvaro Fernández Rojas > <noltari@gmail.com> wrote: > >> This patch adds the device tree bindings for the Broadcom's BCM6345 >> memory-mapped GPIO controllers. >> >> The gpios will be supported by gpio-mmio code of the >> GPIO generic library. >> >> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >> --- >> v2: add improvements suggested by Jonas Gorski: >> - use native-endian instead of big-endian. > > What is this? Does it come from some other existing binding? > I was feeling native endian is the default unless LE or BE is > explicitly specified... Not really, even if the kernel is configured as big endian, either native-endian or big-endian are needed. The default is little-endian if nothing else is specified: https://github.com/torvalds/linux/blob/master/drivers/of/base.c#L598 > >> +Required properties: >> + - compatible: should be "brcm,bcm6345-gpio" >> + - reg-names: must contain >> + "dat" - data register >> + "dirout" - direction (output) register > > I don't like this and don't understand why you can't just cover > all GPIO registers with a single reg property. Because we want to add a simple gpio driver for these old SoCs with the newer ones having a different and more complex gpio/pinctrl driver. > >> + - reg: address + size pairs describing the GPIO register sets; >> + order must correspond with the order of entries in reg-names >> + - #gpio-cells: must be set to 2. The first cell is the pin number and >> + the 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. > > Reference the standard bindings in gpio.txt for cells and controller. Sure, I just copy & pasted this from the other gpio mmio driver: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/gpio/wd%2Cmbl-gpio.txt#L17 > >> +Optional properties: >> + - native-endian: use native endian memory. > > That is weird. Explain why this is needed. (explained above) > >> + - BCM6345: >> + gpio: gpio-controller@fffe0406 { >> + compatible = "brcm,bcm6345-gpio"; >> + reg-names = "dirout", "dat"; >> + reg = <0xfffe0406 2>, <0xfffe040a 2>; > > Also I do not understand this at all. Why pick out two 16bit registers? > Surely the rest of the registers at 0xfffe0400 must be GPIO registers > as well? Are they not? This is the layout for BCM6345: https://gitlab.labs.nic.cz/turris/openwrt/blob/8bfda76892a39c033b5abf2c17035a444fffad08/target/linux/brcm63xx-2.6/files/include/asm-mips/mach-bcm963xx/6345_map_part.h#L141 And this is the layout for BCM6338: https://gitlab.labs.nic.cz/turris/openwrt/blob/8bfda76892a39c033b5abf2c17035a444fffad08/target/linux/brcm63xx-2.6/files/include/asm-mips/mach-bcm963xx/6338_map_part.h#L202 > > If they are, cover them all with a single reg property. > > If they are not all GPIO registers, use MFD syscon for this and access > the constituent parts through that. I don't think that's the case since we're using a generic mmio binding. > > Yours, > Linus Walleij > Regards, Álvaro. -- 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 Wed, Aug 3, 2016 at 2:05 PM, Álvaro Fernández Rojas <noltari@gmail.com> wrote: > This patch adds the device tree bindings for the Broadcom's BCM6345 > memory-mapped GPIO controllers. > > The gpios will be supported by gpio-mmio code of the > GPIO generic library. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> Turns out my remarks on the bindings and driver were stupid. Patch applied. 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/brcm,bcm6345-gpio.txt b/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt new file mode 100644 index 0000000..e785314 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt @@ -0,0 +1,46 @@ +Bindings for the Broadcom's brcm,bcm6345-gpio memory-mapped GPIO controllers. + +These bindings can be used on any BCM63xx SoC. However, BCM6338 and BCM6345 +are the only ones which don't need a pinctrl driver. +BCM6338 have 8-bit data and dirout registers, where GPIO state can be read +and/or written, and the direction changed from input to output. +BCM6345 have 16-bit data and dirout registers, where GPIO state can be read +and/or written, and the direction changed from input to output. + +Required properties: + - compatible: should be "brcm,bcm6345-gpio" + - reg-names: must contain + "dat" - data register + "dirout" - direction (output) register + - reg: address + size pairs describing the GPIO register sets; + order must correspond with the order of entries in reg-names + - #gpio-cells: must be set to 2. The first cell is the pin number and + the 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. + +Optional properties: + - native-endian: use native endian memory. + +Examples: + - BCM6338: + gpio: gpio-controller@fffe0407 { + compatible = "brcm,bcm6345-gpio"; + reg-names = "dirout", "dat"; + reg = <0xfffe0407 1>, <0xfffe040f 1>; + + #gpio-cells = <2>; + gpio-controller; + }; + + - BCM6345: + gpio: gpio-controller@fffe0406 { + compatible = "brcm,bcm6345-gpio"; + reg-names = "dirout", "dat"; + reg = <0xfffe0406 2>, <0xfffe040a 2>; + native-endian; + + #gpio-cells = <2>; + gpio-controller; + };
This patch adds the device tree bindings for the Broadcom's BCM6345 memory-mapped GPIO controllers. The gpios will be supported by gpio-mmio code of the GPIO generic library. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> --- v2: add improvements suggested by Jonas Gorski: - use native-endian instead of big-endian. - use gpio alias instead of gpio0. - be less dramatic with newer Broadcom SoCs. - reorder patches. .../devicetree/bindings/gpio/brcm,bcm6345-gpio.txt | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt