linux-kernel - Re: [PATCH 2/3 v3] gpio: Add support for Intel ICHx/3100/Series[56] GPIO

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] [day] [month] [year] [list]

Message-ID: <20120305150105.GB27229@ponder.secretlab.ca>
Date:	Mon, 5 Mar 2012 08:01:05 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Aaron Sierra <asierra@...-inc.com>
Cc:	Guenter Roeck <guenter@...ck-us.net>,
	Jean Delvare <khali@...ux-fr.org>,
	Peter Tyser <ptyser@...-inc.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3 v3] gpio: Add support for Intel ICHx/3100/Series[56]
 GPIO

On Fri, Feb 17, 2012 at 12:18:26PM -0600, Aaron Sierra wrote:
> From: Peter Tyser <ptyser@...-inc.com>
> 
> This driver works on many Intel chipsets, including the ICH6, ICH7,
> ICH8, ICH9, ICH10, 3100, Series 5/3400 (Ibex Peak), Series 6/C200
> (Cougar Point), and NM10 (Tiger Point).
> 
> Additional Intel chipsets should be easily supported if needed, eg the
> ICH1-5, EP80579, etc.
> 
> Tested on QM67 (Cougar Point), QM57 (Ibex Peak), 3100 (Whitmore Lake),
> and NM10 (Tiger Point).
> 
> Includes work from Jean Delvare:
> 	- Resource leak removal during module load/unload
> 	- GPIO API bit value enforcement
> 
> Also includes code cleanup from Guenter Roeck.
> 
> Signed-off-by: Peter Tyser <ptyser@...-inc.com>
> Signed-off-by: Aaron Sierra <asierra@...-inc.com>
> ---
>  MAINTAINERS             |    6 +
>  drivers/gpio/Kconfig    |   13 ++
>  drivers/gpio/Makefile   |    1 +
>  drivers/gpio/gpio-ich.c |  440 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 460 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/gpio-ich.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a1fce9a..9a0b59f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3284,6 +3284,12 @@ W:	http://www.developer.ibm.com/welcome/netfinity/serveraid.html
>  S:	Supported
>  F:	drivers/scsi/ips.*
>  
> +ICH LPC DRIVER
> +M:	Peter Tyser <ptyser@...-inc.com>
> +S:	Maintained
> +F:	drivers/mfd/lpc_ich.c
> +F:	drivers/gpio/gpio-ich.c
> +
>  IDE SUBSYSTEM
>  M:	"David S. Miller" <davem@...emloft.net>
>  L:	linux-ide@...r.kernel.org
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d0c4118..3359f1e 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -178,6 +178,19 @@ config GPIO_SCH
>  	  The Intel Tunnel Creek processor has 5 GPIOs powered by the
>  	  core power rail and 9 from suspend power supply.
>  
> +config GPIO_ICH
> +	tristate "Intel ICH GPIO"
> +	depends on PCI && X86
> +	select MFD_CORE
> +	select LPC_ICH
> +	help
> +	  Say yes here to support the GPIO functionality of a number of Intel
> +	  ICH-based chipsets.  Currently supported devices: ICH6, ICH7, ICH8
> +	  ICH9, ICH10, Series 5/3400 (eg Ibex Peak), Series 6/C200 (eg
> +	  Cougar Point), NM10 (Tiger Point), and 3100 (Whitmore Lake).
> +
> +	  If unsure, say N.
> +
>  config GPIO_VX855
>  	tristate "VIA VX855/VX875 GPIO"
>  	depends on PCI
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index fa10df6..b538830 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_GPIO_PXA)		+= gpio-pxa.o
>  obj-$(CONFIG_GPIO_RDC321X)	+= gpio-rdc321x.o
>  obj-$(CONFIG_PLAT_SAMSUNG)	+= gpio-samsung.o
>  obj-$(CONFIG_ARCH_SA1100)	+= gpio-sa1100.o
> +obj-$(CONFIG_GPIO_ICH)		+= gpio-ich.o
>  obj-$(CONFIG_GPIO_SCH)		+= gpio-sch.o
>  obj-$(CONFIG_GPIO_STMPE)	+= gpio-stmpe.o
>  obj-$(CONFIG_GPIO_SX150X)	+= gpio-sx150x.o

This list is alphabetized, please keep it that way.

[...]
> +static struct platform_driver ichx_gpio_driver = {
> +	.driver		= {
> +		.owner	= THIS_MODULE,
> +		.name	= DRV_NAME,
> +	},
> +	.probe		= ichx_gpio_probe,
> +	.remove		= __devexit_p(ichx_gpio_remove),
> +};
> +
> +static int __devinit ichx_gpio_init_module(void)
> +{
> +	return platform_driver_register(&ichx_gpio_driver);
> +}
> +
> +static void __devexit ichx_gpio_exit_module(void)
> +{
> +	platform_driver_unregister(&ichx_gpio_driver);
> +}
> +
> +subsys_initcall(ichx_gpio_init_module);
> +module_exit(ichx_gpio_exit_module);

Don't use subsys_initcall.  Futzing with initcalls to get drivers in the
right order isn't sustainable.  I'm going to try to get the probe_defer
patches into v3.4 which is a better solution to the problem.

Also, you should use module_platform_driver() for trivial platform driver
registrations.

Otherwise looks good.

g.

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