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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20120407035327.9D7923E246E@localhost>
Date:	Fri, 06 Apr 2012 20:53:27 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Roland Stigge <stigge@...com.de>, arm@...nel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linus.walleij@...ricsson.com
Cc:	Roland Stigge <stigge@...com.de>
Subject: Re: [PATCH v2] gpio: Device tree support for LPC32xx

On Wed,  4 Apr 2012 01:58:38 +0200, Roland Stigge <stigge@...com.de> wrote:
> This patch adds device tree support for gpio-lpc32xx.c
> 
> Signed-off-by: Roland Stigge <stigge@...com.de>
> 
> ---
> 
>  Applies to v3.4-rc1
> 
>  Changes since last version:
>  * Documented property #address-cells (always 1)
>  * Documented property status="disabled"
>  * Removed gpio-lines property
>  * Removed #gpio-cells from the parent node
>  * Support both DT and non-DT to make patch less disruptive and independent
>    from DT switch of mach-lpc32xx
>  * Use module_platform_driver()
> 
>  Thanks to Arnd Bergmann and Grant Likely for reviewing!
> 
>  Documentation/devicetree/bindings/gpio/gpio_lpc32xx.txt |   65 ++++++++++++++++
>  arch/arm/mach-lpc32xx/include/mach/gpio.h               |    9 +-
>  drivers/gpio/gpio-lpc32xx.c                             |   52 ++++++++++++
>  3 files changed, 123 insertions(+), 3 deletions(-)
> 
> --- /dev/null
> +++ linux-2.6/Documentation/devicetree/bindings/gpio/gpio_lpc32xx.txt
> @@ -0,0 +1,65 @@
> +NXP LPC32xx SoC GPIO controller
> +
> +Required properties:
> +- compatible: "nxp,lpc32xx-gpio"
> +- reg: Physical base address and length of the controller's registers.
> +- #address-cells: Always 1, for indexing of the subnodes (GPIO groups of the
> +  SoC)
> +- #size-cells: Always 0
> +
> +Required properties of sub-nodes which describe the GPIO groups of LPC32xx:
> +- gpio-controller: Marks the device node as a GPIO controller.
> +- #gpio-cells: Should be two. The first cell is the pin number and the
> +  second cell is used to specify optional parameters:
> +  - bit 0 specifies polarity (0 for normal, 1 for inverted)
> +- reg: Index of the GPIO group

If these are merely contiguous register banks of 32 gpio lines, then
established convention is pretty much to only use one node and make
the translate function decode bank and bit out of the gpio specifier.
There isn't a whole lot of value it having all the sub nodes when
there isn't anything significantly different between them.

How is documentation for this gpio controller written?  Is it one big
number space, or are the gpio numbers split up into banks?  You have
two choices here for the encoding; either just use #gpio-cells=<2> as
done now, or use #gpio-cells=<3> and using cell0 for the bank and
cell1 for the bit.  You should choose the option that best lines up
with how the hardware documents its gpio pins because that will be the
most user-friendly.

g.

> +
> +Optional properties of sub-nodes which describe the GPIO groups of LPC32xx:
> +- status: Set to "disabled" if you don't use the respective GPIO group
> +  of the LPC32 SoC
> +
> +Example:
> +
> +	gpio: gpio@...28000 {
> +		compatible = "nxp,lpc32xx-gpio";
> +		reg = <0x40028000 0x1000>;
> +		/* create a private address space for enumeration */
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		gpio_p0: gpio-bank@0 {
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			reg = <0>;
> +		};
> +
> +		gpio_p1: gpio-bank@1 {
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			reg = <1>;
> +		};
> +
> +		gpio_p2: gpio-bank@2 {
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			reg = <2>;
> +		};
> +
> +		gpio_p3: gpio-bank@3 {
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			reg = <3>;
> +		};
> +
> +		gpi_p3: gpio-bank@4 {
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			reg = <4>;
> +		};
> +
> +		gpo_p3: gpio-bank@5 {
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			reg = <5>;
> +		};
> +	};
> --- linux-2.6.orig/arch/arm/mach-lpc32xx/include/mach/gpio.h
> +++ linux-2.6/arch/arm/mach-lpc32xx/include/mach/gpio.h
> @@ -1 +1,8 @@
> -/* empty */
> +#ifndef __MACH_GPIO_H
> +#define __MACH_GPIO_H
> +
> +#include "gpio-lpc32xx.h"
> +
> +#define ARCH_NR_GPIOS (LPC32XX_GPO_P3_GRP + LPC32XX_GPO_P3_MAX)
> +
> +#endif /* __MACH_GPIO_H */
> --- linux-2.6.orig/drivers/gpio/gpio-lpc32xx.c
> +++ linux-2.6/drivers/gpio/gpio-lpc32xx.c
> @@ -21,6 +21,9 @@
>  #include <linux/io.h>
>  #include <linux/errno.h>
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
>  
>  #include <mach/hardware.h>
>  #include <mach/platform.h>
> @@ -454,10 +457,55 @@ static struct lpc32xx_gpio_chip lpc32xx_
>  	},
>  };
>  
> +/* Empty now, can be removed later when mach-lpc32xx is finally switched over
> + * to DT support
> + */
>  void __init lpc32xx_gpio_init(void)
>  {
> +}
> +
> +static int __devinit lpc32xx_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node;
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(lpc32xx_gpiochip); i++)
> -		gpiochip_add(&lpc32xx_gpiochip[i].chip);
> +	if (pdev->dev.of_node) {
> +		for_each_child_of_node(pdev->dev.of_node, node) {
> +			if (of_device_is_available(node)) {
> +				u32 index;
> +				struct gpio_chip *chip;
> +				if (of_property_read_u32(node,
> +							 "reg", &index) <  0)
> +					continue;
> +				if (index >= ARRAY_SIZE(lpc32xx_gpiochip))
> +					continue;
> +				chip = &lpc32xx_gpiochip[index].chip;
> +				chip->of_node = of_node_get(node);
> +				gpiochip_add(chip);
> +			}
> +		}
> +	} else {
> +		for (i = 0; i < ARRAY_SIZE(lpc32xx_gpiochip); i++)
> +			gpiochip_add(&lpc32xx_gpiochip[i].chip);
> +	}
> +
> +	return 0;
>  }
> +
> +#ifdef CONFIG_OF
> +static struct of_device_id lpc32xx_gpio_of_match[] __devinitdata = {
> +	{ .compatible = "nxp,lpc32xx-gpio", },
> +	{ },
> +};
> +#endif
> +
> +static struct platform_driver lpc32xx_gpio_driver = {
> +	.driver		= {
> +		.name	= "lpc32xx-gpio",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = lpc32xx_gpio_of_match,
> +	},
> +	.probe		= lpc32xx_gpio_probe,
> +};
> +
> +module_platform_driver(lpc32xx_gpio_driver);

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