| 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
| ||
|
Date: Wed, 28 Sep 2016 09:51:19 +0000
From: "Tan, Jui Nee" <jui.nee.tan@...el.com>
To: 'Lee Jones' <lee.jones@...aro.org>
CC: "mika.westerberg@...ux.intel.com" <mika.westerberg@...ux.intel.com>,
"heikki.krogerus@...ux.intel.com" <heikki.krogerus@...ux.intel.com>,
"andriy.shevchenko@...ux.intel.com"
<andriy.shevchenko@...ux.intel.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
"ptyser@...-inc.com" <ptyser@...-inc.com>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Yong, Jonathan" <jonathan.yong@...el.com>,
"Yu, Ong Hock" <ong.hock.yu@...el.com>,
"Wan Mohamad, Wan Ahmad Zainie"
<wan.ahmad.zainie.wan.mohamad@...el.com>,
"Luck, Tony" <tony.luck@...el.com>
Subject: RE: [PATCH v6 3/3] mfd: lpc_ich: Add support for Intel Apollo Lake
GPIO pinctrl in non-ACPI system
> -----Original Message-----
> From: Lee Jones [mailto:lee.jones@...aro.org]
> Sent: Tuesday, August 9, 2016 3:16 PM
> To: Tan, Jui Nee <jui.nee.tan@...el.com>
> Cc: mika.westerberg@...ux.intel.com; heikki.krogerus@...ux.intel.com;
> andriy.shevchenko@...ux.intel.com; tglx@...utronix.de;
> mingo@...hat.com; hpa@...or.com; x86@...nel.org; ptyser@...-inc.com;
> linus.walleij@...aro.org; linux-gpio@...r.kernel.org; linux-
> kernel@...r.kernel.org; Yong, Jonathan <jonathan.yong@...el.com>; Yu,
> Ong Hock <ong.hock.yu@...el.com>; Voon, Weifeng
> <weifeng.voon@...el.com>; Wan Mohamad, Wan Ahmad Zainie
> <wan.ahmad.zainie.wan.mohamad@...el.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.tan@...el.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.tan@...el.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
Powered by blists - more mailing lists