> -----Original Message-----
> From: Lee Jones [mailto:lee.jo...@linaro.org]
> Sent: Tuesday, August 9, 2016 3:16 PM
> To: Tan, Jui Nee <jui.nee....@intel.com>
> Cc: mika.westerb...@linux.intel.com; heikki.kroge...@linux.intel.com;
> andriy.shevche...@linux.intel.com; t...@linutronix.de;
> mi...@redhat.com; h...@zytor.com; x...@kernel.org; pty...@xes-inc.com;
> linus.wall...@linaro.org; linux-g...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Yong, Jonathan <jonathan.y...@intel.com>; Yu,
> Ong Hock <ong.hock...@intel.com>; Voon, Weifeng
> <weifeng.v...@intel.com>; Wan Mohamad, Wan Ahmad Zainie
> <wan.ahmad.zainie.wan.moha...@intel.com>
> Subject: Re: [PATCH v6 3/3] mfd: lpc_ich: Add support for Intel Apollo Lake
> GPIO pinctrl in non-ACPI system
> 
> On Thu, 14 Jul 2016, Tan Jui Nee wrote:
> 
> > This driver uses the P2SB hide/unhide mechanism cooperatively to pass
> > the PCI BAR address to the gpio platform driver.
> >
> > Signed-off-by: Tan Jui Nee <jui.nee....@intel.com>
> > ---
> > Changes in V6:
> >     - Rename CONFIG_X86_INTEL_APL to CONFIG_X86_INTEL_IVI so
> that it
> >       relates to the actual product, as suggested by Mika.
> >     - Rework Makefile according Andy's comments.
> >     - Rename lpc_ich_misc() to lpc_ich_add_gpio() so that the name
> should not
> >       be so generic, as suggested by Andy.
> >     - Call lpc_ich_add_gpio() via priv->chipset.
> >     - lpc_ich_add_gpio() function will be moved from
> >       .../include/linux/mfd/lpc_ich.h to
> >       .../drivers/mfd/lpc_ich-apl.h
> >       as this is a part of internal driver interface as suggested by Andy.
> >     - Move enum lpc_chipsets from
> >       .../drivers/mfd/lpc_ich-core.c to
> >       .../include/linux/mfd/lpc_ich.h
> >       as lpc_chipsets is also accessed by lpc_ich_add_gpio().
> >     - Check if kasprintf return value for all 4 gpio controllers before
> >       proceed to add platform device by using mfd_add_devices().
> >
> > Changes in V5:
> >     - Split lpc-ich driver into two parts (lpc_ich-core and lpc_ich-apl).
> >       The file lpc_ich-apl.c introduces gpio platform driver in MFD.
> >     - Rename Kconfig option CONFIG_X86_INTEL_NON_ACPI to
> CONFIG_X86_INTEL_APL
> >       so that it reflects actual product as suggested by Mika.
> >
> > Changes in V4:
> >     - Move Kconfig option CONFIG_X86_INTEL_NON_ACPI from
> >       [PATCH 2/3] x86/platform/p2sb: New Primary to Sideband bridge
> support driver for Intel SOC's
> >       to
> >       [PATCH 3/3] mfd: lpc_ich: Add support for Intel Apollo Lake GPIO
> pinctrl in non-ACPI system
> >       since the config is used in latter patch.
> >     - Select CONFIG_P2SB when CONFIG_LPC_ICH is enabled.
> >     - Remove #ifdef CONFIG_X86_INTEL_NON_ACPI and use
> >       #if defined(CONFIG_X86_INTEL_NON_ACPI) when lpc_ich_misc is
> called
> >       as suggested by Lee Jones.
> >     - Use single dimensional array instead of 2D array for apl_gpio_io_res
> >       structure and use DEFINE_RES_IRQ for its IRQ resource.
> >
> > Changes in V3:
> >     - Simplify register addresses calculation and use
> DEFINE_RES_MEM_NAMED
> >       defines for apl_gpio_io_res structure
> >     - Define magic number for P2SB PCI ID
> >     - Replace switch-case with if-else since currently we have only one
> >       use case
> >     - Only call mfd_add_devices() once for all gpio communities
> >
> > Changes in V2:
> >     - Add new config option CONFIG_X86_INTEL_NON_ACPI and "select
> PINCTRL"
> >       to fix kbuildbot error
> >
> >  arch/x86/Kconfig            |   9 +++
> >  drivers/mfd/Kconfig         |   3 +-
> >  drivers/mfd/Makefile        |   5 +-
> >  drivers/mfd/lpc_ich-apl.c   | 130
> ++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/mfd/lpc_ich-apl.h   |  28 ++++++++++
> >  drivers/mfd/lpc_ich-core.c  |  81 ++++-----------------------
> > include/linux/mfd/lpc_ich.h |  73 +++++++++++++++++++++++++
> >  7 files changed, 256 insertions(+), 73 deletions(-)  create mode
> > 100644 drivers/mfd/lpc_ich-apl.c  create mode 100644
> > drivers/mfd/lpc_ich-apl.h
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
> > d305d81..c0b427b 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -513,6 +513,15 @@ config X86_INTEL_CE
> >       This option compiles in support for the CE4100 SOC for settop
> >       boxes and media devices.
> >
> > +config X86_INTEL_IVI
> > +   bool "Intel in-vehicle infotainment (IVI) systems used in cars"
> > +   select PINCTRL
> > +   ---help---
> > +     Select this option to enable MMIO BAR access over the P2SB for
> > +     non-ACPI Intel Apollo Lake SoC platforms. This driver uses the P2SB
> > +     hide/unhide mechanism cooperatively to pass the PCI BAR address
> to
> > +     the platform driver, currently GPIO.
> > +
> 
> This should be a separate patch.
> 
Thanks for your comment. The changes will be applied in next patch-set.
> >  config X86_INTEL_MID
> >     bool "Intel MID platform support"
> >     depends on X86_EXTENDED_PLATFORM
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index
> > 1bcf601..dc4e543 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -369,8 +369,9 @@ config MFD_INTEL_QUARK_I2C_GPIO
> >
> >  config LPC_ICH
> >     tristate "Intel ICH LPC"
> > -   depends on PCI
> > +   depends on X86 && PCI
> >     select MFD_CORE
> > +   select P2SB
> >     help
> >       The LPC bridge function of the Intel ICH provides support for
> >       many functional units. This driver provides needed support for
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index
> > 1dfe5fb..0aa3e1f 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -155,8 +155,11 @@ obj-$(CONFIG_PMIC_ADP5520)     += adp5520.o
> >  obj-$(CONFIG_MFD_KEMPLD)   += kempld-core.o
> >  obj-$(CONFIG_MFD_INTEL_QUARK_I2C_GPIO)     +=
> intel_quark_i2c_gpio.o
> >  obj-$(CONFIG_LPC_SCH)              += lpc_sch.o
> > -lpc_ich-objs               := lpc_ich-core.o
> >  obj-$(CONFIG_LPC_ICH)              += lpc_ich.o
> > +lpc_ich-objs               := lpc_ich-core.o
> > +ifeq ($(CONFIG_X86_INTEL_IVI),y)
> > +lpc_ich-objs += lpc_ich-apl.o
> > +endif
> >  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-apl.c b/drivers/mfd/lpc_ich-apl.c new
> > file mode 100644 index 0000000..e4b9688
> > --- /dev/null
> > +++ b/drivers/mfd/lpc_ich-apl.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * Purpose: Create a platform device to bind with Intel Apollo Lake
> 
> Never seen this before.  Looks like more of a commit log entry than
> something that should live in a source file header.
> 
> > + * Pinctrl GPIO platform driver
> 
> No need to mention "platform driver" here. Most drivers are.
> 
Okay, the changes will be applied in next patch-set.
> > + * Copyright (C) 2016 Intel Corporation
> 
> Author?
> 
I will add the Author information in next patch-set.
> > + * This program is free software; you can redistribute it and/or
> > + modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/pci.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/lpc_ich.h>
> > +#include <linux/pinctrl/pinctrl.h>
> > +#include <asm/p2sb.h>
> 
> Alphabetical.
> 
Thanks for pointing that out.
> > +#include "lpc_ich-apl.h"
> 
> Don't' mix '_'s with '-'s.
> 
The header file will be renamed as lpc_ich_apl.h.
> > +/* Offset data for Apollo Lake GPIO communities */
> > +#define APL_GPIO_SOUTHWEST_OFFSET  0xc00000
> > +#define APL_GPIO_NORTHWEST_OFFSET  0xc40000
> > +#define APL_GPIO_NORTH_OFFSET              0xc50000
> > +#define APL_GPIO_WEST_OFFSET               0xc70000
> > +
> > +#define APL_GPIO_SOUTHWEST_NPIN            43
> > +#define APL_GPIO_NORTHWEST_NPIN            77
> > +#define APL_GPIO_NORTH_NPIN                78
> > +#define APL_GPIO_WEST_NPIN         47
> > +
> > +#define APL_GPIO_IRQ 14
> > +
> > +#define PCI_IDSEL_P2SB     0x0d
> > +
> > +static struct resource apl_gpio_io_res[] = {
> > +   DEFINE_RES_MEM_NAMED(APL_GPIO_NORTH_OFFSET,
> > +           APL_GPIO_NORTH_NPIN * SZ_8, "apl_pinctrl_n"),
> > +   DEFINE_RES_MEM_NAMED(APL_GPIO_NORTHWEST_OFFSET,
> > +           APL_GPIO_NORTHWEST_NPIN * SZ_8, "apl_pinctrl_nw"),
> > +   DEFINE_RES_MEM_NAMED(APL_GPIO_WEST_OFFSET,
> > +           APL_GPIO_WEST_NPIN * SZ_8, "apl_pinctrl_w"),
> > +   DEFINE_RES_MEM_NAMED(APL_GPIO_SOUTHWEST_OFFSET,
> > +           APL_GPIO_SOUTHWEST_NPIN * SZ_8, "apl_pinctrl_sw"),
> > +   DEFINE_RES_IRQ(APL_GPIO_IRQ),
> > +};
> > +
> > +static struct pinctrl_pin_desc apl_pinctrl_pdata;
> 
> This seems pretty pointless.  What's it for?
> 
Since we do not need to access platform data at here, I have removed
the struct, .pdata_size and .platform_data (from below mfd_cell).
> > +static struct mfd_cell apl_gpio_devices[] = {
> > +   {
> > +           .name = "apl-pinctrl",
> > +           .id = 0,
> > +           .num_resources = ARRAY_SIZE(apl_gpio_io_res),
> > +           .resources = apl_gpio_io_res,
> > +           .pdata_size = sizeof(apl_pinctrl_pdata),
> > +           .platform_data = &apl_pinctrl_pdata,
> > +           .ignore_resource_conflicts = true,
> > +   },
> > +   {
> > +           .name = "apl-pinctrl",
> > +           .id = 1,
> > +           .num_resources = ARRAY_SIZE(apl_gpio_io_res),
> > +           .resources = apl_gpio_io_res,
> > +           .pdata_size = sizeof(apl_pinctrl_pdata),
> > +           .platform_data = &apl_pinctrl_pdata,
> > +           .ignore_resource_conflicts = true,
> > +   },
> > +   {
> > +           .name = "apl-pinctrl",
> > +           .id = 2,
> > +           .num_resources = ARRAY_SIZE(apl_gpio_io_res),
> > +           .resources = apl_gpio_io_res,
> > +           .pdata_size = sizeof(apl_pinctrl_pdata),
> > +           .platform_data = &apl_pinctrl_pdata,
> > +           .ignore_resource_conflicts = true,
> > +   },
> > +   {
> > +           .name = "apl-pinctrl",
> > +           .id = 3,
> > +           .num_resources = ARRAY_SIZE(apl_gpio_io_res),
> > +           .resources = apl_gpio_io_res,
> > +           .pdata_size = sizeof(apl_pinctrl_pdata),
> > +           .platform_data = &apl_pinctrl_pdata,
> > +           .ignore_resource_conflicts = true,
> > +   },
> > +};
> > +
> > +int lpc_ich_add_gpio(struct pci_dev *dev, enum lpc_chipsets chipset)
> > +{
> > +   unsigned int apl_p2sb = PCI_DEVFN(PCI_IDSEL_P2SB, 0);
> 
> You only use this variable one.  Just switch it out for the macro.
> 
Noted. The changes will be applied in next patch-set.
> > +   unsigned int i;
> > +   int ret;
> > +
> > +   if (chipset != LPC_APL)
> > +           return -ENODEV;
> > +   /*
> > +    * Apollo lake, has not 1, but 4 gpio controllers,
> > +    * handle it a bit differently.
> > +    */
> > +
> > +   for (i = 0; i < ARRAY_SIZE(apl_gpio_io_res)-1; i++) {
> 
> This is fragile.  Just define a GPIO_CONTROLLER_COUNT or similar.
> 
Okay.
> > +           struct resource *res = &apl_gpio_io_res[i];
> > +
> > +           apl_gpio_devices[i].resources = res;
> 
> What's happening here?  Are you manually pulling resources out of the
> resource structure and linking them to the device cells?
> 
> Why?
> 
There are total 4 GPIO controllers / communities and each of them has
different MMIO offset addresses. I have removed following line
        apl_gpio_devices[i].resources = res;
in my next patch-set and make modification at device cell, something
like below:
        static struct mfd_cell apl_gpio_devices[] = {
                {
                        .name = "apl-pinctrl",
                        .id = 0,
                        .num_resources = ARRAY_SIZE(apl_gpio_io_res),
                        .resources = &apl_gpio_io_res[0],
                        .ignore_resource_conflicts = true,
                },
        ...
> > +           /* Fill MEM resource */
> > +           ret = p2sb_bar(dev, apl_p2sb, res++);
> 
> Why res++?
> 
I have simplified it so that no need to call p2sb_bar() function four times
inside the for loop. And make p2sb_bar() function just to fill in the base
address into a scratch "struct resource" and have the loop do the additions
to base/end. Something like:
        struct resource base;
        
        ret = p2sb_bar(dev, PCI_DEVFN(PCI_IDSEL_P2SB, 0), &base);
        
        for (i = 0; i < ARRAY_SIZE(GPIO_CONTROLLER_COUNT); i++) {
                struct resource *res = apl_gpio_io_res[i];
                
                apl_gpio_devices[i].resources = res;
                
                /* Fill MEM resource */
                res->start += base.start;
                res->end += base.start;
                res->flags = base.flags;

                res++;
                ...
> > +           if (ret)
> > +                   goto warn_continue;
> > +
> > +           apl_pinctrl_pdata.name = kasprintf(GFP_KERNEL, "%u",
> > +                   i + 1);
> 
> Are you sure this does what you think it does?
> 
> 
> I think you will keep changing *.name, and the only one you will see/use is
> the last one.
> 
> Have you tested this?
> 
> > +           if (apl_pinctrl_pdata.name == NULL) {
> > +                   ret = -ENOMEM;
> > +                   goto warn_continue;
> 
> No, this is a failure.  You need to 'error' out and return ret.
> 
The entire apl_pinctrl_pdata.name memory allocation is no longer needed here
and tested working fine after remove.
> > +           }
> > +   }
> > +
> > +   ret = mfd_add_devices(&dev->dev, 0,
> > +           apl_gpio_devices, ARRAY_SIZE(apl_gpio_devices),
> > +                   NULL, 0, NULL);
> > +
> > +warn_continue:
> > +   if (ret)
> > +           dev_warn(&dev->dev,
> > +                   "Failed to add Apollo Lake GPIO %s: %d\n",
> > +                           apl_pinctrl_pdata.name, ret);
> > +
> > +   kfree(apl_pinctrl_pdata.name);
> 
> You've just passed the child device a NULL pointer.
> 
> > +   return 0;
> 
> Why are you returning 0 even during failure?
> 
It should be return ret. 
> > +}
> > diff --git a/drivers/mfd/lpc_ich-apl.h b/drivers/mfd/lpc_ich-apl.h new
> > file mode 100644 index 0000000..db8325d
> > --- /dev/null
> > +++ b/drivers/mfd/lpc_ich-apl.h
> > @@ -0,0 +1,28 @@
> > +/*
> > + * lpc_ich-apl.h - Intel in-vehicle infotainment (IVI) systems used in cars
> > + *                 support
> 
> The "In", "Vehicle" and "Infotainment" should be capitalised.
> 
Noted. The changes will be applied in next patch-set. 
> > + * Copyright (C) 2016, Intel Corporation
> > + *
> > + * Authors: Tan, Jui Nee <jui.nee....@intel.com>
> 
> "Author"
> 
Thanks for pointing that out. The changes will be applied in next patch-set.
> > + * This program is free software; you can redistribute it and/or
> > + modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __LPC_ICH_APL_H__
> > +#define __LPC_ICH_APL_H__
> > +
> > +#include <linux/pci.h>
> > +
> > +#if IS_ENABLED(CONFIG_X86_INTEL_IVI)
> > +int lpc_ich_add_gpio(struct pci_dev *dev, enum lpc_chipsets chipset);
> > +#else /* CONFIG_X86_INTEL_IVI is not set */ static inline int
> > +lpc_ich_add_gpio(struct pci_dev *dev, enum lpc_chipsets chipset) {
> > +   return -ENODEV;
> > +}
> > +#endif
> > +
> > +#endif
> > diff --git a/drivers/mfd/lpc_ich-core.c b/drivers/mfd/lpc_ich-core.c
> > index bd3aa45..e09d1e2 100644
> > --- a/drivers/mfd/lpc_ich-core.c
> > +++ b/drivers/mfd/lpc_ich-core.c
> > @@ -69,6 +69,8 @@
> >  #include <linux/mfd/lpc_ich.h>
> >  #include <linux/platform_data/itco_wdt.h>
> >
> > +#include "lpc_ich-apl.h"
> > +
> >  #define ACPIBASE           0x40
> >  #define ACPIBASE_GPE_OFF   0x28
> >  #define ACPIBASE_GPE_END   0x2f
> > @@ -147,77 +149,6 @@ static struct mfd_cell lpc_ich_gpio_cell = {
> >     .ignore_resource_conflicts = true,
> >  };
> >
> > -/* 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 */
> > -   LPC_LPT_LP,     /* Lynx Point-LP */
> > -   LPC_WBG,        /* Wellsburg */
> > -   LPC_AVN,        /* Avoton SoC */
> > -   LPC_BAYTRAIL,   /* Bay Trail SoC */
> > -   LPC_COLETO,     /* Coleto Creek */
> > -   LPC_WPT_LP,     /* Wildcat Point-LP */
> > -   LPC_BRASWELL,   /* Braswell SoC */
> > -   LPC_LEWISBURG,  /* Lewisburg */
> > -   LPC_9S,         /* 9 Series */
> > -};
> 
> What does this move have to do with this patch?
> 
> I think it should be separate and be paired with a reason for the change.
> 
Noted. I will place that in separate patch.
> >  static struct lpc_ich_info lpc_chipset_info[] = {
> >     [LPC_ICH] = {
> >             .name = "ICH",
> > @@ -531,6 +462,10 @@ static struct lpc_ich_info lpc_chipset_info[] = {
> >             .name = "9 Series",
> >             .iTCO_version = 2,
> >     },
> > +   [LPC_APL]  = {
> > +           .name = "Apollo Lake SoC",
> > +           .iTCO_version = 5,
> > +   },
> 
> Enabling a new platform should be a separate patch.
> 
Okay. I will place that in separate patch.
> >  };
> >
> >  /*
> > @@ -679,6 +614,7 @@ static const struct pci_device_id lpc_ich_ids[] = {
> >     { PCI_VDEVICE(INTEL, 0x3b14), LPC_3420},
> >     { PCI_VDEVICE(INTEL, 0x3b16), LPC_3450},
> >     { PCI_VDEVICE(INTEL, 0x5031), LPC_EP80579},
> > +   { PCI_VDEVICE(INTEL, 0x5ae8), LPC_APL},
> >     { PCI_VDEVICE(INTEL, 0x8c40), LPC_LPT},
> >     { PCI_VDEVICE(INTEL, 0x8c41), LPC_LPT},
> >     { PCI_VDEVICE(INTEL, 0x8c42), LPC_LPT}, @@ -1093,6 +1029,9 @@
> static
> > int lpc_ich_probe(struct pci_dev *dev,
> >                     cell_added = true;
> >     }
> >
> > +   if (!lpc_ich_add_gpio(dev, priv->chipset))
> > +           cell_added = true;
> > +
> >     /*
> >      * We only care if at least one or none of the cells registered
> >      * successfully.
> > diff --git a/include/linux/mfd/lpc_ich.h b/include/linux/mfd/lpc_ich.h
> > index 2b300b4..03c2ca3 100644
> > --- a/include/linux/mfd/lpc_ich.h
> > +++ b/include/linux/mfd/lpc_ich.h
> > @@ -34,6 +34,7 @@ enum {
> >     ICH_V10CORP_GPIO,
> >     ICH_V10CONS_GPIO,
> >     AVOTON_GPIO,
> > +   APL_GPIO,
> >  };
> >
> >  struct lpc_ich_info {
> > @@ -43,4 +44,76 @@ struct lpc_ich_info {
> >     u8 use_gpio;
> >  };
> >
> > +/* 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 */
> > +   LPC_LPT_LP,     /* Lynx Point-LP */
> > +   LPC_WBG,        /* Wellsburg */
> > +   LPC_AVN,        /* Avoton SoC */
> > +   LPC_BAYTRAIL,   /* Bay Trail SoC */
> > +   LPC_COLETO,     /* Coleto Creek */
> > +   LPC_WPT_LP,     /* Wildcat Point-LP */
> > +   LPC_BRASWELL,   /* Braswell SoC */
> > +   LPC_LEWISBURG,  /* Lewisburg */
> > +   LPC_9S,         /* 9 Series */
> > +   LPC_APL,        /* Apollo Lake SoC */
> > +};
> > +
> >  #endif
> 
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source
> software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog

Reply via email to