| Message ID | 1461650991-15185-4-git-send-email-jui.nee.tan@intel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 26 Apr 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@intel.com> > --- > drivers/mfd/Kconfig | 3 +- > drivers/mfd/lpc_ich.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 130 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index eea61e3..54e595c 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -359,8 +359,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 if X86_INTEL_NON_ACPI > 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/lpc_ich.c b/drivers/mfd/lpc_ich.c > index bd3aa45..5d0cc9b 100644 > --- a/drivers/mfd/lpc_ich.c > +++ b/drivers/mfd/lpc_ich.c > @@ -68,6 +68,10 @@ > #include <linux/mfd/core.h> > #include <linux/mfd/lpc_ich.h> > #include <linux/platform_data/itco_wdt.h> > +#include <linux/pinctrl/pinctrl.h> > +#include <linux/types.h> > + > +#include <asm/p2sb.h> > > #define ACPIBASE 0x40 > #define ACPIBASE_GPE_OFF 0x28 > @@ -94,6 +98,19 @@ > #define wdt_mem_res(i) wdt_res(ICH_RES_MEM_OFF, i) > #define wdt_res(b, i) (&wdt_ich_res[(b) + (i)]) > > +/* Offset data for Apollo Lake GPIO communities */ > +#define APL_GPIO_SOUTHWEST_OFFSET 0xc0 > +#define APL_GPIO_NORTHWEST_OFFSET 0xc4 > +#define APL_GPIO_NORTH_OFFSET 0xc5 > +#define APL_GPIO_WEST_OFFSET 0xc7 > + > +#define APL_GPIO_SOUTHWEST_END (43 * 0x8) > +#define APL_GPIO_NORTHWEST_END (77 * 0x8) > +#define APL_GPIO_NORTH_END (90 * 0x8) > +#define APL_GPIO_WEST_END (47 * 0x8) > + > +#define APL_GPIO_IRQ 14 > + > struct lpc_ich_priv { > int chipset; > > @@ -133,6 +150,50 @@ static struct resource gpio_ich_res[] = { > }, > }; > > +#ifdef CONFIG_X86_INTEL_NON_ACPI No, thank you. No unnecessary #ifery. > +static struct resource apl_gpio_io_res[][2] = { The "[][2]" is a warning sign to me. > + { > + { > + .start = APL_GPIO_NORTH_OFFSET << 16, > + .end = (APL_GPIO_NORTH_OFFSET << 16) > + + APL_GPIO_NORTH_END, This is a strange (and complicated) way of calculating register addresses. Please simplify. If in doubt, check out how other drivers/platforms handle it. > + }, > + }, > + { > + { > + .start = APL_GPIO_NORTHWEST_OFFSET << 16, > + .end = (APL_GPIO_NORTHWEST_OFFSET << 16) > + + APL_GPIO_NORTHWEST_END, > + }, > + }, > + { > + { > + .start = APL_GPIO_WEST_OFFSET << 16, > + .end = (APL_GPIO_WEST_OFFSET << 16) > + + APL_GPIO_WEST_END, > + }, > + }, > + { > + { > + .start = APL_GPIO_SOUTHWEST_OFFSET << 16, > + .end = (APL_GPIO_SOUTHWEST_OFFSET << 16) > + + APL_GPIO_SOUTHWEST_END, > + }, > + }, > +}; Use the DEFINE_RES_* defines from include/linux/ioport.h. > +static struct pinctrl_pin_desc apl_pinctrl_pdata; No externs. Have you ran this through checkpatch.pl? I don't even see this struct. Where is it? > +static struct mfd_cell apl_gpio_devices = { > + .name = "apl-pinctrl", > + .num_resources = ARRAY_SIZE(apl_gpio_io_res), > + .resources = apl_gpio_io_res[1], > + .pdata_size = sizeof(apl_pinctrl_pdata), > + .platform_data = &apl_pinctrl_pdata, > + .ignore_resource_conflicts = true, > +}; > +#endif /* CONFIG_X86_INTEL_NON_ACPI */ > + > static struct mfd_cell lpc_ich_wdt_cell = { > .name = "iTCO_wdt", > .num_resources = ARRAY_SIZE(wdt_ich_res), > @@ -216,6 +277,7 @@ enum lpc_chipsets { > LPC_BRASWELL, /* Braswell SoC */ > LPC_LEWISBURG, /* Lewisburg */ > LPC_9S, /* 9 Series */ > + LPC_APL, /* Apollo Lake SoC */ > }; > > static struct lpc_ich_info lpc_chipset_info[] = { > @@ -531,6 +593,10 @@ static struct lpc_ich_info lpc_chipset_info[] = { > .name = "9 Series", > .iTCO_version = 2, > }, > + [LPC_APL] = { > + .name = "Apollo Lake SoC", > + .iTCO_version = 5, > + }, > }; > > /* > @@ -679,6 +745,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}, > @@ -1050,6 +1117,64 @@ wdt_done: > return ret; > } > > +#ifdef CONFIG_X86_INTEL_NON_ACPI > +static int lpc_ich_misc(struct pci_dev *dev, enum lpc_chipsets chipset) > +{ > + unsigned int apl_p2sb = PCI_DEVFN(0x0d, 0); No magic numbers? Please define them. > + unsigned int i; > + int ret; > + > + switch (chipset) { Why a switch? This would look better if you: if (chipset != LPC_APL) return -ENODEV; ... until you actually *need* a switch. > + case LPC_APL: > + /* > + * Apollo lake, has not 1, but 4 gpio controllers, > + * handle it a bit differently. > + */ > + > + for (i = 0; i < ARRAY_SIZE(apl_gpio_io_res); i++) { > + struct resource *res = apl_gpio_io_res[i]; > + > + apl_gpio_devices.resources = res; > + > + /* Fill MEM resource */ > + ret = p2sb_bar(dev, apl_p2sb, res++); > + if (ret) > + goto warn_continue; What does this do? > + /* Fill IRQ resource */ > + res->start = APL_GPIO_IRQ; > + res->end = res->start; > + res->flags = IORESOURCE_IRQ; > + > + apl_pinctrl_pdata.name = kasprintf(GFP_KERNEL, "%u", > + i + 1); All this to call a device "1", "2", etc? > + if (apl_pinctrl_pdata.name) > + ret = mfd_add_devices(&dev->dev, i, > + &apl_gpio_devices, 1, NULL, 0, NULL); mfd_add_devices() is designed to take a group of devices and register them all for you. Calling it once for each separate device you have is not correct. > + else > + ret = -ENOMEM; > + > +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); > + } This code just looks like one big hack to me. Why don't you just declare the correct amount of resources in apl_gpio_devices instead of this hodge-podge? > + break; > + default: > + return -ENODEV; > + } > + return 0; > +} > +#else > +static inline int lpc_ich_misc(struct pci_dev *dev, enum lpc_chipsets chipset) > +{ > + return -ENODEV; > +} > +#endif /* CONFIG_X86_INTEL_NON_ACPI */ Don't do this in drivers. > static int lpc_ich_probe(struct pci_dev *dev, > const struct pci_device_id *id) > { > @@ -1093,6 +1218,9 @@ static int lpc_ich_probe(struct pci_dev *dev, > cell_added = true; > } > > + if (!lpc_ich_misc(dev, priv->chipset)) > + cell_added = true; > + > /* > * We only care if at least one or none of the cells registered > * successfully.
> -----Original Message----- > From: Lee Jones [mailto:lee.jones@linaro.org] > Sent: Monday, May 9, 2016 8:25 PM > To: Tan, Jui Nee <jui.nee.tan@intel.com> > Cc: mika.westerberg@linux.intel.com; heikki.krogerus@linux.intel.com; > andriy.shevchenko@linux.intel.com; tglx@linutronix.de; > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; ptyser@xes-inc.com; > linux-gpio@vger.kernel.org; linux-kernel@vger.kernel.org; Yong, Jonathan > <jonathan.yong@intel.com>; Yu, Ong Hock <ong.hock.yu@intel.com>; Voon, > Weifeng <weifeng.voon@intel.com>; Wan Mohamad, Wan Ahmad Zainie > <wan.ahmad.zainie.wan.mohamad@intel.com> > Subject: Re: [PATCH v2 3/3] mfd: lpc_ich: Add support for Intel Apollo Lake > GPIO pinctrl in non-ACPI system > > On Tue, 26 Apr 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@intel.com> > > --- > > drivers/mfd/Kconfig | 3 +- > > drivers/mfd/lpc_ich.c | 128 > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 130 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index > > eea61e3..54e595c 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -359,8 +359,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 if X86_INTEL_NON_ACPI > > 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/lpc_ich.c b/drivers/mfd/lpc_ich.c index > > bd3aa45..5d0cc9b 100644 > > --- a/drivers/mfd/lpc_ich.c > > +++ b/drivers/mfd/lpc_ich.c > > @@ -68,6 +68,10 @@ > > #include <linux/mfd/core.h> > > #include <linux/mfd/lpc_ich.h> > > #include <linux/platform_data/itco_wdt.h> > > +#include <linux/pinctrl/pinctrl.h> > > +#include <linux/types.h> > > + > > +#include <asm/p2sb.h> > > > > #define ACPIBASE 0x40 > > #define ACPIBASE_GPE_OFF 0x28 > > @@ -94,6 +98,19 @@ > > #define wdt_mem_res(i) wdt_res(ICH_RES_MEM_OFF, i) #define > > wdt_res(b, i) (&wdt_ich_res[(b) + (i)]) > > > > +/* Offset data for Apollo Lake GPIO communities */ > > +#define APL_GPIO_SOUTHWEST_OFFSET 0xc0 > > +#define APL_GPIO_NORTHWEST_OFFSET 0xc4 > > +#define APL_GPIO_NORTH_OFFSET 0xc5 > > +#define APL_GPIO_WEST_OFFSET 0xc7 > > + > > +#define APL_GPIO_SOUTHWEST_END (43 * 0x8) > > +#define APL_GPIO_NORTHWEST_END (77 * 0x8) > > +#define APL_GPIO_NORTH_END (90 * 0x8) > > +#define APL_GPIO_WEST_END (47 * 0x8) > > + > > +#define APL_GPIO_IRQ 14 > > + > > struct lpc_ich_priv { > > int chipset; > > > > @@ -133,6 +150,50 @@ static struct resource gpio_ich_res[] = { > > }, > > }; > > > > +#ifdef CONFIG_X86_INTEL_NON_ACPI > > No, thank you. No unnecessary #ifery. > Sorry for the long delay. There is complaint by kbuidbot about specific kernel configuration, i.e. CONFIG_PINCTRL=n. To solve the issue, I introduced a new config option CONFIG_X86_INTEL_NON_ACPI. I don't know the best solution here, or maybe you can advise something. > > +static struct resource apl_gpio_io_res[][2] = { > > The "[][2]" is a warning sign to me. > Since there are 4 GPIO community for APL, I have modified something like below. Please comment. Thanks. static struct resource apl_gpio_io_res[4][2] = { > > + { > > + { > > + .start = APL_GPIO_NORTH_OFFSET << 16, > > + .end = (APL_GPIO_NORTH_OFFSET << 16) > > + + APL_GPIO_NORTH_END, > > This is a strange (and complicated) way of calculating register addresses. > Please simplify. If in doubt, check out how other drivers/platforms handle it. > I have modified to something like below in next patch-set: ... #define APL_GPIO_SOUTHWEST_OFFSET 0xc00000 ... #define APL_GPIO_SOUTHWEST_NPIN 43 ... static struct resource apl_gpio_io_res[4][2] = { DEFINE_RES_MEM_NAMED(APL_GPIO_SOUTHWEST_OFFSET, APL_GPIO_SOUTHWEST_NPIN * SZ_8, "apl_pinctrl_sw"), ... > > + }, > > + }, > > + { > > + { > > + .start = APL_GPIO_NORTHWEST_OFFSET << 16, > > + .end = (APL_GPIO_NORTHWEST_OFFSET << 16) > > + + APL_GPIO_NORTHWEST_END, > > + }, > > + }, > > + { > > + { > > + .start = APL_GPIO_WEST_OFFSET << 16, > > + .end = (APL_GPIO_WEST_OFFSET << 16) > > + + APL_GPIO_WEST_END, > > + }, > > + }, > > + { > > + { > > + .start = APL_GPIO_SOUTHWEST_OFFSET << 16, > > + .end = (APL_GPIO_SOUTHWEST_OFFSET << 16) > > + + APL_GPIO_SOUTHWEST_END, > > + }, > > + }, > > +}; > > Use the DEFINE_RES_* defines from include/linux/ioport.h. > I will replace it with DEFINE_RES_MEM_NAMED in my next patch-set. > > +static struct pinctrl_pin_desc apl_pinctrl_pdata; > > No externs. Have you ran this through checkpatch.pl? > > I don't even see this struct. Where is it? > The pinctrl_pin_desc struct is located at include/linux/pinctrl/pinctrl.h and that is the purpose we need CONFIG_X86_INTEL_NON_ACPI to select CONFIG_PINCTRL. Else, kbuidbot will complain. > > +static struct mfd_cell apl_gpio_devices = { > > + .name = "apl-pinctrl", > > + .num_resources = ARRAY_SIZE(apl_gpio_io_res), > > + .resources = apl_gpio_io_res[1], > > + .pdata_size = sizeof(apl_pinctrl_pdata), > > + .platform_data = &apl_pinctrl_pdata, > > + .ignore_resource_conflicts = true, > > +}; > > +#endif /* CONFIG_X86_INTEL_NON_ACPI */ > > + > > static struct mfd_cell lpc_ich_wdt_cell = { > > .name = "iTCO_wdt", > > .num_resources = ARRAY_SIZE(wdt_ich_res), @@ -216,6 +277,7 @@ > enum > > lpc_chipsets { > > LPC_BRASWELL, /* Braswell SoC */ > > LPC_LEWISBURG, /* Lewisburg */ > > LPC_9S, /* 9 Series */ > > + LPC_APL, /* Apollo Lake SoC */ > > }; > > > > static struct lpc_ich_info lpc_chipset_info[] = { @@ -531,6 +593,10 > > @@ static struct lpc_ich_info lpc_chipset_info[] = { > > .name = "9 Series", > > .iTCO_version = 2, > > }, > > + [LPC_APL] = { > > + .name = "Apollo Lake SoC", > > + .iTCO_version = 5, > > + }, > > }; > > > > /* > > @@ -679,6 +745,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}, @@ -1050,6 +1117,64 @@ > > wdt_done: > > return ret; > > } > > > > +#ifdef CONFIG_X86_INTEL_NON_ACPI > > +static int lpc_ich_misc(struct pci_dev *dev, enum lpc_chipsets > > +chipset) { > > + unsigned int apl_p2sb = PCI_DEVFN(0x0d, 0); > > No magic numbers? Please define them. > I will fix it in next patch-set. > > + unsigned int i; > > + int ret; > > + > > + switch (chipset) { > > Why a switch? This would look better if you: > > if (chipset != LPC_APL) > return -ENODEV; > I will fix it in next patch-set. > ... until you actually *need* a switch. > > > + case LPC_APL: > > + /* > > + * Apollo lake, has not 1, but 4 gpio controllers, > > + * handle it a bit differently. > > + */ > > + > > + for (i = 0; i < ARRAY_SIZE(apl_gpio_io_res); i++) { > > + struct resource *res = apl_gpio_io_res[i]; > > + > > + apl_gpio_devices.resources = res; > > + > > + /* Fill MEM resource */ > > + ret = p2sb_bar(dev, apl_p2sb, res++); > > + if (ret) > > + goto warn_continue; > > What does this do? > The PCI devices acts strangely when enumerated by the kernel, including rejecting all further PCI writes. As far as I know, it is also hidden for the sake of Windows. We need to use Primary to Sideband bridge (p2sb) communication interface in order to get IO or MMIO bar hidden by BIOS. > > + /* Fill IRQ resource */ > > + res->start = APL_GPIO_IRQ; > > + res->end = res->start; > > + res->flags = IORESOURCE_IRQ; > > + > > + apl_pinctrl_pdata.name = kasprintf(GFP_KERNEL, > "%u", > > + i + 1); > > All this to call a device "1", "2", etc? > > > + if (apl_pinctrl_pdata.name) > > + ret = mfd_add_devices(&dev->dev, i, > > + &apl_gpio_devices, 1, NULL, 0, > NULL); > > mfd_add_devices() is designed to take a group of devices and register them > all for you. Calling it once for each separate device you have is not correct. > Rework the patches that look something like below: ret = mfd_add_devices(&dev->dev, apl_gpio_devices->id, apl_gpio_devices, ARRAY_SIZE(apl_gpio_devices), NULL, 0, NULL); > > + else > > + ret = -ENOMEM; > > + > > +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); > > + } > > This code just looks like one big hack to me. > > Why don't you just declare the correct amount of resources in > apl_gpio_devices instead of this hodge-podge? > I will rewrite the patch following your comments that based on the correct resources amount. > > + break; > > + default: > > + return -ENODEV; > > + } > > + return 0; > > +} > > +#else > > +static inline int lpc_ich_misc(struct pci_dev *dev, enum lpc_chipsets > > +chipset) { > > + return -ENODEV; > > +} > > +#endif /* CONFIG_X86_INTEL_NON_ACPI */ > > Don't do this in drivers. > This is to solve kbuidbot complaint about kernel configuration, i.e. CONFIG_PINCTRL=n. Appreciate if you could advise something on this. > > static int lpc_ich_probe(struct pci_dev *dev, > > const struct pci_device_id *id) > > { > > @@ -1093,6 +1218,9 @@ static int lpc_ich_probe(struct pci_dev *dev, > > cell_added = true; > > } > > > > + if (!lpc_ich_misc(dev, priv->chipset)) > > + cell_added = true; > > + > > /* > > * We only care if at least one or none of the cells registered > > * successfully. > > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source > software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index eea61e3..54e595c 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -359,8 +359,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 if X86_INTEL_NON_ACPI 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/lpc_ich.c b/drivers/mfd/lpc_ich.c index bd3aa45..5d0cc9b 100644 --- a/drivers/mfd/lpc_ich.c +++ b/drivers/mfd/lpc_ich.c @@ -68,6 +68,10 @@ #include <linux/mfd/core.h> #include <linux/mfd/lpc_ich.h> #include <linux/platform_data/itco_wdt.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/types.h> + +#include <asm/p2sb.h> #define ACPIBASE 0x40 #define ACPIBASE_GPE_OFF 0x28 @@ -94,6 +98,19 @@ #define wdt_mem_res(i) wdt_res(ICH_RES_MEM_OFF, i) #define wdt_res(b, i) (&wdt_ich_res[(b) + (i)]) +/* Offset data for Apollo Lake GPIO communities */ +#define APL_GPIO_SOUTHWEST_OFFSET 0xc0 +#define APL_GPIO_NORTHWEST_OFFSET 0xc4 +#define APL_GPIO_NORTH_OFFSET 0xc5 +#define APL_GPIO_WEST_OFFSET 0xc7 + +#define APL_GPIO_SOUTHWEST_END (43 * 0x8) +#define APL_GPIO_NORTHWEST_END (77 * 0x8) +#define APL_GPIO_NORTH_END (90 * 0x8) +#define APL_GPIO_WEST_END (47 * 0x8) + +#define APL_GPIO_IRQ 14 + struct lpc_ich_priv { int chipset; @@ -133,6 +150,50 @@ static struct resource gpio_ich_res[] = { }, }; +#ifdef CONFIG_X86_INTEL_NON_ACPI +static struct resource apl_gpio_io_res[][2] = { + { + { + .start = APL_GPIO_NORTH_OFFSET << 16, + .end = (APL_GPIO_NORTH_OFFSET << 16) + + APL_GPIO_NORTH_END, + }, + }, + { + { + .start = APL_GPIO_NORTHWEST_OFFSET << 16, + .end = (APL_GPIO_NORTHWEST_OFFSET << 16) + + APL_GPIO_NORTHWEST_END, + }, + }, + { + { + .start = APL_GPIO_WEST_OFFSET << 16, + .end = (APL_GPIO_WEST_OFFSET << 16) + + APL_GPIO_WEST_END, + }, + }, + { + { + .start = APL_GPIO_SOUTHWEST_OFFSET << 16, + .end = (APL_GPIO_SOUTHWEST_OFFSET << 16) + + APL_GPIO_SOUTHWEST_END, + }, + }, +}; + +static struct pinctrl_pin_desc apl_pinctrl_pdata; + +static struct mfd_cell apl_gpio_devices = { + .name = "apl-pinctrl", + .num_resources = ARRAY_SIZE(apl_gpio_io_res), + .resources = apl_gpio_io_res[1], + .pdata_size = sizeof(apl_pinctrl_pdata), + .platform_data = &apl_pinctrl_pdata, + .ignore_resource_conflicts = true, +}; +#endif /* CONFIG_X86_INTEL_NON_ACPI */ + static struct mfd_cell lpc_ich_wdt_cell = { .name = "iTCO_wdt", .num_resources = ARRAY_SIZE(wdt_ich_res), @@ -216,6 +277,7 @@ enum lpc_chipsets { LPC_BRASWELL, /* Braswell SoC */ LPC_LEWISBURG, /* Lewisburg */ LPC_9S, /* 9 Series */ + LPC_APL, /* Apollo Lake SoC */ }; static struct lpc_ich_info lpc_chipset_info[] = { @@ -531,6 +593,10 @@ static struct lpc_ich_info lpc_chipset_info[] = { .name = "9 Series", .iTCO_version = 2, }, + [LPC_APL] = { + .name = "Apollo Lake SoC", + .iTCO_version = 5, + }, }; /* @@ -679,6 +745,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}, @@ -1050,6 +1117,64 @@ wdt_done: return ret; } +#ifdef CONFIG_X86_INTEL_NON_ACPI +static int lpc_ich_misc(struct pci_dev *dev, enum lpc_chipsets chipset) +{ + unsigned int apl_p2sb = PCI_DEVFN(0x0d, 0); + unsigned int i; + int ret; + + switch (chipset) { + case LPC_APL: + /* + * Apollo lake, has not 1, but 4 gpio controllers, + * handle it a bit differently. + */ + + for (i = 0; i < ARRAY_SIZE(apl_gpio_io_res); i++) { + struct resource *res = apl_gpio_io_res[i]; + + apl_gpio_devices.resources = res; + + /* Fill MEM resource */ + ret = p2sb_bar(dev, apl_p2sb, res++); + if (ret) + goto warn_continue; + + /* Fill IRQ resource */ + res->start = APL_GPIO_IRQ; + res->end = res->start; + res->flags = IORESOURCE_IRQ; + + apl_pinctrl_pdata.name = kasprintf(GFP_KERNEL, "%u", + i + 1); + if (apl_pinctrl_pdata.name) + ret = mfd_add_devices(&dev->dev, i, + &apl_gpio_devices, 1, NULL, 0, NULL); + else + ret = -ENOMEM; + +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); + } + break; + default: + return -ENODEV; + } + return 0; +} +#else +static inline int lpc_ich_misc(struct pci_dev *dev, enum lpc_chipsets chipset) +{ + return -ENODEV; +} +#endif /* CONFIG_X86_INTEL_NON_ACPI */ + static int lpc_ich_probe(struct pci_dev *dev, const struct pci_device_id *id) { @@ -1093,6 +1218,9 @@ static int lpc_ich_probe(struct pci_dev *dev, cell_added = true; } + if (!lpc_ich_misc(dev, priv->chipset)) + cell_added = true; + /* * We only care if at least one or none of the cells registered * successfully.
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@intel.com> --- drivers/mfd/Kconfig | 3 +- drivers/mfd/lpc_ich.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-)