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]

Message-ID: <51C23060.50807@wwwdotorg.org>
Date:	Wed, 19 Jun 2013 16:27:44 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Christian Ruppert <christian.ruppert@...lis.com>
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 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.

> +   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.

--
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/