On Wed, Jun 13, 2018 at 01:28:35PM -0600, Rob Herring wrote:
> On Wed, Jun 13, 2018 at 10:23 AM, Sergio Paracuellos
> <sergio.paracuellos@xxxxxxxxx> wrote:
> > Hi Rob,
> >
> > Thanks for your time in reviewing this.
> >
> > On Tue, Jun 12, 2018 at 10:56 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
> >> On Sat, Jun 02, 2018 at 09:30:10AM +0200, Sergio Paracuellos wrote:
> >>> Add a devicetree binding documentation for the mt7621 driver.
> >>
>
> >>> + second cell specifies GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
> >>> + Only the GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
> >>> +- gpio-controller : Marks the device node as a GPIO controller.
> >>> +- reg : The id of the bank that the node describes.
> >>
> >> I'd prefer to not have banks defined in DT. Do you have a variable
> >> number or resources that are per bank? If not, then you don't need them.
> >
> > Mmmm, That's what I understood from documentation:
> >
> > "Some system-on-chips (SoCs) use the concept of GPIO banks. ...
> > Usually each such bank is
> > exposed in the device tree as an individual gpio-controller node. ..."
>
> This should be conditioned on being able to divide up the registers by
> bank which seems like you can't. Or there's the case like the DW GPIO
> block and the number of banks is configurable.
I see. Thanks for clarifing this.
>
> > If this is not a good approach, could you please me point me out to a
> > device tree example where
> > the correct approach is being used?
>
> I'm not sure offhand. There are lots of examples of single nodes I'm
> sure. Which ones have banks I haven't a clue. IIRC, there were some
> cases where the bank # was part of the GPIO cells, but I seem to
> recall Linus prefers not having 3 cells.
Ok, so... does the following single node sounds acceptable?
gpio: gpio@600 {
#gpio-cells = <2>;
#interrupt-cells = <2>;
compatible = "mediatek,mt7621-gpio";
gpio-controller;
interrupt-controller;
reg = <0x600 0x60>;
interrupt-parent = <&gic>;
interrupts = <GIC_SHARED 12 IRQ_TYPE_LEVEL_HIGH>;
mediatek,gpio-bank-widths = <32 32 32>;
}
Changing definition for "reg" and adding a new one for
"mediatek,gpio-bank-widths" as follows:
reg:
Define the base and range of the address space containing
the mediatek GPIO controller registers
mediatek,gpio-bank-widths:
Number of GPIO lines for each bank. Number of elements must
correspond to number of banks suggested by the 'reg' property.
Thanks in advance.
>
> Rob
Best regards,
Sergio Paracuellos
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
