lkml.org 
[lkml]   [2016]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 2/2] gpio: dt-bindings: add brcm,bcm6345-gpio bindings
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

\
 
 \ /
  Last update: 2016-08-03 15:01    [W:0.085 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site