All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <guenter.roeck@ericsson.com>
To: Aaron Sierra <asierra@xes-inc.com>
Cc: Jean Delvare <khali@linux-fr.org>,
	Grant Likely <grant.likely@secretlab.ca>,
	LKML <linux-kernel@vger.kernel.org>,
	Peter Tyser <ptyser@xes-inc.com>
Subject: Re: [PATCH 1/3 v2] mfd: Add LPC driver for Intel ICH chipsets
Date: Tue, 7 Feb 2012 12:15:17 -0800	[thread overview]
Message-ID: <1328645717.2261.286.camel@groeck-laptop> (raw)
In-Reply-To: <19991da0-d4ad-4a62-9ce0-6b7c8d5b7c24@zimbra>

Hi Aaron,

On Tue, 2012-02-07 at 14:56 -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@xes-inc.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

Not sure how to handle this since some of the code is from me ....
usually one doesn't add other people's Signed-off unless both are in the
commit chain. Comments, anyone ?

Anyway, couple of comments below. 

>  drivers/mfd/Kconfig         |    9 +
>  drivers/mfd/Makefile        |    1 +
>  drivers/mfd/lpc_ich.c       |  510 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/lpc_ich.h |   32 +++
>  4 files changed, 552 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 cd13e9f..f581e59 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -724,6 +724,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 b953bab..6636c5a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -98,6 +98,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..3f159fc
> --- /dev/null
> +++ b/drivers/mfd/lpc_ich.c
> @@ -0,0 +1,510 @@
> +/*
> + *  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@xes-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.
> + *
> + *  This driver supports the following I/O Controller hubs:
> + *     (See the intel documentation on http://developer.intel.com.)
> + *     document number 290655-003, 290677-014: 82801AA (ICH), 82801AB (ICHO)
> + *     document number 290687-002, 298242-027: 82801BA (ICH2)
> + *     document number 290733-003, 290739-013: 82801CA (ICH3-S)
> + *     document number 290716-001, 290718-007: 82801CAM (ICH3-M)
> + *     document number 290744-001, 290745-025: 82801DB (ICH4)
> + *     document number 252337-001, 252663-008: 82801DBM (ICH4-M)
> + *     document number 273599-001, 273645-002: 82801E (C-ICH)
> + *     document number 252516-001, 252517-028: 82801EB (ICH5), 82801ER (ICH5R)
> + *     document number 300641-004, 300884-013: 6300ESB
> + *     document number 301473-002, 301474-026: 82801F (ICH6)
> + *     document number 313082-001, 313075-006: 631xESB, 632xESB
> + *     document number 307013-003, 307014-024: 82801G (ICH7)
> + *     document number 322896-001, 322897-001: NM10
> + *     document number 313056-003, 313057-017: 82801H (ICH8)
> + *     document number 316972-004, 316973-012: 82801I (ICH9)
> + *     document number 319973-002, 319974-002: 82801J (ICH10)
> + *     document number 322169-001, 322170-003: 5 Series, 3400 Series (PCH)
> + *     document number 320066-003, 320257-008: EP80597 (IICH)
> + *     document number 324645-001, 324646-001: Cougar Point (CPT)
> + *     document number TBD : Patsburg (PBG)
> + *     document number TBD : DH89xxCC
> + *     document number TBD : Panther Point
> + *     document number TBD : Lynx Point
> + */
> +
> +#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>
> +
> +#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 int lpc_ich_acpi_save = -1;
> +static int lpc_ich_gpio_save = -1;
> +
> +static struct resource gpio_ich_res[] = {
> +       /* BASE */
> +       {
> +               .flags = IORESOURCE_IO,
> +       },
> +       /* ACPI */
> +       {
> +               .flags = IORESOURCE_IO,
> +       },
> +};
> +
> +enum lpc_cells {
> +       LPC_GPIO = 0,
> +};
> +
> +static struct mfd_cell lpc_ich_cells[] = {
> +       [LPC_GPIO] = {
> +               .name = "gpio_ich",
> +               .num_resources = ARRAY_SIZE(gpio_ich_res),
> +               .resources = gpio_ich_res,
> +       },
> +};
> +
> +/* chipset related info */
> +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_CPT,        /* Cougar Point */
> +       LPC_CPTD,       /* Cougar Point Desktop */
> +       LPC_CPTM,       /* Cougar Point Mobile */
> +       LPC_PBG,        /* Patsburg */
> +       LPC_DH89XXCC,   /* DH89xxCC */
> +       LPC_PPT,        /* Panther Point */
> +       LPC_LPT,        /* Lynx Point */
> +};
> +
> +struct lpc_ich_info lpc_chipset_info[] __devinitdata = {
> +       [LPC_ICH] = {"ICH",                             0},
> +       [LPC_ICH0] = {"ICH0",                           0},
> +       [LPC_ICH2] = {"ICH2",                           0},
> +       [LPC_ICH2M] = {"ICH2-M",                        0},
> +       [LPC_ICH3] = {"ICH3-S",                         0},
> +       [LPC_ICH3M] = {"ICH3-M",                        0},
> +       [LPC_ICH4] = {"ICH4",                           0},
> +       [LPC_ICH4M] = {"ICH4-M",                        0},
> +       [LPC_CICH] = {"C-ICH",                          0},
> +       [LPC_ICH5] = {"ICH5 or ICH5R",                  0},
> +       [LPC_6300ESB] = {"6300ESB",                     0},
> +       [LPC_ICH6] = {"ICH6 or ICH6R",                  0x0601},
> +       [LPC_ICH6M] = {"ICH6-M",                        0x0601},
> +       [LPC_ICH6W] = {"ICH6W or ICH6RW",               0x0601},
> +       [LPC_631XESB] = {"631xESB/632xESB",             0x0601},
> +       [LPC_ICH7] = {"ICH7 or ICH7R",                  0x0701},
> +       [LPC_ICH7DH] = {"ICH7DH",                       0x0701},
> +       [LPC_ICH7M] = {"ICH7-M or ICH7-U",              0x0701},
> +       [LPC_ICH7MDH] = {"ICH7-M DH",                   0x0701},
> +       [LPC_NM10] = {"NM10",                           0},
> +       [LPC_ICH8] = {"ICH8 or ICH8R",                  0x0701},
> +       [LPC_ICH8DH] = {"ICH8DH",                       0x0701},
> +       [LPC_ICH8DO] = {"ICH8DO",                       0x0701},
> +       [LPC_ICH8M] = {"ICH8M",                         0x0701},
> +       [LPC_ICH8ME] = {"ICH8M-E",                      0x0701},
> +       [LPC_ICH9] = {"ICH9",                           0x0801},
> +       [LPC_ICH9R] = {"ICH9R",                         0x0801},
> +       [LPC_ICH9DH] = {"ICH9DH",                       0x0801},
> +       [LPC_ICH9DO] = {"ICH9DO",                       0x0801},
> +       [LPC_ICH9M] = {"ICH9M",                         0x0801},
> +       [LPC_ICH9ME] = {"ICH9M-E",                      0x0801},
> +       [LPC_ICH10] = {"ICH10",                         0x0a11},
> +       [LPC_ICH10R] = {"ICH10R",                       0x0a11},
> +       [LPC_ICH10D] = {"ICH10D",                       0x0a01},
> +       [LPC_ICH10DO] = {"ICH10DO",                     0x0a01},
> +       [LPC_PCH] = {"PCH Desktop Full Featured",       0x0501},
> +       [LPC_PCHM] = {"PCH Mobile Full Featured",       0x0501},
> +       [LPC_P55] = {"P55",                             0x0501},
> +       [LPC_PM55] = {"PM55",                           0x0501},
> +       [LPC_H55] = {"H55",                             0x0501},
> +       [LPC_QM57] = {"QM57",                           0x0501},
> +       [LPC_H57] = {"H57",                             0x0501},
> +       [LPC_HM55] = {"HM55",                           0x0501},
> +       [LPC_Q57] = {"Q57",                             0x0501},
> +       [LPC_HM57] = {"HM57",                           0x0501},
> +       [LPC_PCHMSFF] = {"PCH Mobile SFF Full Featured",0x0501},
> +       [LPC_QS57] = {"QS57",                           0x0501},
> +       [LPC_3400] = {"3400",                           0x0501},
> +       [LPC_3420] = {"3420",                           0x0501},
> +       [LPC_3450] = {"3450",                           0x0501},
> +       [LPC_EP80579] = {"EP80579",                     0},
> +       [LPC_CPT] = {"Cougar Point",                    0x0501},
> +       [LPC_CPTD] = {"Cougar Point Desktop",           0x0501},
> +       [LPC_CPTM] = {"Cougar Point Mobile",            0x0501},
> +       [LPC_PBG] = {"Patsburg",                        0},
> +       [LPC_DH89XXCC] = {"DH89xxCC",                   0},
> +       [LPC_PPT] = {"Panther Point",                   0},
> +       [LPC_LPT] = {"Lynx Point",                      0},
> +};
> +
> +/*
> + * This data only exists for exporting the supported PCI ids
> + * via MODULE_DEVICE_TABLE.  We do not actually register a
> + * pci_driver, because the I/O Controller Hub has also other
> + * functions that probably will be registered by other drivers.
> + */
> +static DEFINE_PCI_DEVICE_TABLE(lpc_ich_ids) = {
> +       { PCI_VDEVICE(INTEL, 0x2410), LPC_ICH},
> +       { PCI_VDEVICE(INTEL, 0x2420), LPC_ICH0},
> +       { PCI_VDEVICE(INTEL, 0x2440), LPC_ICH2},
> +       { PCI_VDEVICE(INTEL, 0x244c), LPC_ICH2M},
> +       { PCI_VDEVICE(INTEL, 0x2480), LPC_ICH3},
> +       { PCI_VDEVICE(INTEL, 0x248c), LPC_ICH3M},
> +       { PCI_VDEVICE(INTEL, 0x24c0), LPC_ICH4},
> +       { PCI_VDEVICE(INTEL, 0x24cc), LPC_ICH4M},
> +       { PCI_VDEVICE(INTEL, 0x2450), LPC_CICH},
> +       { PCI_VDEVICE(INTEL, 0x24d0), LPC_ICH5},
> +       { PCI_VDEVICE(INTEL, 0x25a1), LPC_6300ESB},
> +       { PCI_VDEVICE(INTEL, 0x2640), LPC_ICH6},
> +       { PCI_VDEVICE(INTEL, 0x2641), LPC_ICH6M},
> +       { PCI_VDEVICE(INTEL, 0x2642), LPC_ICH6W},
> +       { PCI_VDEVICE(INTEL, 0x2670), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2671), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2672), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2673), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2674), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2675), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2676), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2677), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2678), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x2679), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x267a), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x267b), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x267c), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x267d), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x267e), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x267f), LPC_631XESB},
> +       { PCI_VDEVICE(INTEL, 0x27b8), LPC_ICH7},
> +       { PCI_VDEVICE(INTEL, 0x27b0), LPC_ICH7DH},
> +       { PCI_VDEVICE(INTEL, 0x27b9), LPC_ICH7M},
> +       { PCI_VDEVICE(INTEL, 0x27bd), LPC_ICH7MDH},
> +       { PCI_VDEVICE(INTEL, 0x27bc), LPC_NM10},
> +       { PCI_VDEVICE(INTEL, 0x2810), LPC_ICH8},
> +       { PCI_VDEVICE(INTEL, 0x2812), LPC_ICH8DH},
> +       { PCI_VDEVICE(INTEL, 0x2814), LPC_ICH8DO},
> +       { PCI_VDEVICE(INTEL, 0x2815), LPC_ICH8M},
> +       { PCI_VDEVICE(INTEL, 0x2811), LPC_ICH8ME},
> +       { PCI_VDEVICE(INTEL, 0x2918), LPC_ICH9},
> +       { PCI_VDEVICE(INTEL, 0x2916), LPC_ICH9R},
> +       { PCI_VDEVICE(INTEL, 0x2912), LPC_ICH9DH},
> +       { PCI_VDEVICE(INTEL, 0x2914), LPC_ICH9DO},
> +       { PCI_VDEVICE(INTEL, 0x2919), LPC_ICH9M},
> +       { PCI_VDEVICE(INTEL, 0x2917), LPC_ICH9ME},
> +       { PCI_VDEVICE(INTEL, 0x3a18), LPC_ICH10},
> +       { PCI_VDEVICE(INTEL, 0x3a16), LPC_ICH10R},
> +       { PCI_VDEVICE(INTEL, 0x3a1a), LPC_ICH10D},
> +       { PCI_VDEVICE(INTEL, 0x3a14), LPC_ICH10DO},
> +       { PCI_VDEVICE(INTEL, 0x3b00), LPC_PCH},
> +       { PCI_VDEVICE(INTEL, 0x3b01), LPC_PCHM},
> +       { PCI_VDEVICE(INTEL, 0x3b02), LPC_P55},
> +       { PCI_VDEVICE(INTEL, 0x3b03), LPC_PM55},
> +       { PCI_VDEVICE(INTEL, 0x3b06), LPC_H55},
> +       { PCI_VDEVICE(INTEL, 0x3b07), LPC_QM57},
> +       { PCI_VDEVICE(INTEL, 0x3b08), LPC_H57},
> +       { PCI_VDEVICE(INTEL, 0x3b09), LPC_HM55},
> +       { PCI_VDEVICE(INTEL, 0x3b0a), LPC_Q57},
> +       { PCI_VDEVICE(INTEL, 0x3b0b), LPC_HM57},
> +       { PCI_VDEVICE(INTEL, 0x3b0d), LPC_PCHMSFF},
> +       { PCI_VDEVICE(INTEL, 0x3b0f), LPC_QS57},
> +       { PCI_VDEVICE(INTEL, 0x3b12), LPC_3400},
> +       { PCI_VDEVICE(INTEL, 0x3b14), LPC_3420},
> +       { PCI_VDEVICE(INTEL, 0x3b16), LPC_3450},
> +       { PCI_VDEVICE(INTEL, 0x5031), LPC_EP80579},
> +       { PCI_VDEVICE(INTEL, 0x1c41), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c42), LPC_CPTD},
> +       { PCI_VDEVICE(INTEL, 0x1c43), LPC_CPTM},
> +       { PCI_VDEVICE(INTEL, 0x1c44), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c45), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c46), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c47), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c48), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c49), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c4a), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c4b), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c4c), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c4d), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c4e), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c4f), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c50), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c51), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c52), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c53), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c54), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c55), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c56), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c57), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c58), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c59), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c5a), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c5b), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c5c), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c5d), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c5e), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1c5f), LPC_CPT},
> +       { PCI_VDEVICE(INTEL, 0x1d40), LPC_PBG},
> +       { PCI_VDEVICE(INTEL, 0x1d41), LPC_PBG},
> +       { PCI_VDEVICE(INTEL, 0x2310), LPC_DH89XXCC},
> +       { PCI_VDEVICE(INTEL, 0x1e40), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e41), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e42), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e43), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e44), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e45), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e46), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e47), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e48), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e49), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e4a), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e4b), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e4c), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e4d), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e4e), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e4f), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e50), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e51), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e52), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e53), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e54), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e55), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e56), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e57), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e58), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e59), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e5a), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e5b), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e5c), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e5d), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e5e), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x1e5f), LPC_PPT},
> +       { PCI_VDEVICE(INTEL, 0x8c40), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c41), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c42), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c43), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c44), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c45), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c46), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c47), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c48), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c49), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c4a), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c4b), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c4c), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c4d), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c4e), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c4f), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c50), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c51), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c52), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c53), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c54), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c55), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c56), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c57), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c58), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c59), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c5a), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c5b), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c5c), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c5d), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c5e), LPC_LPT},
> +       { PCI_VDEVICE(INTEL, 0x8c5f), LPC_LPT},
> +       { 0, },                 /* End of list */
> +};
> +MODULE_DEVICE_TABLE(pci, lpc_ich_ids);
> +
> +static void lpc_ich_finalize_cell(struct mfd_cell *cell,
> +                                       const struct pci_device_id *id)
> +{
> +       cell->id = id->driver_data;
> +       cell->platform_data = &lpc_chipset_info[id->driver_data];
> +       cell->pdata_size = sizeof(struct lpc_ich_info);
> +}
> +
> +static int __devinit lpc_ich_probe(struct pci_dev *dev,
> +                               const struct pci_device_id *id)
> +{
> +       u32 base_addr_cfg;
> +       u32 base_addr;
> +       u8 reg_save;
> +       int ret;
> +       int cells = 0;
> +       int acpi_conflict = 0;
> +
You can use bool for acpi_conflict (and cells, but I don't think that is
needed anyway).

> +       /* 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");
> +               goto pm_done;
> +       }
> +
> +       gpio_ich_res[ICH_RES_GPE0].start = base_addr + ACPIBASE_GPE_OFF;
> +       gpio_ich_res[ICH_RES_GPE0].end = base_addr + ACPIBASE_GPE_END;
> +       ret = acpi_check_resource_conflict(&gpio_ich_res[ICH_RES_GPE0]);
> +       if (ret) {
> +               /* this isn't necessarily fatal for the GPIO */
> +               gpio_ich_res[ICH_RES_GPE0].start = 0;
> +               gpio_ich_res[ICH_RES_GPE0].end = 0;
> +               acpi_conflict++;

		acpi_conflict = true;

> +       }
> +
> +       /* Enable LPC ACPI space */
> +       pci_read_config_byte(dev, ACPICTRL, &reg_save);
> +       pci_write_config_byte(dev, ACPICTRL, reg_save | 0x10);
> +       lpc_ich_acpi_save = (int)reg_save;
> +
Unnecessary typecast.

