[PATCH 2/2] gpio: dt-bindings: add brcm,bcm6345-gpio bindings

[PATCH 2/2] gpio: dt-bindings: add brcm,bcm6345-gpio bindings

[Álvaro Fernández Rojas]

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>
---
 .../devicetree/bindings/gpio/brcm,bcm6345-gpio.txt | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt

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..48e35ec
--- /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 should only be used for BCM6338 and BCM6345 SoCs, since newer
+ones need a proper 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:
+	- big-endian: Memory is big endian.
+
+Examples:
+	- BCM6338:
+	gpio0: gpio-controller@fffe0407 {
+		compatible = "brcm,bcm6345-gpio";
+		reg-names = "dirout", "dat";
+		reg = <0xfffe0407 1>, <0xfffe040f 1>;
+
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+
+	- BCM6345:
+	gpio0: gpio-controller@fffe0406 {
+		compatible = "brcm,bcm6345-gpio";
+		reg-names = "dirout", "dat";
+		reg = <0xfffe0406 2>, <0xfffe040a 2>;
+		big-endian;
+
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
-- 
2.1.4

Re: [PATCH 2/2] gpio: dt-bindings: add brcm,bcm6345-gpio bindings

[Jonas Gorski]

Hi,

On 3 August 2016 at 13:02, Álvaro Fernández Rojas <noltari@gmail.com> wrote:
> Subject: [PATCH 2/2] gpio: dt-bindings: add brcm,bcm6345-gpio bindings

Minor nitpick: I think the bindings should come first, then the
consumer. We shouldn't add support for it without having okayed the
binding.

> 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>
> ---
>  .../devicetree/bindings/gpio/brcm,bcm6345-gpio.txt | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.txt
>
> 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..48e35ec
> --- /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 should only be used for BCM6338 and BCM6345 SoCs, since newer
> +ones need a proper pinctrl driver.

There's nothing wrong with using it on newer SoCs to have gpio now and
then later replacing it in the dts file with a pinctrl node once the
bindings/drivers are available. We won't/can't drop support for the
gpio-driver, as 6338/6345 don't have pinctrl anyway. So older dts's
with gpio nodes will continue to work, and newer dts's will use the
pinctrl drivers.

> +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:
> +       - big-endian: Memory is big endian.

I think we want native-endian here.

> +
> +Examples:
> +       - BCM6338:
> +       gpio0: gpio-controller@fffe0407 {

Since both have only one bank, it might make sense to call it just
"gpio" instead of numbering it.

> +               compatible = "brcm,bcm6345-gpio";
> +               reg-names = "dirout", "dat";
> +               reg = <0xfffe0407 1>, <0xfffe040f 1>;
> +
> +               #gpio-cells = <2>;
> +               gpio-controller;
> +       };
> +
> +       - BCM6345:
> +       gpio0: gpio-controller@fffe0406 {
> +               compatible = "brcm,bcm6345-gpio";
> +               reg-names = "dirout", "dat";
> +               reg = <0xfffe0406 2>, <0xfffe040a 2>;
> +               big-endian;
> +
> +               #gpio-cells = <2>;
> +               gpio-controller;
> +       };

Apart from those (minor) nitpicks, it looks good to me.


Regards
Jonas