| 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
| ||
|
Message-ID: <1328296462.2261.193.camel@groeck-laptop>
Date: Fri, 3 Feb 2012 11:14:22 -0800
From: Guenter Roeck <guenter.roeck@...csson.com>
To: Aaron Sierra <asierra@...-inc.com>
CC: Jean Delvare <khali@...ux-fr.org>,
Grant Likely <grant.likely@...retlab.ca>,
LKML <linux-kernel@...r.kernel.org>,
Peter Tyser <ptyser@...-inc.com>
Subject: Re: [PATCH 1/3] mfd: Add LPC driver for Intel ICH chipsets
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 <asierra@...-inc.com>
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 <asierra@...-inc.com>
> + *
> + * 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 <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/acpi.h>
> +#include <linux/pci.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/lpc_ich.h>
> +
> +/* 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
--
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