> +pm_done:
> +       /* 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");
> +               /* GPIO in power-management space may still be available */
> +               goto gpio_reg;
> +       }
> +
> +       gpio_ich_res[ICH_RES_GPIO].start = base_addr;
> +       gpio_ich_res[ICH_RES_GPIO].end = base_addr + GPIOBASE_IO_SIZE - 1;
> +       ret = acpi_check_resource_conflict(&gpio_ich_res[ICH_RES_GPIO]);
> +       if (ret) {
> +               /* this isn't necessarily fatal for the GPIO */
> +               gpio_ich_res[ICH_RES_GPIO].start = 0;
> +               gpio_ich_res[ICH_RES_GPIO].end = 0;
> +               acpi_conflict++;

	acpi_conflict = true;

After all, it does not really matter how many conflicts were detected.

> +               goto gpio_reg;
> +       }
> +
> +       /* Enable LPC GPIO space */
> +       pci_read_config_byte(dev, GPIOCTRL, &reg_save);
> +       pci_write_config_byte(dev, GPIOCTRL, reg_save | 0x10);
> +       lpc_ich_gpio_save = (int)reg_save;
> +

That typecast is unnecessary.

> +gpio_reg:
> +       lpc_ich_finalize_cell(&lpc_ich_cells[LPC_GPIO], id);
> +       ret = mfd_add_devices(&dev->dev, 0, &lpc_ich_cells[LPC_GPIO],
> +                               1, NULL, 0);
> +       if (!ret)
> +               cells++;
> +
So if there is an error, ret < 0, correct ?

