Re: [PATCH 1/4] Make non-linear GPIO ranges accesible from gpiolib

[Christian Ruppert]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/