From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754531Ab2BCTQO (ORCPT ); Fri, 3 Feb 2012 14:16:14 -0500 Received: from imr4.ericy.com ([198.24.6.9]:46993 "EHLO imr4.ericy.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753377Ab2BCTQL (ORCPT ); Fri, 3 Feb 2012 14:16:11 -0500 Message-ID: <1328296462.2261.193.camel@groeck-laptop> Subject: Re: [PATCH 1/3] mfd: Add LPC driver for Intel ICH chipsets From: Guenter Roeck Reply-To: guenter.roeck@ericsson.com To: Aaron Sierra CC: Jean Delvare , Grant Likely , LKML , Peter Tyser Date: Fri, 3 Feb 2012 11:14:22 -0800 In-Reply-To: <484bd1ed-f456-40b5-b5c2-c0e50f2787d5@zimbra> References: <484bd1ed-f456-40b5-b5c2-c0e50f2787d5@zimbra> Organization: Ericsson Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.2- Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2012-02-02 at 18:25 -0500, Aaron Sierra wrote: > This driver currently creates resources for use by a forthcoming ICH > chipset GPIO driver. It could be expanded to created the resources for > converting the esb2rom (mtd) and iTCO_wdt (wdt), and potentially more, > drivers to use the mfd model. > > Signed-off-by: Aaron Sierra Hi Aaron, looks pretty good. Couple of comments below. > --- > drivers/mfd/Kconfig | 9 + > drivers/mfd/Makefile | 1 + > drivers/mfd/lpc_ich.c | 531 +++++++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/lpc_ich.h | 33 +++ > 4 files changed, 574 insertions(+), 0 deletions(-) > create mode 100644 drivers/mfd/lpc_ich.c > create mode 100644 include/linux/mfd/lpc_ich.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 6ca938a..18eca82 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -636,6 +636,15 @@ config LPC_SCH > LPC bridge function of the Intel SCH provides support for > System Management Bus and General Purpose I/O. > > +config LPC_ICH > + tristate "Intel ICH LPC" > + depends on PCI > + select MFD_CORE > + help > + The LPC bridge function of the Intel ICH provides support for > + many functional units. This driver provides needed support for > + other drivers to control these functions, currently GPIO. > + > config MFD_RDC321X > tristate "Support for RDC-R321x southbridge" > select MFD_CORE > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index d7d47d2..d479882 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -83,6 +83,7 @@ obj-$(CONFIG_MFD_DB5500_PRCMU) += db5500-prcmu.o > obj-$(CONFIG_MFD_TIMBERDALE) += timberdale.o > obj-$(CONFIG_PMIC_ADP5520) += adp5520.o > obj-$(CONFIG_LPC_SCH) += lpc_sch.o > +obj-$(CONFIG_LPC_ICH) += lpc_ich.o > obj-$(CONFIG_MFD_RDC321X) += rdc321x-southbridge.o > obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o > obj-$(CONFIG_MFD_JZ4740_ADC) += jz4740-adc.o > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c > new file mode 100644 > index 0000000..a288c74 > --- /dev/null > +++ b/drivers/mfd/lpc_ich.c > @@ -0,0 +1,531 @@ > +/* > + * lpc_ich.c - LPC interface for Intel ICH > + * > + * LPC bridge function of the Intel ICH contains many other > + * functional units, such as Interrupt controllers, Timers, > + * Power Management, System Management, GPIO, RTC, and LPC > + * Configuration Registers. > + * > + * This driver is derived from lpc_sch. > + > + * Copyright (c) 2011 Extreme Engineering Solution, Inc. > + * Author: Aaron Sierra > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License 2 as published > + * by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; see the file COPYING. If not, write to > + * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. It might make sense to list the Intel document numbers, like iTCO_wdt.c. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Convenient wrapper to make our PCI ID table */ > +#define ICHX_PDEV(dev, idx) \ > + .vendor = PCI_VENDOR_ID_INTEL, \ > + .device = dev, \ > + .subvendor = PCI_ANY_ID, \ > + .subdevice = PCI_ANY_ID, \ > + .class = 0, \ > + .class_mask = 0, \ > + .driver_data = idx > + Later kernel versions have a macro PCI_VDEVICE which can be used instead (and is now used by iTCO_wdt.c). > +#define ACPIBASE 0x40 > +#define ACPIBASE_GPE_OFF 0x20 > +#define ACPIBASE_GPE_END 0x2f > +#define ACPICTRL 0x44 > + > +#define GPIOBASE 0x48 > +#define GPIOCTRL 0x4C > +#define GPIOBASE_IO_SIZE 0x80 > + > +static u8 lpc_ich_acpi_save, lpc_ich_gpio_save; > + > +static struct resource gpio_ich_res[] = { > + /* BASE */ > + { > + .flags = IORESOURCE_IO, > + }, > + /* ACPI */ > + { > + .flags = IORESOURCE_IO, > + }, > +}; > + > +static struct mfd_cell lpc_ich_cells[] = { > + { > + .name = "ich_gpio", > + .num_resources = ARRAY_SIZE(gpio_ich_res), > + .resources = gpio_ich_res, > + }, > +}; > + > +/* TCO related info */ s/TCO/chipset/ > +enum lpc_chipsets { > + LPC_ICH = 0, /* ICH */ > + LPC_ICH0, /* ICH0 */ > + LPC_ICH2, /* ICH2 */ > + LPC_ICH2M, /* ICH2-M */ > + LPC_ICH3, /* ICH3-S */ > + LPC_ICH3M, /* ICH3-M */ > + LPC_ICH4, /* ICH4 */ > + LPC_ICH4M, /* ICH4-M */ > + LPC_CICH, /* C-ICH */ > + LPC_ICH5, /* ICH5 & ICH5R */ > + LPC_6300ESB, /* 6300ESB */ > + LPC_ICH6, /* ICH6 & ICH6R */ > + LPC_ICH6M, /* ICH6-M */ > + LPC_ICH6W, /* ICH6W & ICH6RW */ > + LPC_631XESB, /* 631xESB/632xESB */ > + LPC_ICH7, /* ICH7 & ICH7R */ > + LPC_ICH7DH, /* ICH7DH */ > + LPC_ICH7M, /* ICH7-M & ICH7-U */ > + LPC_ICH7MDH, /* ICH7-M DH */ > + LPC_NM10, /* NM10 */ > + LPC_ICH8, /* ICH8 & ICH8R */ > + LPC_ICH8DH, /* ICH8DH */ > + LPC_ICH8DO, /* ICH8DO */ > + LPC_ICH8M, /* ICH8M */ > + LPC_ICH8ME, /* ICH8M-E */ > + LPC_ICH9, /* ICH9 */ > + LPC_ICH9R, /* ICH9R */ > + LPC_ICH9DH, /* ICH9DH */ > + LPC_ICH9DO, /* ICH9DO */ > + LPC_ICH9M, /* ICH9M */ > + LPC_ICH9ME, /* ICH9M-E */ > + LPC_ICH10, /* ICH10 */ > + LPC_ICH10R, /* ICH10R */ > + LPC_ICH10D, /* ICH10D */ > + LPC_ICH10DO, /* ICH10DO */ > + LPC_PCH, /* PCH Desktop Full Featured */ > + LPC_PCHM, /* PCH Mobile Full Featured */ > + LPC_P55, /* P55 */ > + LPC_PM55, /* PM55 */ > + LPC_H55, /* H55 */ > + LPC_QM57, /* QM57 */ > + LPC_H57, /* H57 */ > + LPC_HM55, /* HM55 */ > + LPC_Q57, /* Q57 */ > + LPC_HM57, /* HM57 */ > + LPC_PCHMSFF, /* PCH Mobile SFF Full Featured */ > + LPC_QS57, /* QS57 */ > + LPC_3400, /* 3400 */ > + LPC_3420, /* 3420 */ > + LPC_3450, /* 3450 */ > + LPC_EP80579, /* EP80579 */ > + LPC_CPT1, /* Cougar Point */ > + LPC_CPT2, /* Cougar Point Desktop */ > + LPC_CPT3, /* Cougar Point Mobile */ > + LPC_CPT4, /* Cougar Point */ > + LPC_CPT5, /* Cougar Point */ > + LPC_CPT6, /* Cougar Point */ > + LPC_CPT7, /* Cougar Point */ > + LPC_CPT8, /* Cougar Point */ > + LPC_CPT9, /* Cougar Point */ > + LPC_CPT10, /* Cougar Point */ > + LPC_CPT11, /* Cougar Point */ > + LPC_CPT12, /* Cougar Point */ > + LPC_CPT13, /* Cougar Point */ > + LPC_CPT14, /* Cougar Point */ > + LPC_CPT15, /* Cougar Point */ > + LPC_CPT16, /* Cougar Point */ > + LPC_CPT17, /* Cougar Point */ > + LPC_CPT18, /* Cougar Point */ > + LPC_CPT19, /* Cougar Point */ > + LPC_CPT20, /* Cougar Point */ > + LPC_CPT21, /* Cougar Point */ > + LPC_CPT22, /* Cougar Point */ > + LPC_CPT23, /* Cougar Point */ > + LPC_CPT24, /* Cougar Point */ > + LPC_CPT25, /* Cougar Point */ > + LPC_CPT26, /* Cougar Point */ > + LPC_CPT27, /* Cougar Point */ > + LPC_CPT28, /* Cougar Point */ > + LPC_CPT29, /* Cougar Point */ > + LPC_CPT30, /* Cougar Point */ > + LPC_CPT31, /* Cougar Point */ > + LPC_PBG1, /* Patsburg */ > + LPC_PBG2, /* Patsburg */ > + LPC_DH89XXCC, /* DH89xxCC */ > + LPC_PPT0, /* Panther Point */ > + LPC_PPT1, /* Panther Point */ > + LPC_PPT2, /* Panther Point */ > + LPC_PPT3, /* Panther Point */ > + LPC_PPT4, /* Panther Point */ > + LPC_PPT5, /* Panther Point */ > + LPC_PPT6, /* Panther Point */ > + LPC_PPT7, /* Panther Point */ > + LPC_PPT8, /* Panther Point */ > + LPC_PPT9, /* Panther Point */ > + LPC_PPT10, /* Panther Point */ > + LPC_PPT11, /* Panther Point */ > + LPC_PPT12, /* Panther Point */ > + LPC_PPT13, /* Panther Point */ > + LPC_PPT14, /* Panther Point */ > + LPC_PPT15, /* Panther Point */ > + LPC_PPT16, /* Panther Point */ > + LPC_PPT17, /* Panther Point */ > + LPC_PPT18, /* Panther Point */ > + LPC_PPT19, /* Panther Point */ > + LPC_PPT20, /* Panther Point */ > + LPC_PPT21, /* Panther Point */ > + LPC_PPT22, /* Panther Point */ > + LPC_PPT23, /* Panther Point */ > + LPC_PPT24, /* Panther Point */ > + LPC_PPT25, /* Panther Point */ > + LPC_PPT26, /* Panther Point */ > + LPC_PPT27, /* Panther Point */ > + LPC_PPT28, /* Panther Point */ > + LPC_PPT29, /* Panther Point */ > + LPC_PPT30, /* Panther Point */ > + LPC_PPT31, /* Panther Point */ > +}; > + I was always irritated by those duplicate defines and array entries in iTCO_wdt.c. Apparently someone else too, because it is gone in the latest version of iTCO_wdt.c. Should do the same here. > +struct lpc_ich_info lpc_chipset_info[] __devinitdata = { __devinitdata is not a good idea for a to-be-exported data structure. Though it does not need to be exported (see below), so that is really a moot point. > + {"ICH", 0}, > + {"ICH0", 0}, > + {"ICH2", 0}, > + {"ICH2-M", 0}, > + {"ICH3-S", 0}, > + {"ICH3-M", 0}, > + {"ICH4", 0}, > + {"ICH4-M", 0}, > + {"C-ICH", 0}, > + {"ICH5 or ICH5R", 0}, > + {"6300ESB", 0}, > + {"ICH6 or ICH6R", 0x0601}, > + {"ICH6-M", 0x0601}, > + {"ICH6W or ICH6RW", 0x0601}, > + {"631xESB/632xESB", 0x0601}, > + {"ICH7 or ICH7R", 0x0701}, > + {"ICH7DH", 0x0701}, > + {"ICH7-M or ICH7-U", 0x0701}, > + {"ICH7-M DH", 0x0701}, > + {"NM10", 0}, > + {"ICH8 or ICH8R", 0x0701}, > + {"ICH8DH", 0x0701}, > + {"ICH8DO", 0x0701}, > + {"ICH8M", 0x0701}, > + {"ICH8M-E", 0x0701}, > + {"ICH9", 0x0801}, > + {"ICH9R", 0x0801}, > + {"ICH9DH", 0x0801}, > + {"ICH9DO", 0x0801}, > + {"ICH9M", 0x0801}, > + {"ICH9M-E", 0x0801}, > + {"ICH10", 0x0a11}, > + {"ICH10R", 0x0a11}, > + {"ICH10D", 0x0a01}, > + {"ICH10DO", 0x0a01}, > + {"PCH Desktop Full Featured", 0x0501}, > + {"PCH Mobile Full Featured", 0x0501}, > + {"P55", 0x0501}, > + {"PM55", 0x0501}, > + {"H55", 0x0501}, > + {"QM57", 0x0501}, > + {"H57", 0x0501}, > + {"HM55", 0x0501}, > + {"Q57", 0x0501}, > + {"HM57", 0x0501}, > + {"PCH Mobile SFF Full Featured", 0x0501}, > + {"QS57", 0x0501}, > + {"3400", 0x0501}, > + {"3420", 0x0501}, > + {"3450", 0x0501}, > + {"EP80579", 0}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Cougar Point", 0x0501}, > + {"Patsburg", 0}, > + {"Patsburg", 0}, > + {"DH89xxCC", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {"Panther Point", 0}, > + {NULL, 0} The last (empty) entry is not needed here, since the table is indexed by enum lpc_chipsets. Also, it might be better to use explicit initialization such as [LPC_PPT] = {"Panther Point", 0}, since that would ensure that entries can not get out of sync. > +}; > +EXPORT_SYMBOL(lpc_chipset_info); > + No need to export the table. See below. > +static DEFINE_PCI_DEVICE_TABLE(lpc_ich_ids) = { > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801AA_0, LPC_ICH)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801AB_0, LPC_ICH0)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801BA_0, LPC_ICH2)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801BA_10, LPC_ICH2M)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801CA_0, LPC_ICH3)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801CA_12, LPC_ICH3M)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801DB_0, LPC_ICH4)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801DB_12, LPC_ICH4M)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801E_0, LPC_CICH)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_82801EB_0, LPC_ICH5)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ESB_1, LPC_6300ESB)}, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH6_0, LPC_ICH6) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH6_1, LPC_ICH6M) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH6_2, LPC_ICH6W) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ESB2_0, LPC_631XESB) }, The latest version if iTCO_wdt.c got rid of the symbolic defines. Maybe follow its lead. > + { ICHX_PDEV(0x2671, LPC_631XESB) }, > + { ICHX_PDEV(0x2672, LPC_631XESB) }, > + { ICHX_PDEV(0x2673, LPC_631XESB) }, > + { ICHX_PDEV(0x2674, LPC_631XESB) }, > + { ICHX_PDEV(0x2675, LPC_631XESB) }, > + { ICHX_PDEV(0x2676, LPC_631XESB) }, > + { ICHX_PDEV(0x2677, LPC_631XESB) }, > + { ICHX_PDEV(0x2678, LPC_631XESB) }, > + { ICHX_PDEV(0x2679, LPC_631XESB) }, > + { ICHX_PDEV(0x267a, LPC_631XESB) }, > + { ICHX_PDEV(0x267b, LPC_631XESB) }, > + { ICHX_PDEV(0x267c, LPC_631XESB) }, > + { ICHX_PDEV(0x267d, LPC_631XESB) }, > + { ICHX_PDEV(0x267e, LPC_631XESB) }, > + { ICHX_PDEV(0x267f, LPC_631XESB) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH7_0, LPC_ICH7) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH7_30, LPC_ICH7DH) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH7_1, LPC_ICH7M) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH7_31, LPC_ICH7MDH) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_TGP_LPC, LPC_NM10) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH8_0, LPC_ICH8) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH8_2, LPC_ICH8DH) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH8_3, LPC_ICH8DO) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH8_4, LPC_ICH8M) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH8_1, LPC_ICH8ME) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH9_8, LPC_ICH9) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH9_7, LPC_ICH9R) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH9_2, LPC_ICH9DH) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH9_4, LPC_ICH9DO) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH9_5, LPC_ICH9M) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH9_1, LPC_ICH9ME) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH10_2, LPC_ICH10) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH10_1, LPC_ICH10R) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH10_3, LPC_ICH10D) }, > + { ICHX_PDEV(PCI_DEVICE_ID_INTEL_ICH10_0, LPC_ICH10DO) }, > + { ICHX_PDEV(0x3b00, LPC_PCH) }, > + { ICHX_PDEV(0x3b01, LPC_PCHM) }, > + { ICHX_PDEV(0x3b02, LPC_P55) }, > + { ICHX_PDEV(0x3b03, LPC_PM55) }, > + { ICHX_PDEV(0x3b06, LPC_H55) }, > + { ICHX_PDEV(0x3b07, LPC_QM57) }, > + { ICHX_PDEV(0x3b08, LPC_H57) }, > + { ICHX_PDEV(0x3b09, LPC_HM55) }, > + { ICHX_PDEV(0x3b0a, LPC_Q57) }, > + { ICHX_PDEV(0x3b0b, LPC_HM57) }, > + { ICHX_PDEV(0x3b0d, LPC_PCHMSFF) }, > + { ICHX_PDEV(0x3b0f, LPC_QS57) }, > + { ICHX_PDEV(0x3b12, LPC_3400) }, > + { ICHX_PDEV(0x3b14, LPC_3420) }, > + { ICHX_PDEV(0x3b16, LPC_3450) }, > + { ICHX_PDEV(0x5031, LPC_EP80579) }, > + { ICHX_PDEV(0x1c41, LPC_CPT1) }, > + { ICHX_PDEV(0x1c42, LPC_CPT2) }, > + { ICHX_PDEV(0x1c43, LPC_CPT3) }, > + { ICHX_PDEV(0x1c44, LPC_CPT4) }, > + { ICHX_PDEV(0x1c45, LPC_CPT5) }, > + { ICHX_PDEV(0x1c46, LPC_CPT6) }, > + { ICHX_PDEV(0x1c47, LPC_CPT7) }, > + { ICHX_PDEV(0x1c48, LPC_CPT8) }, > + { ICHX_PDEV(0x1c49, LPC_CPT9) }, > + { ICHX_PDEV(0x1c4a, LPC_CPT10) }, > + { ICHX_PDEV(0x1c4b, LPC_CPT11) }, > + { ICHX_PDEV(0x1c4c, LPC_CPT12) }, > + { ICHX_PDEV(0x1c4d, LPC_CPT13) }, > + { ICHX_PDEV(0x1c4e, LPC_CPT14) }, > + { ICHX_PDEV(0x1c4f, LPC_CPT15) }, > + { ICHX_PDEV(0x1c50, LPC_CPT16) }, > + { ICHX_PDEV(0x1c51, LPC_CPT17) }, > + { ICHX_PDEV(0x1c52, LPC_CPT18) }, > + { ICHX_PDEV(0x1c53, LPC_CPT19) }, > + { ICHX_PDEV(0x1c54, LPC_CPT20) }, > + { ICHX_PDEV(0x1c55, LPC_CPT21) }, > + { ICHX_PDEV(0x1c56, LPC_CPT22) }, > + { ICHX_PDEV(0x1c57, LPC_CPT23) }, > + { ICHX_PDEV(0x1c58, LPC_CPT24) }, > + { ICHX_PDEV(0x1c59, LPC_CPT25) }, > + { ICHX_PDEV(0x1c5a, LPC_CPT26) }, > + { ICHX_PDEV(0x1c5b, LPC_CPT27) }, > + { ICHX_PDEV(0x1c5c, LPC_CPT28) }, > + { ICHX_PDEV(0x1c5d, LPC_CPT29) }, > + { ICHX_PDEV(0x1c5e, LPC_CPT30) }, > + { ICHX_PDEV(0x1c5f, LPC_CPT31) }, > + { ICHX_PDEV(0x1d40, LPC_PBG1) }, > + { ICHX_PDEV(0x1d41, LPC_PBG2) }, > + { ICHX_PDEV(0x2310, LPC_DH89XXCC) }, > + { ICHX_PDEV(0x1e40, LPC_PPT0) }, > + { ICHX_PDEV(0x1e41, LPC_PPT1) }, > + { ICHX_PDEV(0x1e42, LPC_PPT2) }, > + { ICHX_PDEV(0x1e43, LPC_PPT3) }, > + { ICHX_PDEV(0x1e44, LPC_PPT4) }, > + { ICHX_PDEV(0x1e45, LPC_PPT5) }, > + { ICHX_PDEV(0x1e46, LPC_PPT6) }, > + { ICHX_PDEV(0x1e47, LPC_PPT7) }, > + { ICHX_PDEV(0x1e48, LPC_PPT8) }, > + { ICHX_PDEV(0x1e49, LPC_PPT9) }, > + { ICHX_PDEV(0x1e4a, LPC_PPT10) }, > + { ICHX_PDEV(0x1e4b, LPC_PPT11) }, > + { ICHX_PDEV(0x1e4c, LPC_PPT12) }, > + { ICHX_PDEV(0x1e4d, LPC_PPT13) }, > + { ICHX_PDEV(0x1e4e, LPC_PPT14) }, > + { ICHX_PDEV(0x1e4f, LPC_PPT15) }, > + { ICHX_PDEV(0x1e50, LPC_PPT16) }, > + { ICHX_PDEV(0x1e51, LPC_PPT17) }, > + { ICHX_PDEV(0x1e52, LPC_PPT18) }, > + { ICHX_PDEV(0x1e53, LPC_PPT19) }, > + { ICHX_PDEV(0x1e54, LPC_PPT20) }, > + { ICHX_PDEV(0x1e55, LPC_PPT21) }, > + { ICHX_PDEV(0x1e56, LPC_PPT22) }, > + { ICHX_PDEV(0x1e57, LPC_PPT23) }, > + { ICHX_PDEV(0x1e58, LPC_PPT24) }, > + { ICHX_PDEV(0x1e59, LPC_PPT25) }, > + { ICHX_PDEV(0x1e5a, LPC_PPT26) }, > + { ICHX_PDEV(0x1e5b, LPC_PPT27) }, > + { ICHX_PDEV(0x1e5c, LPC_PPT28) }, > + { ICHX_PDEV(0x1e5d, LPC_PPT29) }, > + { ICHX_PDEV(0x1e5e, LPC_PPT30) }, > + { ICHX_PDEV(0x1e5f, LPC_PPT31) }, ITCO_wdt.c now also supports Lynx Point. > + { 0, }, > +}; > +MODULE_DEVICE_TABLE(pci, lpc_ich_ids); > + > +static int __devinit lpc_ich_probe(struct pci_dev *dev, > + const struct pci_device_id *id) > +{ > + u32 base_addr_cfg; > + u32 base_addr; > + int i; > + > + /* Setup power management base register */ > + pci_read_config_dword(dev, ACPIBASE, &base_addr_cfg); > + base_addr = base_addr_cfg & 0x0000ff80; > + if (!base_addr) { > + dev_err(&dev->dev, "I/O space for ACPI uninitialized\n"); > + return -ENODEV; > + } > + > + gpio_ich_res[ICH_RES_GPE0].start = base_addr + ACPIBASE_GPE_OFF; > + gpio_ich_res[ICH_RES_GPE0].end = base_addr + ACPIBASE_GPE_END; > + > + /* Enable LPC ACPI space */ > + pci_read_config_byte(dev, ACPICTRL, &lpc_ich_acpi_save); > + pci_write_config_byte(dev, ACPICTRL, lpc_ich_acpi_save | 0x10); > + > + /* Setup GPIO base register */ > + pci_read_config_dword(dev, GPIOBASE, &base_addr_cfg); > + base_addr = base_addr_cfg & 0x0000ff80; > + if (!base_addr) { > + dev_err(&dev->dev, "I/O space for GPIO uninitialized\n"); > + return -ENODEV; > + } > + > + gpio_ich_res[ICH_RES_GPIO].start = base_addr; > + gpio_ich_res[ICH_RES_GPIO].end = base_addr + GPIOBASE_IO_SIZE - 1; > + > + /* Enable LPC GPIO space */ > + pci_read_config_byte(dev, GPIOCTRL, &lpc_ich_gpio_save); > + pci_write_config_byte(dev, GPIOCTRL, lpc_ich_gpio_save | 0x10); > + > + for (i=0; i < ARRAY_SIZE(lpc_ich_cells); i++) checkpatch error (should be 'i = 0'). > + lpc_ich_cells[i].id = id->driver_data; > + It would be better to use lpc_ich_cells[i].platform_data = &lpc_chipset_info[id->driver_data]; lpc_ich_cells[i].pdata_size = sizeof(struct lpc_ich_info); ie pass the structure in the cell's platform_data. This way you can avoid the need to export lpc_chipset_info, and you don't have to keep it around either. You don't need mfd_cell.platform_data to pass the pci_device pointer, since the child driver can get it using "to_pci_dev(platform_device->dev.parent)". I ended up making all the changes suggested above (plus the matching changes needed in the subsequent patches) while I rebased the code to 3.3-rc2. Right now I have an (untested) patchset which applies to 3.3-rc2. Please let me know how you would like to proceed; I can send it to you if you like, or to the list. Thanks, Guenter