Re: [PATCH v4 5/8] mfd: lpc_ich: Add support for pinctrl in non-ACPI system

[Henning Schild <henning.schild@siemens.com>]

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)