> +       if (acpi_conflict)
> +               dev_info(&dev->dev, "ACPI resource conflicts found; "
> +                               "consider using acpi_enforce_resources=lax?\n");
> +
> +       if (cells)
> +               return 0;
> +       else
> +               return -ENODEV;

If the above is true, you can just return ret, and you don't need the
cells variable. Or, even better, move the acpi warning above the call to
mfd_add_devices().

> +}
> +
> +static void __devexit lpc_ich_remove(struct pci_dev *dev)
> +{
> +       mfd_remove_devices(&dev->dev);
> +
> +       pci_write_config_byte(dev, GPIOCTRL, lpc_ich_gpio_save);
> +       pci_write_config_byte(dev, ACPICTRL, lpc_ich_acpi_save);
> +}
> +
> +static struct pci_driver lpc_ich_driver = {
> +       .name           = "lpc_ich",
> +       .id_table       = lpc_ich_ids,
> +       .probe          = lpc_ich_probe,
> +       .remove         = __devexit_p(lpc_ich_remove),
> +};
> +
> +static int __init lpc_ich_init(void)
> +{
> +       return pci_register_driver(&lpc_ich_driver);
> +}
> +
> +static void __exit lpc_ich_exit(void)
> +{
> +       pci_unregister_driver(&lpc_ich_driver);
> +}
> +
> +module_init(lpc_ich_init);
> +module_exit(lpc_ich_exit);
> +
> +MODULE_AUTHOR("Aaron Sierra <asierra@xes-inc.com>");
> +MODULE_DESCRIPTION("LPC interface for Intel ICH");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
> new file mode 100644
> index 0000000..286c778
> --- /dev/null
> +++ b/include/linux/mfd/lpc_ich.h
> @@ -0,0 +1,32 @@
> +/*
> + *  linux/drivers/mfd/lpc_ich.h
> + *
> + *  Copyright (c) 2012 Extreme Engineering Solution, Inc.
> + *  Author: Aaron Sierra <asierra@xes-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.
> + */
> +#ifndef LPC_ICH_H
> +#define LPC_ICH_H
> +
> +/* GPIO resources */
> +#define ICH_RES_GPIO   0
> +#define ICH_RES_GPE0   1
> +
> +struct lpc_ich_info {
> +       char name[32];
> +       unsigned int gpio_version;
> +};
> +
> +#endif
> --
> 1.7.0.4



  reply	other threads:[~2012-02-07 20:17 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-19 12:53 [PATCH] gpio: New driver for the Intel 82801 (ICH) GPIO pins Jean Delvare
2011-04-19 14:44 ` Grant Likely
2011-04-19 14:54   ` Alan Cox
2011-04-19 15:05     ` Grant Likely
2011-04-19 15:57       ` Alan Cox
2011-04-19 16:40         ` Anton Vorontsov
2011-04-19 17:08           ` Alan Cox
2011-04-19 20:30             ` Anton Vorontsov
2011-04-19 21:16               ` Alan Cox
2011-04-19 21:20                 ` Alan Cox
2011-04-23 13:45   ` Jean Delvare
2011-04-23 14:47     ` Alan Cox
2011-05-19 11:33       ` Jean Delvare
2011-05-27  3:09 ` Grant Likely
2012-02-02  2:31 ` Guenter Roeck
2012-02-02  7:49   ` Jean Delvare
2012-02-02 17:35     ` Guenter Roeck
2012-02-02 19:56       ` Peter Tyser
2012-02-02 22:02         ` Guenter Roeck
2012-02-02 23:25           ` [PATCH 1/3] mfd: Add LPC driver for Intel ICH chipsets Aaron Sierra
2012-02-03  6:43             ` Guenter Roeck
2012-02-03 15:34               ` Aaron Sierra
2012-02-03 19:14             ` Guenter Roeck
2012-02-03 19:35               ` Aaron Sierra
2012-02-03 19:45                 ` Guenter Roeck
2012-02-03 22:50                   ` Aaron Sierra
2012-02-04  8:45                     ` Jean Delvare
2012-02-04 16:45                       ` Guenter Roeck
2012-02-07 19:56                         ` [PATCH 1/3 v2] " Aaron Sierra
2012-02-07 20:15                           ` Guenter Roeck [this message]
2012-02-07 20:31                             ` Jean Delvare
2012-02-07 20:43                               ` Guenter Roeck
2012-02-07 21:00                             ` Aaron Sierra
2012-02-07 21:09                               ` Guenter Roeck
2012-02-02 23:27           ` [PATCH 2/3] gpio: Add support for Intel ICHx/3100/Series[56] GPIO Aaron Sierra
2012-02-03 20:19             ` Guenter Roeck
2012-02-07 19:58               ` [PATCH 2/3 v2] " Aaron Sierra
2012-02-07 20:42                 ` Guenter Roeck
2012-02-07 22:07                 ` Jean Delvare
2012-02-07 23:25                   ` Aaron Sierra
2012-02-02 23:29           ` [PATCH 3/3] watchdog: Convert iTCO_wdt driver to mfd model Aaron Sierra
2012-02-07 19:59             ` [PATCH 3/3 v2] " Aaron Sierra
2012-02-07 21:07               ` Guenter Roeck
2012-02-08 17:48                 ` Aaron Sierra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1328645717.2261.286.camel@groeck-laptop \
    --to=guenter.roeck@ericsson.com \
    --cc=asierra@xes-inc.com \
    --cc=grant.likely@secretlab.ca \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ptyser@xes-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.