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

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, 26 Jun 2013 13:46:54 +0200
From:	Christian Ruppert <christian.ruppert@...lis.com>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Patrice CHOTARD <patrice.chotard@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	Sascha Leuenberger <sascha.leuenberger@...lis.com>,
	Pierrick Hascoet <pierrick.hascoet@...lis.com>,
	devicetree-discuss@...ts.ozlabs.org, linux-doc@...r.kernel.org,
	Alexandre Courbot <acourbot@...dia.com>
Subject: Re: [PATCH 1/4] Make non-linear GPIO ranges accesible from gpiolib

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@...0 {
> > +		#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@...lis.com>
                                /|
  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/