On Wed, Jun 19, 2013 at 04:27:44PM -0600, Stephen Warren wrote:
> On 06/18/2013 03:29 AM, Christian Ruppert wrote:
> > This patch adds the infrastructure required to register non-linear gpio
> > ranges through gpiolib and the standard GPIO device tree bindings.
>
> I review this in case we decide to go with it anyway.
>
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
>
> > +In addition, named groups of pins can be mapped to pin groups of a given
> > +pin controller:
> > +
> > + gpio_pio_g: gpio-controller@1480 {
> > + #gpio-cells = <2>;
> > + compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
> > + reg = <0x1480 0x18>;
> > + gpio-controller;
> > + gpio-ranges = <&pinctrl1 0 0 0>, <&pinctrl2 3 0 0>;
> > + gpio-ranges-group-names = "foo", "bar";
> > + };
> > +
> > +where,
> > + &pinctrl1 and &pinctrl2 is the phandle to the pinctrl DT node.
> > +
> > + The following value specifies the base GPIO offset of the pin range with
> > + respect to the GPIO controller's base. The remaining two values must be
> > + 0 to indicate that a named pin group should be used for the respective
> > + range. The number of pins in the range is the number of pins in the pin
> > + group.
>
> It'd be good to re-write this section in a similar style to the cleanup
> patches that I sent for the existing gpio-ranges documentation. That
> makes the format description more of a raw syntax than English text.
can you please point me to some place where I can find those patches on
the web?
> > + gpio-ranges-group-names defines the name of each pingroup of the
> > + respective pin controller.
> > +
> > +The pinctrl node must have a "#gpio-#gpio-range-cells" property set to three
> > +to define the number of arguments to pass with the phandle.
>
> There's some mistake in the property name there. I'd assert we should
> remove those two lines anyway, and use the new OF parsing code I posted
> when cleaning up gpio-ranges.
>
> > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
>
> > + if (pinspec.args[2]) {
> > + /* npins != 0: linear range */
> > + ret = gpiochip_add_pin_range(chip,
> > + pinctrl_dev_get_devname(pctldev),
> > + pinspec.args[0],
> > + pinspec.args[1],
> > + pinspec.args[2]);
> > + if (ret)
> > + break;
> > + } else {
>
> I think here we should validate !pinspec.args[1], to ensure that value
> doesn't get set to anything wonky.
>
--
Christian Ruppert , <christian.ruppert@xxxxxxxxxx>
/|
Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri
_// | bilis Systems CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
