linux-kernel - Re: [PATCH v8] gpio: add a driver for the Synopsys DesignWare APB GPIO block

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 4 Dec 2013 11:56:28 +0000
From:	Mark Rutland <mark.rutland@....com>
To:	Alan Tull <delicious.quinoa@...il.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Jamie Iles <jamie@...ieiles.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	Steffen Trumtrar <s.trumtrar@...gutronix.de>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Heiko Stuebner <heiko@...ech.de>, Alan Tull <atull@...era.com>,
	Dinh Nguyen <dinguyen@...era.com>,
	Yves Vandervennet <rocket.yvanderv@...il.com>
Subject: Re: [PATCH v8] gpio: add a driver for the Synopsys DesignWare APB
 GPIO block

On Tue, Dec 03, 2013 at 04:41:16PM +0000, Alan Tull wrote:
> From: Jamie Iles <jamie@...ieiles.com>
> 
> The Synopsys DesignWare block is used in some ARM devices (picoxcell)
> and can be configured to provide multiple banks of GPIO pins.
> 
> Signed-off-by: Alan Tull <atull@...era.com>
> Reviewed-by: Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
> 
> v8:     - remove socfpga.dtsi changes
>         - minor cleanup in devicetree documentation
> v7:     - use irq_generic_chip
>         - support one irq per gpio line or one irq for many
>         - s/bank/port/ and other cleanup
> v6:     - (atull) squash the set of patches
>         - use linear irq domain
>         - build fixes. Original driver was reviewed on v3.2.
>         - Fix setting irq edge type for 'rising' and 'both'.
>         - Support as a loadable module.
>         - Use bgpio_chip's spinlock during register access.
>         - Clean up register names to match spec
>         - s/bank/port/ because register names use the word 'port'
>         - s/nr-gpio/nr-gpios/
>         - don't get/put the of_node
>         - remove signoffs/acked-by's because of changes
>         - other cleanup
> v5:     - handle sparse bank population correctly
> v3:     - depend on rather than select IRQ_DOMAIN
>         - split IRQ support into a separate patch
> v2:     - use Rob Herring's irqdomain in generic irq chip patches
>         - use reg property to indicate bank index
>         - support irqs on both edges based on LinusW's u300 driver
> ---
>  .../devicetree/bindings/gpio/snps-dwapb-gpio.txt   |   57 +++
>  drivers/gpio/Kconfig                               |    9 +
>  drivers/gpio/Makefile                              |    1 +
>  drivers/gpio/gpio-dwapb.c                          |  426 ++++++++++++++++++++
>  4 files changed, 493 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
>  create mode 100644 drivers/gpio/gpio-dwapb.c
> 
> diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> new file mode 100644
> index 0000000..e7f144f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> @@ -0,0 +1,57 @@
> +* Synopsys DesignWare APB GPIO controller
> +
> +Required properties:
> +- compatible : Should be "snps,dw-apb-gpio"

s/be/contain/

> +- reg : Address and length of the register set for the device

As this has children with reg entries, it should have #address-cells and
#size-cells (as the example does).

> +
> +The GPIO controller has a configurable number of ports, each of which are
> +represented as child nodes with the following properties:
> +
> +Required properties:
> +- compatible : "snps,dw-apb-gpio-port"
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be two.  The first cell is the pin number and
> +  the second cell is used to specify optional parameters (currently
> +  unused).
> +- reg : The integer port index of the port, a single cell.
> +
> +Optional properties:
> +- interrupt-controller : The first port may be configured to be an interrupt
> +controller.
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +interrupt.  Shall be set to 2.  The first cell defines the interrupt number,
> +the second encodes the triger flags encoded as described in
> +Documentation/devicetree/bindings/interrupts.txt
> +- interrupt-parent : The parent interrupt controller.
> +- interrupts : The interrupts to the parent controller raised when GPIOs
> +generate the interrupts.
> +- snps,nr-gpios : The number of pins in the port, a single cell.
> +
> +Example:
> +
> +gpio: gpio@...00 {
> +       compatible = "snps,dw-apb-gpio";
> +       reg = <0x20000 0x1000>;
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       porta: gpio-controller@0 {
> +               compatible = "snps,dw-apb-gpio-port";
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +               snps,nr-gpio = <8>;
> +               reg = <0>;
> +               interrupt-controller;
> +               #interrupt-cells = <2>;
> +               interrupt-parent = <&vic1>;
> +               interrupts = <0 1 2 3 4 5 6 7>;

Nit: please bracket list entries individually.

Otherwise this looks fine to me.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/