| Message ID | 20220131151346.45792-6-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | platform/x86: introduce p2sb_bar() helper | expand |
On Mon, 31 Jan 2022, Andy Shevchenko wrote: > From: Tan Jui Nee <jui.nee.tan@intel.com> > > Add support for non-ACPI systems, such as system that uses > Advanced Boot Loader (ABL) whereby a platform device has to be created > in order to bind with pin control and GPIO. > > At the moment, Intel Apollo Lake In-Vehicle Infotainment (IVI) system > requires a driver to hide and unhide P2SB to lookup P2SB BAR and pass > the PCI BAR address to GPIO. > > Signed-off-by: Tan Jui Nee <jui.nee.tan@intel.com> > Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Hans de Goede <hdegoede@redhat.com> > Acked-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/mfd/lpc_ich.c | 101 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 100 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c > index 95dca5434917..e1bca5325ce7 100644 > --- a/drivers/mfd/lpc_ich.c > +++ b/drivers/mfd/lpc_ich.c > @@ -8,7 +8,8 @@ > * Configuration Registers. > * > * This driver is derived from lpc_sch. > - > + * > + * Copyright (c) 2017, 2021-2022 Intel Corporation > * Copyright (c) 2011 Extreme Engineering Solution, Inc. > * Author: Aaron Sierra <asierra@xes-inc.com> > * > @@ -42,6 +43,7 @@ > #include <linux/errno.h> > #include <linux/acpi.h> > #include <linux/pci.h> > +#include <linux/pinctrl/pinctrl.h> > #include <linux/mfd/core.h> > #include <linux/mfd/lpc_ich.h> > #include <linux/platform_data/itco_wdt.h> > @@ -140,6 +142,70 @@ static struct mfd_cell lpc_ich_gpio_cell = { > .ignore_resource_conflicts = true, > }; > > +#define APL_GPIO_NORTH 0 > +#define APL_GPIO_NORTHWEST 1 > +#define APL_GPIO_WEST 2 > +#define APL_GPIO_SOUTHWEST 3 > +#define APL_GPIO_NR_DEVICES 4 > + > +/* Offset data for Apollo Lake GPIO controllers */ > +#define APL_GPIO_NORTH_OFFSET 0xc50000 > +#define APL_GPIO_NORTHWEST_OFFSET 0xc40000 > +#define APL_GPIO_WEST_OFFSET 0xc70000 > +#define APL_GPIO_SOUTHWEST_OFFSET 0xc00000 > + > +#define APL_GPIO_IRQ 14 > + > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { > + [APL_GPIO_NORTH] = { > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), Are these 0x1000's being over-written in lpc_ich_init_pinctrl()? If so, why pre-initialise? > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > + [APL_GPIO_NORTHWEST] = { > + DEFINE_RES_MEM(APL_GPIO_NORTHWEST_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > + [APL_GPIO_WEST] = { > + DEFINE_RES_MEM(APL_GPIO_WEST_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > + [APL_GPIO_SOUTHWEST] = { > + DEFINE_RES_MEM(APL_GPIO_SOUTHWEST_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > +}; > + > +/* The order must be in sync with apl_pinctrl_soc_data */ Why does the order matter if you've pre-enumerated them all? > +static const struct mfd_cell apl_gpio_devices[APL_GPIO_NR_DEVICES] = { > + [APL_GPIO_NORTH] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_NORTH, > + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTH]), > + .resources = apl_gpio_resources[APL_GPIO_NORTH], > + .ignore_resource_conflicts = true, > + }, > + [APL_GPIO_NORTHWEST] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_NORTHWEST, > + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTHWEST]), > + .resources = apl_gpio_resources[APL_GPIO_NORTHWEST], > + .ignore_resource_conflicts = true, > + }, > + [APL_GPIO_WEST] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_WEST, > + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_WEST]), > + .resources = apl_gpio_resources[APL_GPIO_WEST], > + .ignore_resource_conflicts = true, > + }, > + [APL_GPIO_SOUTHWEST] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_SOUTHWEST, > + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_SOUTHWEST]), > + .resources = apl_gpio_resources[APL_GPIO_SOUTHWEST], > + .ignore_resource_conflicts = true, > + }, > +}; > > static struct mfd_cell lpc_ich_spi_cell = { > .name = "intel-spi", > @@ -1083,6 +1149,33 @@ static int lpc_ich_init_wdt(struct pci_dev *dev) > return ret; > } > > +static int lpc_ich_init_pinctrl(struct pci_dev *dev) > +{ > + struct resource base; > + unsigned int i; > + int ret; > + > + /* Check, if GPIO has been exported as an ACPI device */ > + if (acpi_dev_present("INT3452", NULL, -1)) > + return -EEXIST; > + > + ret = p2sb_bar(dev->bus, 0, &base); > + if (ret) > + return ret; > + > + for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { > + struct resource *mem = &apl_gpio_resources[i][0]; > + > + /* Fill MEM resource */ > + mem->start += base.start; > + mem->end += base.start; > + mem->flags = base.flags; > + } > + > + return mfd_add_devices(&dev->dev, 0, apl_gpio_devices, > + ARRAY_SIZE(apl_gpio_devices), NULL, 0, NULL); > +} > + > static void lpc_ich_test_spi_write(struct pci_dev *dev, unsigned int devfn, > struct intel_spi_boardinfo *info) > { > @@ -1199,6 +1292,12 @@ static int lpc_ich_probe(struct pci_dev *dev, > cell_added = true; > } > > + if (priv->chipset == LPC_APL) { > + ret = lpc_ich_init_pinctrl(dev); > + if (!ret) > + cell_added = true; > + } > + > if (lpc_chipset_info[priv->chipset].spi_type) { > ret = lpc_ich_init_spi(dev); > if (!ret)
On Tue, Feb 15, 2022 at 04:54:00PM +0000, Lee Jones wrote: > On Mon, 31 Jan 2022, Andy Shevchenko wrote: Thank you for the review, my answers below. ... > > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { > > + [APL_GPIO_NORTH] = { > > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), > > Are these 0x1000's being over-written in lpc_ich_init_pinctrl()? > > If so, why pre-initialise? You mean to pre-initialize the offsets, but leave the length to be added in the function? It can be done, but it feels inconsistent, since we would have offsets and lengths in different places for the same thingy. That said, I prefer current way for the sake of consistency. > > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > > + }, ... > > +/* The order must be in sync with apl_pinctrl_soc_data */ > > Why does the order matter if you've pre-enumerated them all? Indeed. I will drop the confusing comment in the next version.
On Tue, 15 Feb 2022, Andy Shevchenko wrote: > On Tue, Feb 15, 2022 at 04:54:00PM +0000, Lee Jones wrote: > > On Mon, 31 Jan 2022, Andy Shevchenko wrote: > > Thank you for the review, my answers below. > > ... > > > > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { > > > + [APL_GPIO_NORTH] = { > > > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), > > > > Are these 0x1000's being over-written in lpc_ich_init_pinctrl()? > > > > If so, why pre-initialise? > > You mean to pre-initialize the offsets, but leave the length to be added > in the function? It can be done, but it feels inconsistent, since we would > have offsets and lengths in different places for the same thingy. That said, > I prefer current way for the sake of consistency. Don't you over-write this entry entirely? for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { struct resource *mem = &apl_gpio_resources[i][0]; /* Fill MEM resource */ mem->start += base.start; mem->end += base.start; mem->flags = base.flags; } Oh wait, you're just adding the base value to the offsets. In which case that comment is also confusing!
Can this patch not be proposed separately? Maybe i am wrong but it seems unrelated to the p2sb story. The whole p2sb base and size discovery is easy and switching the simatic drivers is also. It is an interface change, where the old open coding remains working. But having to switch to GPIO in the same series is kind of weird. That is a functional change which even might deserve its own cover letter. I bet there are tons of out-of-tree modules which will stop working on apl after that gets merged. I still did not understand why apl is special and other boards do not get their pinctrl brought up without ACPI/p2sb-visible. I have patches floating around, but still would be happy if we could do one thing at a time. Or maybe it is strongly coupled and i do not understand why. regards, Henning Am Mon, 31 Jan 2022 17:13:43 +0200 schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > From: Tan Jui Nee <jui.nee.tan@intel.com> > > Add support for non-ACPI systems, such as system that uses > Advanced Boot Loader (ABL) whereby a platform device has to be created > in order to bind with pin control and GPIO. > > At the moment, Intel Apollo Lake In-Vehicle Infotainment (IVI) system > requires a driver to hide and unhide P2SB to lookup P2SB BAR and pass > the PCI BAR address to GPIO. > > Signed-off-by: Tan Jui Nee <jui.nee.tan@intel.com> > Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Acked-by: Hans de Goede <hdegoede@redhat.com> > Acked-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/mfd/lpc_ich.c | 101 > +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 100 > insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c > index 95dca5434917..e1bca5325ce7 100644 > --- a/drivers/mfd/lpc_ich.c > +++ b/drivers/mfd/lpc_ich.c > @@ -8,7 +8,8 @@ > * Configuration Registers. > * > * This driver is derived from lpc_sch. > - > + * > + * Copyright (c) 2017, 2021-2022 Intel Corporation > * Copyright (c) 2011 Extreme Engineering Solution, Inc. > * Author: Aaron Sierra <asierra@xes-inc.com> > * > @@ -42,6 +43,7 @@ > #include <linux/errno.h> > #include <linux/acpi.h> > #include <linux/pci.h> > +#include <linux/pinctrl/pinctrl.h> > #include <linux/mfd/core.h> > #include <linux/mfd/lpc_ich.h> > #include <linux/platform_data/itco_wdt.h> > @@ -140,6 +142,70 @@ static struct mfd_cell lpc_ich_gpio_cell = { > .ignore_resource_conflicts = true, > }; > > +#define APL_GPIO_NORTH 0 > +#define APL_GPIO_NORTHWEST 1 > +#define APL_GPIO_WEST 2 > +#define APL_GPIO_SOUTHWEST 3 > +#define APL_GPIO_NR_DEVICES 4 > + > +/* Offset data for Apollo Lake GPIO controllers */ > +#define APL_GPIO_NORTH_OFFSET 0xc50000 > +#define APL_GPIO_NORTHWEST_OFFSET 0xc40000 > +#define APL_GPIO_WEST_OFFSET 0xc70000 > +#define APL_GPIO_SOUTHWEST_OFFSET 0xc00000 > + > +#define APL_GPIO_IRQ 14 > + > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { > + [APL_GPIO_NORTH] = { > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > + [APL_GPIO_NORTHWEST] = { > + DEFINE_RES_MEM(APL_GPIO_NORTHWEST_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > + [APL_GPIO_WEST] = { > + DEFINE_RES_MEM(APL_GPIO_WEST_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > + [APL_GPIO_SOUTHWEST] = { > + DEFINE_RES_MEM(APL_GPIO_SOUTHWEST_OFFSET, 0x1000), > + DEFINE_RES_IRQ(APL_GPIO_IRQ), > + }, > +}; > + > +/* The order must be in sync with apl_pinctrl_soc_data */ > +static const struct mfd_cell apl_gpio_devices[APL_GPIO_NR_DEVICES] = > { > + [APL_GPIO_NORTH] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_NORTH, > + .num_resources = > ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTH]), > + .resources = apl_gpio_resources[APL_GPIO_NORTH], > + .ignore_resource_conflicts = true, > + }, > + [APL_GPIO_NORTHWEST] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_NORTHWEST, > + .num_resources = > ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTHWEST]), > + .resources = apl_gpio_resources[APL_GPIO_NORTHWEST], > + .ignore_resource_conflicts = true, > + }, > + [APL_GPIO_WEST] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_WEST, > + .num_resources = > ARRAY_SIZE(apl_gpio_resources[APL_GPIO_WEST]), > + .resources = apl_gpio_resources[APL_GPIO_WEST], > + .ignore_resource_conflicts = true, > + }, > + [APL_GPIO_SOUTHWEST] = { > + .name = "apollolake-pinctrl", > + .id = APL_GPIO_SOUTHWEST, > + .num_resources = > ARRAY_SIZE(apl_gpio_resources[APL_GPIO_SOUTHWEST]), > + .resources = apl_gpio_resources[APL_GPIO_SOUTHWEST], > + .ignore_resource_conflicts = true, > + }, > +}; > > static struct mfd_cell lpc_ich_spi_cell = { > .name = "intel-spi", > @@ -1083,6 +1149,33 @@ static int lpc_ich_init_wdt(struct pci_dev > *dev) return ret; > } > > +static int lpc_ich_init_pinctrl(struct pci_dev *dev) > +{ > + struct resource base; > + unsigned int i; > + int ret; > + > + /* Check, if GPIO has been exported as an ACPI device */ > + if (acpi_dev_present("INT3452", NULL, -1)) > + return -EEXIST; > + > + ret = p2sb_bar(dev->bus, 0, &base); > + if (ret) > + return ret; > + > + for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { > + struct resource *mem = &apl_gpio_resources[i][0]; > + > + /* Fill MEM resource */ > + mem->start += base.start; > + mem->end += base.start; > + mem->flags = base.flags; > + } > + > + return mfd_add_devices(&dev->dev, 0, apl_gpio_devices, > + ARRAY_SIZE(apl_gpio_devices), NULL, > 0, NULL); +} > + > static void lpc_ich_test_spi_write(struct pci_dev *dev, unsigned int > devfn, struct intel_spi_boardinfo *info) > { > @@ -1199,6 +1292,12 @@ static int lpc_ich_probe(struct pci_dev *dev, > cell_added = true; > } > > + if (priv->chipset == LPC_APL) { > + ret = lpc_ich_init_pinctrl(dev); > + if (!ret) > + cell_added = true; > + } > + > if (lpc_chipset_info[priv->chipset].spi_type) { > ret = lpc_ich_init_spi(dev); > if (!ret)
On Mon, Mar 7, 2022 at 8:21 PM Henning Schild <henning.schild@siemens.com> wrote: Please, do not top-post. > Can this patch not be proposed separately? Maybe i am wrong but it > seems unrelated to the p2sb story. The entire story happens to begin from this very change. The author (you may see that's not me) proposed the change a long time ago and AFAIU this is the requirement to have it upstreamed. > The whole p2sb base and size discovery is easy and switching the > simatic drivers is also. It is an interface change, where the old open > coding remains working. > > But having to switch to GPIO in the same series is kind of weird. That > is a functional change which even might deserve its own cover letter. I > bet there are tons of out-of-tree modules which will stop working on > apl after that gets merged. Upstream rarely, if at all, cares about 3rd party modules. From the upstream point of view the thing (whatever the 3rd party module supports) wasn't working ("no driver" in upstream) and won't work (still "no driver" in upstream) after the change, so there may not be any regression. > I still did not understand why apl is special and other boards do not > get their pinctrl brought up without ACPI/p2sb-visible. The platform is being heavily used by one of our departments in such configuration with firmwares that may not be fully compatible with UEFI.They want to support that along with the case when BIOS has no GPIO device being provided. > I have patches floating around, but still would be happy if we could do > one thing at a time. Either way any new changes must use a pin control driver and the previous work was accepted only on this basis. > Or maybe it is strongly coupled and I do not understand why. That's the initial requirement by our peer departament.
On Tue, Feb 15, 2022 at 05:29:51PM +0000, Lee Jones wrote: > On Tue, 15 Feb 2022, Andy Shevchenko wrote: > > On Tue, Feb 15, 2022 at 04:54:00PM +0000, Lee Jones wrote: > > > On Mon, 31 Jan 2022, Andy Shevchenko wrote: ... > > > > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { > > > > + [APL_GPIO_NORTH] = { > > > > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), > > > > > > Are these 0x1000's being over-written in lpc_ich_init_pinctrl()? > > > > > > If so, why pre-initialise? > > > > You mean to pre-initialize the offsets, but leave the length to be added > > in the function? It can be done, but it feels inconsistent, since we would > > have offsets and lengths in different places for the same thingy. That said, > > I prefer current way for the sake of consistency. > > Don't you over-write this entry entirely? > > for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { > struct resource *mem = &apl_gpio_resources[i][0]; > > /* Fill MEM resource */ > mem->start += base.start; > mem->end += base.start; > mem->flags = base.flags; > } > > Oh wait, you're just adding the base value to the offsets. Right. > In which case that comment is also confusing! I will fix a comment in the next version.
On Tue, Feb 15, 2022 at 05:29:51PM +0000, Lee Jones wrote: > On Tue, 15 Feb 2022, Andy Shevchenko wrote: > > On Tue, Feb 15, 2022 at 04:54:00PM +0000, Lee Jones wrote: > > > On Mon, 31 Jan 2022, Andy Shevchenko wrote: > > Thank you for the review, my answers below. ... > > > > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { > > > > + [APL_GPIO_NORTH] = { > > > > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), > > > > > > Are these 0x1000's being over-written in lpc_ich_init_pinctrl()? > > > > > > If so, why pre-initialise? > > > > You mean to pre-initialize the offsets, but leave the length to be added > > in the function? It can be done, but it feels inconsistent, since we would > > have offsets and lengths in different places for the same thingy. That said, > > I prefer current way for the sake of consistency. > > Don't you over-write this entry entirely? > > for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { > struct resource *mem = &apl_gpio_resources[i][0]; > > /* Fill MEM resource */ > mem->start += base.start; > mem->end += base.start; > mem->flags = base.flags; > } > > Oh wait, you're just adding the base value to the offsets. > > In which case that comment is also confusing! I have realised that in current form it has a bug (*), so I re-do a bit the way that comment stays and the actual actions will be to *fill* the resource. *) unbinding and binding back will bring us to the completely wrong resources.
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c index 95dca5434917..e1bca5325ce7 100644 --- a/drivers/mfd/lpc_ich.c +++ b/drivers/mfd/lpc_ich.c @@ -8,7 +8,8 @@ * Configuration Registers. * * This driver is derived from lpc_sch. - + * + * Copyright (c) 2017, 2021-2022 Intel Corporation * Copyright (c) 2011 Extreme Engineering Solution, Inc. * Author: Aaron Sierra <asierra@xes-inc.com> * @@ -42,6 +43,7 @@ #include <linux/errno.h> #include <linux/acpi.h> #include <linux/pci.h> +#include <linux/pinctrl/pinctrl.h> #include <linux/mfd/core.h> #include <linux/mfd/lpc_ich.h> #include <linux/platform_data/itco_wdt.h> @@ -140,6 +142,70 @@ static struct mfd_cell lpc_ich_gpio_cell = { .ignore_resource_conflicts = true, }; +#define APL_GPIO_NORTH 0 +#define APL_GPIO_NORTHWEST 1 +#define APL_GPIO_WEST 2 +#define APL_GPIO_SOUTHWEST 3 +#define APL_GPIO_NR_DEVICES 4 + +/* Offset data for Apollo Lake GPIO controllers */ +#define APL_GPIO_NORTH_OFFSET 0xc50000 +#define APL_GPIO_NORTHWEST_OFFSET 0xc40000 +#define APL_GPIO_WEST_OFFSET 0xc70000 +#define APL_GPIO_SOUTHWEST_OFFSET 0xc00000 + +#define APL_GPIO_IRQ 14 + +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = { + [APL_GPIO_NORTH] = { + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000), + DEFINE_RES_IRQ(APL_GPIO_IRQ), + }, + [APL_GPIO_NORTHWEST] = { + DEFINE_RES_MEM(APL_GPIO_NORTHWEST_OFFSET, 0x1000), + DEFINE_RES_IRQ(APL_GPIO_IRQ), + }, + [APL_GPIO_WEST] = { + DEFINE_RES_MEM(APL_GPIO_WEST_OFFSET, 0x1000), + DEFINE_RES_IRQ(APL_GPIO_IRQ), + }, + [APL_GPIO_SOUTHWEST] = { + DEFINE_RES_MEM(APL_GPIO_SOUTHWEST_OFFSET, 0x1000), + DEFINE_RES_IRQ(APL_GPIO_IRQ), + }, +}; + +/* The order must be in sync with apl_pinctrl_soc_data */ +static const struct mfd_cell apl_gpio_devices[APL_GPIO_NR_DEVICES] = { + [APL_GPIO_NORTH] = { + .name = "apollolake-pinctrl", + .id = APL_GPIO_NORTH, + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTH]), + .resources = apl_gpio_resources[APL_GPIO_NORTH], + .ignore_resource_conflicts = true, + }, + [APL_GPIO_NORTHWEST] = { + .name = "apollolake-pinctrl", + .id = APL_GPIO_NORTHWEST, + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTHWEST]), + .resources = apl_gpio_resources[APL_GPIO_NORTHWEST], + .ignore_resource_conflicts = true, + }, + [APL_GPIO_WEST] = { + .name = "apollolake-pinctrl", + .id = APL_GPIO_WEST, + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_WEST]), + .resources = apl_gpio_resources[APL_GPIO_WEST], + .ignore_resource_conflicts = true, + }, + [APL_GPIO_SOUTHWEST] = { + .name = "apollolake-pinctrl", + .id = APL_GPIO_SOUTHWEST, + .num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_SOUTHWEST]), + .resources = apl_gpio_resources[APL_GPIO_SOUTHWEST], + .ignore_resource_conflicts = true, + }, +}; static struct mfd_cell lpc_ich_spi_cell = { .name = "intel-spi", @@ -1083,6 +1149,33 @@ static int lpc_ich_init_wdt(struct pci_dev *dev) return ret; } +static int lpc_ich_init_pinctrl(struct pci_dev *dev) +{ + struct resource base; + unsigned int i; + int ret; + + /* Check, if GPIO has been exported as an ACPI device */ + if (acpi_dev_present("INT3452", NULL, -1)) + return -EEXIST; + + ret = p2sb_bar(dev->bus, 0, &base); + if (ret) + return ret; + + for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) { + struct resource *mem = &apl_gpio_resources[i][0]; + + /* Fill MEM resource */ + mem->start += base.start; + mem->end += base.start; + mem->flags = base.flags; + } + + return mfd_add_devices(&dev->dev, 0, apl_gpio_devices, + ARRAY_SIZE(apl_gpio_devices), NULL, 0, NULL); +} + static void lpc_ich_test_spi_write(struct pci_dev *dev, unsigned int devfn, struct intel_spi_boardinfo *info) { @@ -1199,6 +1292,12 @@ static int lpc_ich_probe(struct pci_dev *dev, cell_added = true; } + if (priv->chipset == LPC_APL) { + ret = lpc_ich_init_pinctrl(dev); + if (!ret) + cell_added = true; + } + if (lpc_chipset_info[priv->chipset].spi_type) { ret = lpc_ich_init_spi(dev); if (!ret)