Re: [PATCH 4/5] dt-bindings: gpio: add documentation for mt7621-gpio

[Sergio Paracuellos <sergio.paracuellos@gmail.com>]

On Tue, May 15, 2018 at 11:21:20AM +1000, NeilBrown wrote:
> On Mon, May 14 2018, Sergio Paracuellos wrote:
> 
> > This commit add missing dt bindings documentation for mt7621-gpio
> > driver. After this checkpatch script complain about this
> > issue dissapears.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../devicetree/bindings/gpio/mtk,mt7621-gpio.txt   | 51 ++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> > new file mode 100644
> > index 0000000..5fe4bb5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/mtk,mt7621-gpio.txt
> > @@ -0,0 +1,51 @@
> > +Mediatek SoC GPIO controller bindings
> > +
> > +The IP core used inside these SoCs has 3 banks of 32 GPIOs each.
> > +The registers of all the banks are interwoven inside one single IO range.
> > +We load one GPIO controller instance per bank. To make this possible
> > +we support 2 types of nodes. The parent node defines the memory I/O range and
> > +has 3 children each describing a single bank.
> > +
> > +Required properties for the top level node:
> > +- compatible:
> > +  - "mtk,mt7621-gpio" for Mediatek controllers
> > +- reg : Physical base address and length of the controller's registers
> > +
> > +Required properties for the GPIO bank node:
> > +- compatible:
> > +  - "mtk,mt7621-gpio-bank" for Mediatek banks
> > +- #gpio-cells : Should be two.
> > +  - first cell is the pin number
> > +  - second cell is used to specify optional parameters (unused)
> > +- gpio-controller : Marks the device node as a GPIO controller
> > +- reg : The id of the bank that the node describes.
> 
> This is really good, but not quite complete.
> Searching for "of_" in gpio-mt7621.c I find code handling everything
> you've described, but also:
> 
> 	mediatek_gpio_irq = irq_of_parse_and_map(np, 0);
> 	if (mediatek_gpio_irq) {
> 		mediatek_gpio_irq_domain = irq_domain_add_linear(np,
> 
> The GPIO controller can receive interrupts on any of the GPIOs,
> either edge or level.  It then interrupts the CPU using GIC INT12.
> 
> so
>     interrupt-parent = <&gic>;
>     interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>
> (I think).
> Then you need whatever irq_domain_add_linear() expects.
> I don't know what that is...  I tried following through
> code and got lost in little twisty mazes.
> 
> So if you change this patch to add the file to
> drivers/staging/mt7621-gpio
> then I can give it
>   Reviewed-by: NeilBrown <neil@brown.name>

I moved this to the driver directory as it is because dtsi is not
including yet stuff about gpio interrupts. This should be added when
make sure that interrupt works and gtsi will be updated.

> 
> and then we can fix it when an understanding of the interrupts is
> available.
> But I cannot approve it for Documentation/devicetree/bindings yet.
> 
> Thanks a lot,
> NeilBrown
>

Best regards,
    Sergio PAracuellos 
> 
> > +
> > +Example:
> > +	gpio@600 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		compatible = "mtk,mt7621-gpio";
> > +		reg = <0x600 0x100>;
> > +
> > +		gpio0: bank@0 {
> > +			reg = <0>;
> > +			compatible = "mtk,mt7621-gpio-bank";
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +
> > +		gpio1: bank@1 {
> > +			reg = <1>;
> > +			compatible = "mtk,mt7621-gpio-bank";
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +
> > +		gpio2: bank@2 {
> > +			reg = <2>;
> > +			compatible = "mtk,mt7621-gpio-bank";
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +		};
> > +	};
> > -- 
> > 2.7.4


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel