[PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[Shawn Guo]

It adds ACPI probe support for pinctrl-sc8180x driver.  We have one
problem with ACPI table, i.e. GIO0 (TLMM) block has one single memory
resource to cover 3 tiles defined by SC8180X.  To follow the hardware
layout of 3 tiles which is already supported DT probe, it adds one
function to replace the original single memory resource with 3 named
ones for tiles.  With that, We can map memory for ACPI in the same way
as DT.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Changes for v5:
- Keep .ngpios number as 190 to match SoC spec.
- Add comments for sc8180x_pinctrl_add_tile_resources().
- Drop redundant error message.

Changes for v4:
- Add sc8180x_pinctrl_add_tile_resources() to massage memory resource
  for ACPI probe.

Changes for v3:
- Remove the use of tiles completely.
- Drop unneed include of acpi.h.

Changes for v2:
- Pass soc_data pointer via .driver_data.
- Drop use of CONFIG_ACPI and ACPI_PTR().
- Add comment for sc8180x_acpi_reserved_gpios[] terminator.

 drivers/pinctrl/qcom/Kconfig           |   2 +-
 drivers/pinctrl/qcom/pinctrl-sc8180x.c | 123 ++++++++++++++++++++++++-
 2 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 6853a896c476..9f0218c4f9b3 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -222,7 +222,7 @@ config PINCTRL_SC7280
 
 config PINCTRL_SC8180X
 	tristate "Qualcomm Technologies Inc SC8180x pin controller driver"
-	depends on GPIOLIB && OF
+	depends on GPIOLIB && (OF || ACPI)
 	select PINCTRL_MSM
 	help
 	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
diff --git a/drivers/pinctrl/qcom/pinctrl-sc8180x.c b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
index b765bf667574..0d9654b4ab60 100644
--- a/drivers/pinctrl/qcom/pinctrl-sc8180x.c
+++ b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
@@ -23,6 +23,21 @@ enum {
 	WEST
 };
 
+/*
+ * ACPI DSDT has one single memory resource for TLMM.  The offsets below are
+ * used to locate different tiles for ACPI probe.
+ */
+struct tile_info {
+	u32 offset;
+	u32 size;
+};
+
+static const struct tile_info sc8180x_tile_info[] = {
+	{ 0x00d00000, 0x00300000, },
+	{ 0x00500000, 0x00700000, },
+	{ 0x00100000, 0x00300000, },
+};
+
 #define FUNCTION(fname)					\
 	[msm_mux_##fname] = {				\
 		.name = #fname,				\
@@ -1557,6 +1572,13 @@ static const struct msm_pingroup sc8180x_groups[] = {
 	[193] = SDC_QDSD_PINGROUP(sdc2_data, 0x4b2000, 9, 0),
 };
 
+static const int sc8180x_acpi_reserved_gpios[] = {
+	0, 1, 2, 3,
+	47, 48, 49, 50,
+	126, 127, 128, 129,
+	-1 /* terminator */
+};
+
 static const struct msm_gpio_wakeirq_map sc8180x_pdc_map[] = {
 	{ 3, 31 }, { 5, 32 }, { 8, 33 }, { 9, 34 }, { 10, 100 }, { 12, 104 },
 	{ 24, 37 }, { 26, 38 }, { 27, 41 }, { 28, 42 }, { 30, 39 }, { 36, 43 },
@@ -1588,13 +1610,109 @@ static struct msm_pinctrl_soc_data sc8180x_pinctrl = {
 	.nwakeirq_map = ARRAY_SIZE(sc8180x_pdc_map),
 };
 
+static const struct msm_pinctrl_soc_data sc8180x_acpi_pinctrl = {
+	.tiles = sc8180x_tiles,
+	.ntiles = ARRAY_SIZE(sc8180x_tiles),
+	.pins = sc8180x_pins,
+	.npins = ARRAY_SIZE(sc8180x_pins),
+	.groups = sc8180x_groups,
+	.ngroups = ARRAY_SIZE(sc8180x_groups),
+	.reserved_gpios = sc8180x_acpi_reserved_gpios,
+	.ngpios = 190,
+};
+
+/*
+ * ACPI DSDT has one single memory resource for TLMM, which voilates the
+ * hardware layout of 3 sepearte tiles.  Let's split the memory resource into
+ * 3 named ones, so that msm_pinctrl_probe() can map memory for ACPI in the
+ * same way as for DT probe.
+ */
+static int sc8180x_pinctrl_add_tile_resources(struct platform_device *pdev)
+{
+	int nres_num = pdev->num_resources + ARRAY_SIZE(sc8180x_tiles) - 1;
+	struct resource *mres, *nres, *res;
+	int i, ret;
+
+	/*
+	 * DT already has tiles defined properly, so nothing needs to be done
+	 * for DT probe.
+	 */
+	if (pdev->dev.of_node)
+		return 0;
+
+	/* Allocate for new resources */
+	nres = devm_kzalloc(&pdev->dev, sizeof(*nres) * nres_num, GFP_KERNEL);
+	if (!nres)
+		return -ENOMEM;
+
+	res = nres;
+
+	for (i = 0; i < pdev->num_resources; i++) {
+		struct resource *r = &pdev->resource[i];
+
+		/* Save memory resource and copy others */
+		if (resource_type(r) == IORESOURCE_MEM)
+			mres = r;
+		else
+			*res++ = *r;
+	}
+
+	/* Append tile memory resources */
+	for (i = 0; i < ARRAY_SIZE(sc8180x_tiles); i++, res++) {
+		const struct tile_info *info = &sc8180x_tile_info[i];
+
+		res->start = mres->start + info->offset;
+		res->end = mres->start + info->offset + info->size - 1;
+		res->flags = mres->flags;
+		res->name = sc8180x_tiles[i];
+
+		/* Add new MEM to resource tree */
+		insert_resource(mres->parent, res);
+	}
+
+	/* Remove old MEM from resource tree */
+	remove_resource(mres);
+
+	/* Free old resources and install new ones */
+	ret = platform_device_add_resources(pdev, nres, nres_num);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add new resources: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int sc8180x_pinctrl_probe(struct platform_device *pdev)
 {
-	return msm_pinctrl_probe(pdev, &sc8180x_pinctrl);
+	const struct msm_pinctrl_soc_data *soc_data;
+	int ret;
+
+	soc_data = device_get_match_data(&pdev->dev);
+	if (!soc_data)
+		return -EINVAL;
+
+	ret = sc8180x_pinctrl_add_tile_resources(pdev);
+	if (ret)
+		return ret;
+
+	return msm_pinctrl_probe(pdev, soc_data);
 }
 
+static const struct acpi_device_id sc8180x_pinctrl_acpi_match[] = {
+	{
+		.id = "QCOM040D",
+		.driver_data = (kernel_ulong_t) &sc8180x_acpi_pinctrl,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, sc8180x_pinctrl_acpi_match);
+
 static const struct of_device_id sc8180x_pinctrl_of_match[] = {
-	{ .compatible = "qcom,sc8180x-tlmm", },
+	{
+		.compatible = "qcom,sc8180x-tlmm",
+		.data = &sc8180x_pinctrl,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, sc8180x_pinctrl_of_match);
@@ -1603,6 +1721,7 @@ static struct platform_driver sc8180x_pinctrl_driver = {
 	.driver = {
 		.name = "sc8180x-pinctrl",
 		.of_match_table = sc8180x_pinctrl_of_match,
+		.acpi_match_table = sc8180x_pinctrl_acpi_match,
 	},
 	.probe = sc8180x_pinctrl_probe,
 	.remove = msm_pinctrl_remove,
-- 
2.17.1

Re: [PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6099 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210311024102.15450-1-shawn.guo@linaro.org>
References: <20210311024102.15450-1-shawn.guo@linaro.org>
TO: Shawn Guo <shawn.guo@linaro.org>
TO: Linus Walleij <linus.walleij@linaro.org>
CC: Bjorn Andersson <bjorn.andersson@linaro.org>
CC: Andy Shevchenko <andriy.shevchenko@intel.com>
CC: linux-gpio(a)vger.kernel.org
CC: linux-arm-msm(a)vger.kernel.org
CC: Shawn Guo <shawn.guo@linaro.org>

Hi Shawn,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pinctrl/devel]
[also build test WARNING on v5.12-rc2 next-20210310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Shawn-Guo/pinctrl-qcom-sc8180x-add-ACPI-probe-support/20210311-104247
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
:::::: branch date: 68 minutes ago
:::::: commit date: 68 minutes ago
config: arm64-randconfig-m031-20210311 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/pinctrl/qcom/pinctrl-sc8180x.c:1664 sc8180x_pinctrl_add_tile_resources() error: potentially dereferencing uninitialized 'mres'.

vim +/mres +1664 drivers/pinctrl/qcom/pinctrl-sc8180x.c

76b21f18b22a6b Shawn Guo 2021-03-11  1623  
76b21f18b22a6b Shawn Guo 2021-03-11  1624  /*
76b21f18b22a6b Shawn Guo 2021-03-11  1625   * ACPI DSDT has one single memory resource for TLMM, which voilates the
76b21f18b22a6b Shawn Guo 2021-03-11  1626   * hardware layout of 3 sepearte tiles.  Let's split the memory resource into
76b21f18b22a6b Shawn Guo 2021-03-11  1627   * 3 named ones, so that msm_pinctrl_probe() can map memory for ACPI in the
76b21f18b22a6b Shawn Guo 2021-03-11  1628   * same way as for DT probe.
76b21f18b22a6b Shawn Guo 2021-03-11  1629   */
76b21f18b22a6b Shawn Guo 2021-03-11  1630  static int sc8180x_pinctrl_add_tile_resources(struct platform_device *pdev)
76b21f18b22a6b Shawn Guo 2021-03-11  1631  {
76b21f18b22a6b Shawn Guo 2021-03-11  1632  	int nres_num = pdev->num_resources + ARRAY_SIZE(sc8180x_tiles) - 1;
76b21f18b22a6b Shawn Guo 2021-03-11  1633  	struct resource *mres, *nres, *res;
76b21f18b22a6b Shawn Guo 2021-03-11  1634  	int i, ret;
76b21f18b22a6b Shawn Guo 2021-03-11  1635  
76b21f18b22a6b Shawn Guo 2021-03-11  1636  	/*
76b21f18b22a6b Shawn Guo 2021-03-11  1637  	 * DT already has tiles defined properly, so nothing needs to be done
76b21f18b22a6b Shawn Guo 2021-03-11  1638  	 * for DT probe.
76b21f18b22a6b Shawn Guo 2021-03-11  1639  	 */
76b21f18b22a6b Shawn Guo 2021-03-11  1640  	if (pdev->dev.of_node)
76b21f18b22a6b Shawn Guo 2021-03-11  1641  		return 0;
76b21f18b22a6b Shawn Guo 2021-03-11  1642  
76b21f18b22a6b Shawn Guo 2021-03-11  1643  	/* Allocate for new resources */
76b21f18b22a6b Shawn Guo 2021-03-11  1644  	nres = devm_kzalloc(&pdev->dev, sizeof(*nres) * nres_num, GFP_KERNEL);
76b21f18b22a6b Shawn Guo 2021-03-11  1645  	if (!nres)
76b21f18b22a6b Shawn Guo 2021-03-11  1646  		return -ENOMEM;
76b21f18b22a6b Shawn Guo 2021-03-11  1647  
76b21f18b22a6b Shawn Guo 2021-03-11  1648  	res = nres;
76b21f18b22a6b Shawn Guo 2021-03-11  1649  
76b21f18b22a6b Shawn Guo 2021-03-11  1650  	for (i = 0; i < pdev->num_resources; i++) {
76b21f18b22a6b Shawn Guo 2021-03-11  1651  		struct resource *r = &pdev->resource[i];
76b21f18b22a6b Shawn Guo 2021-03-11  1652  
76b21f18b22a6b Shawn Guo 2021-03-11  1653  		/* Save memory resource and copy others */
76b21f18b22a6b Shawn Guo 2021-03-11  1654  		if (resource_type(r) == IORESOURCE_MEM)
76b21f18b22a6b Shawn Guo 2021-03-11  1655  			mres = r;
76b21f18b22a6b Shawn Guo 2021-03-11  1656  		else
76b21f18b22a6b Shawn Guo 2021-03-11  1657  			*res++ = *r;
76b21f18b22a6b Shawn Guo 2021-03-11  1658  	}
76b21f18b22a6b Shawn Guo 2021-03-11  1659  
76b21f18b22a6b Shawn Guo 2021-03-11  1660  	/* Append tile memory resources */
76b21f18b22a6b Shawn Guo 2021-03-11  1661  	for (i = 0; i < ARRAY_SIZE(sc8180x_tiles); i++, res++) {
76b21f18b22a6b Shawn Guo 2021-03-11  1662  		const struct tile_info *info = &sc8180x_tile_info[i];
76b21f18b22a6b Shawn Guo 2021-03-11  1663  
76b21f18b22a6b Shawn Guo 2021-03-11 @1664  		res->start = mres->start + info->offset;
76b21f18b22a6b Shawn Guo 2021-03-11  1665  		res->end = mres->start + info->offset + info->size - 1;
76b21f18b22a6b Shawn Guo 2021-03-11  1666  		res->flags = mres->flags;
76b21f18b22a6b Shawn Guo 2021-03-11  1667  		res->name = sc8180x_tiles[i];
76b21f18b22a6b Shawn Guo 2021-03-11  1668  
76b21f18b22a6b Shawn Guo 2021-03-11  1669  		/* Add new MEM to resource tree */
76b21f18b22a6b Shawn Guo 2021-03-11  1670  		insert_resource(mres->parent, res);
76b21f18b22a6b Shawn Guo 2021-03-11  1671  	}
76b21f18b22a6b Shawn Guo 2021-03-11  1672  
76b21f18b22a6b Shawn Guo 2021-03-11  1673  	/* Remove old MEM from resource tree */
76b21f18b22a6b Shawn Guo 2021-03-11  1674  	remove_resource(mres);
76b21f18b22a6b Shawn Guo 2021-03-11  1675  
76b21f18b22a6b Shawn Guo 2021-03-11  1676  	/* Free old resources and install new ones */
76b21f18b22a6b Shawn Guo 2021-03-11  1677  	ret = platform_device_add_resources(pdev, nres, nres_num);
76b21f18b22a6b Shawn Guo 2021-03-11  1678  	if (ret) {
76b21f18b22a6b Shawn Guo 2021-03-11  1679  		dev_err(&pdev->dev, "failed to add new resources: %d\n", ret);
76b21f18b22a6b Shawn Guo 2021-03-11  1680  		return ret;
76b21f18b22a6b Shawn Guo 2021-03-11  1681  	}
76b21f18b22a6b Shawn Guo 2021-03-11  1682  
76b21f18b22a6b Shawn Guo 2021-03-11  1683  	return 0;
76b21f18b22a6b Shawn Guo 2021-03-11  1684  }
76b21f18b22a6b Shawn Guo 2021-03-11  1685  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29405 bytes --]

Re: [PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[Bjorn Andersson]

On Wed 10 Mar 18:41 PST 2021, Shawn Guo wrote:

> It adds ACPI probe support for pinctrl-sc8180x driver.  We have one
> problem with ACPI table, i.e. GIO0 (TLMM) block has one single memory
> resource to cover 3 tiles defined by SC8180X.  To follow the hardware
> layout of 3 tiles which is already supported DT probe, it adds one
> function to replace the original single memory resource with 3 named
> ones for tiles.  With that, We can map memory for ACPI in the same way
> as DT.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes for v5:
> - Keep .ngpios number as 190 to match SoC spec.
> - Add comments for sc8180x_pinctrl_add_tile_resources().
> - Drop redundant error message.
> 
> Changes for v4:
> - Add sc8180x_pinctrl_add_tile_resources() to massage memory resource
>   for ACPI probe.
> 
> Changes for v3:
> - Remove the use of tiles completely.
> - Drop unneed include of acpi.h.
> 
> Changes for v2:
> - Pass soc_data pointer via .driver_data.
> - Drop use of CONFIG_ACPI and ACPI_PTR().
> - Add comment for sc8180x_acpi_reserved_gpios[] terminator.
> 
>  drivers/pinctrl/qcom/Kconfig           |   2 +-
>  drivers/pinctrl/qcom/pinctrl-sc8180x.c | 123 ++++++++++++++++++++++++-
>  2 files changed, 122 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> index 6853a896c476..9f0218c4f9b3 100644
> --- a/drivers/pinctrl/qcom/Kconfig
> +++ b/drivers/pinctrl/qcom/Kconfig
> @@ -222,7 +222,7 @@ config PINCTRL_SC7280
>  
>  config PINCTRL_SC8180X
>  	tristate "Qualcomm Technologies Inc SC8180x pin controller driver"
> -	depends on GPIOLIB && OF
> +	depends on GPIOLIB && (OF || ACPI)
>  	select PINCTRL_MSM
>  	help
>  	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc8180x.c b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> index b765bf667574..0d9654b4ab60 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> @@ -23,6 +23,21 @@ enum {
>  	WEST
>  };
>  
> +/*
> + * ACPI DSDT has one single memory resource for TLMM.  The offsets below are
> + * used to locate different tiles for ACPI probe.
> + */
> +struct tile_info {
> +	u32 offset;
> +	u32 size;
> +};
> +
> +static const struct tile_info sc8180x_tile_info[] = {
> +	{ 0x00d00000, 0x00300000, },
> +	{ 0x00500000, 0x00700000, },
> +	{ 0x00100000, 0x00300000, },
> +};
> +
>  #define FUNCTION(fname)					\
>  	[msm_mux_##fname] = {				\
>  		.name = #fname,				\
> @@ -1557,6 +1572,13 @@ static const struct msm_pingroup sc8180x_groups[] = {
>  	[193] = SDC_QDSD_PINGROUP(sdc2_data, 0x4b2000, 9, 0),
>  };
>  
> +static const int sc8180x_acpi_reserved_gpios[] = {
> +	0, 1, 2, 3,
> +	47, 48, 49, 50,
> +	126, 127, 128, 129,
> +	-1 /* terminator */
> +};
> +
>  static const struct msm_gpio_wakeirq_map sc8180x_pdc_map[] = {
>  	{ 3, 31 }, { 5, 32 }, { 8, 33 }, { 9, 34 }, { 10, 100 }, { 12, 104 },
>  	{ 24, 37 }, { 26, 38 }, { 27, 41 }, { 28, 42 }, { 30, 39 }, { 36, 43 },
> @@ -1588,13 +1610,109 @@ static struct msm_pinctrl_soc_data sc8180x_pinctrl = {
>  	.nwakeirq_map = ARRAY_SIZE(sc8180x_pdc_map),
>  };
>  
> +static const struct msm_pinctrl_soc_data sc8180x_acpi_pinctrl = {
> +	.tiles = sc8180x_tiles,
> +	.ntiles = ARRAY_SIZE(sc8180x_tiles),
> +	.pins = sc8180x_pins,
> +	.npins = ARRAY_SIZE(sc8180x_pins),
> +	.groups = sc8180x_groups,
> +	.ngroups = ARRAY_SIZE(sc8180x_groups),
> +	.reserved_gpios = sc8180x_acpi_reserved_gpios,
> +	.ngpios = 190,
> +};
> +
> +/*
> + * ACPI DSDT has one single memory resource for TLMM, which voilates the
> + * hardware layout of 3 sepearte tiles.  Let's split the memory resource into
> + * 3 named ones, so that msm_pinctrl_probe() can map memory for ACPI in the
> + * same way as for DT probe.
> + */
> +static int sc8180x_pinctrl_add_tile_resources(struct platform_device *pdev)
> +{
> +	int nres_num = pdev->num_resources + ARRAY_SIZE(sc8180x_tiles) - 1;
> +	struct resource *mres, *nres, *res;
> +	int i, ret;
> +
> +	/*
> +	 * DT already has tiles defined properly, so nothing needs to be done
> +	 * for DT probe.
> +	 */
> +	if (pdev->dev.of_node)
> +		return 0;
> +
> +	/* Allocate for new resources */
> +	nres = devm_kzalloc(&pdev->dev, sizeof(*nres) * nres_num, GFP_KERNEL);
> +	if (!nres)
> +		return -ENOMEM;
> +
> +	res = nres;
> +
> +	for (i = 0; i < pdev->num_resources; i++) {
> +		struct resource *r = &pdev->resource[i];
> +
> +		/* Save memory resource and copy others */
> +		if (resource_type(r) == IORESOURCE_MEM)
> +			mres = r;
> +		else
> +			*res++ = *r;
> +	}
> +
> +	/* Append tile memory resources */
> +	for (i = 0; i < ARRAY_SIZE(sc8180x_tiles); i++, res++) {
> +		const struct tile_info *info = &sc8180x_tile_info[i];
> +
> +		res->start = mres->start + info->offset;
> +		res->end = mres->start + info->offset + info->size - 1;
> +		res->flags = mres->flags;
> +		res->name = sc8180x_tiles[i];
> +
> +		/* Add new MEM to resource tree */
> +		insert_resource(mres->parent, res);
> +	}
> +
> +	/* Remove old MEM from resource tree */
> +	remove_resource(mres);
> +
> +	/* Free old resources and install new ones */
> +	ret = platform_device_add_resources(pdev, nres, nres_num);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to add new resources: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int sc8180x_pinctrl_probe(struct platform_device *pdev)
>  {
> -	return msm_pinctrl_probe(pdev, &sc8180x_pinctrl);
> +	const struct msm_pinctrl_soc_data *soc_data;
> +	int ret;
> +
> +	soc_data = device_get_match_data(&pdev->dev);
> +	if (!soc_data)
> +		return -EINVAL;
> +
> +	ret = sc8180x_pinctrl_add_tile_resources(pdev);
> +	if (ret)
> +		return ret;
> +
> +	return msm_pinctrl_probe(pdev, soc_data);
>  }
>  
> +static const struct acpi_device_id sc8180x_pinctrl_acpi_match[] = {
> +	{
> +		.id = "QCOM040D",
> +		.driver_data = (kernel_ulong_t) &sc8180x_acpi_pinctrl,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, sc8180x_pinctrl_acpi_match);
> +
>  static const struct of_device_id sc8180x_pinctrl_of_match[] = {
> -	{ .compatible = "qcom,sc8180x-tlmm", },
> +	{
> +		.compatible = "qcom,sc8180x-tlmm",
> +		.data = &sc8180x_pinctrl,
> +	},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, sc8180x_pinctrl_of_match);
> @@ -1603,6 +1721,7 @@ static struct platform_driver sc8180x_pinctrl_driver = {
>  	.driver = {
>  		.name = "sc8180x-pinctrl",
>  		.of_match_table = sc8180x_pinctrl_of_match,
> +		.acpi_match_table = sc8180x_pinctrl_acpi_match,
>  	},
>  	.probe = sc8180x_pinctrl_probe,
>  	.remove = msm_pinctrl_remove,
> -- 
> 2.17.1
> 

Re: [PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[Andy Shevchenko]

On Thu, Mar 11, 2021 at 10:41:02AM +0800, Shawn Guo wrote:
> It adds ACPI probe support for pinctrl-sc8180x driver.  We have one
> problem with ACPI table, i.e. GIO0 (TLMM) block has one single memory
> resource to cover 3 tiles defined by SC8180X.  To follow the hardware
> layout of 3 tiles which is already supported DT probe, it adds one
> function to replace the original single memory resource with 3 named
> ones for tiles.  With that, We can map memory for ACPI in the same way
> as DT.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

(See some minor comments below)

> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes for v5:
> - Keep .ngpios number as 190 to match SoC spec.
> - Add comments for sc8180x_pinctrl_add_tile_resources().
> - Drop redundant error message.
> 
> Changes for v4:
> - Add sc8180x_pinctrl_add_tile_resources() to massage memory resource
>   for ACPI probe.
> 
> Changes for v3:
> - Remove the use of tiles completely.
> - Drop unneed include of acpi.h.
> 
> Changes for v2:
> - Pass soc_data pointer via .driver_data.
> - Drop use of CONFIG_ACPI and ACPI_PTR().
> - Add comment for sc8180x_acpi_reserved_gpios[] terminator.
> 
>  drivers/pinctrl/qcom/Kconfig           |   2 +-
>  drivers/pinctrl/qcom/pinctrl-sc8180x.c | 123 ++++++++++++++++++++++++-
>  2 files changed, 122 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> index 6853a896c476..9f0218c4f9b3 100644
> --- a/drivers/pinctrl/qcom/Kconfig
> +++ b/drivers/pinctrl/qcom/Kconfig
> @@ -222,7 +222,7 @@ config PINCTRL_SC7280
>  
>  config PINCTRL_SC8180X
>  	tristate "Qualcomm Technologies Inc SC8180x pin controller driver"
> -	depends on GPIOLIB && OF
> +	depends on GPIOLIB && (OF || ACPI)
>  	select PINCTRL_MSM
>  	help
>  	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc8180x.c b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> index b765bf667574..0d9654b4ab60 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> @@ -23,6 +23,21 @@ enum {
>  	WEST
>  };
>  
> +/*
> + * ACPI DSDT has one single memory resource for TLMM.  The offsets below are

One space is enough.

> + * used to locate different tiles for ACPI probe.
> + */
> +struct tile_info {
> +	u32 offset;
> +	u32 size;
> +};
> +
> +static const struct tile_info sc8180x_tile_info[] = {
> +	{ 0x00d00000, 0x00300000, },
> +	{ 0x00500000, 0x00700000, },
> +	{ 0x00100000, 0x00300000, },
> +};
> +
>  #define FUNCTION(fname)					\
>  	[msm_mux_##fname] = {				\
>  		.name = #fname,				\
> @@ -1557,6 +1572,13 @@ static const struct msm_pingroup sc8180x_groups[] = {
>  	[193] = SDC_QDSD_PINGROUP(sdc2_data, 0x4b2000, 9, 0),
>  };
>  
> +static const int sc8180x_acpi_reserved_gpios[] = {
> +	0, 1, 2, 3,
> +	47, 48, 49, 50,
> +	126, 127, 128, 129,
> +	-1 /* terminator */
> +};
> +
>  static const struct msm_gpio_wakeirq_map sc8180x_pdc_map[] = {
>  	{ 3, 31 }, { 5, 32 }, { 8, 33 }, { 9, 34 }, { 10, 100 }, { 12, 104 },
>  	{ 24, 37 }, { 26, 38 }, { 27, 41 }, { 28, 42 }, { 30, 39 }, { 36, 43 },
> @@ -1588,13 +1610,109 @@ static struct msm_pinctrl_soc_data sc8180x_pinctrl = {
>  	.nwakeirq_map = ARRAY_SIZE(sc8180x_pdc_map),
>  };
>  
> +static const struct msm_pinctrl_soc_data sc8180x_acpi_pinctrl = {
> +	.tiles = sc8180x_tiles,
> +	.ntiles = ARRAY_SIZE(sc8180x_tiles),
> +	.pins = sc8180x_pins,
> +	.npins = ARRAY_SIZE(sc8180x_pins),
> +	.groups = sc8180x_groups,
> +	.ngroups = ARRAY_SIZE(sc8180x_groups),
> +	.reserved_gpios = sc8180x_acpi_reserved_gpios,
> +	.ngpios = 190,
> +};
> +
> +/*
> + * ACPI DSDT has one single memory resource for TLMM, which voilates the
> + * hardware layout of 3 sepearte tiles.  Let's split the memory resource into
> + * 3 named ones, so that msm_pinctrl_probe() can map memory for ACPI in the
> + * same way as for DT probe.
> + */
> +static int sc8180x_pinctrl_add_tile_resources(struct platform_device *pdev)
> +{
> +	int nres_num = pdev->num_resources + ARRAY_SIZE(sc8180x_tiles) - 1;
> +	struct resource *mres, *nres, *res;
> +	int i, ret;
> +
> +	/*
> +	 * DT already has tiles defined properly, so nothing needs to be done
> +	 * for DT probe.
> +	 */
> +	if (pdev->dev.of_node)
> +		return 0;
> +
> +	/* Allocate for new resources */
> +	nres = devm_kzalloc(&pdev->dev, sizeof(*nres) * nres_num, GFP_KERNEL);

devm_kcalloc()

> +	if (!nres)
> +		return -ENOMEM;
> +
> +	res = nres;
> +
> +	for (i = 0; i < pdev->num_resources; i++) {
> +		struct resource *r = &pdev->resource[i];
> +
> +		/* Save memory resource and copy others */
> +		if (resource_type(r) == IORESOURCE_MEM)
> +			mres = r;
> +		else
> +			*res++ = *r;
> +	}
> +
> +	/* Append tile memory resources */
> +	for (i = 0; i < ARRAY_SIZE(sc8180x_tiles); i++, res++) {
> +		const struct tile_info *info = &sc8180x_tile_info[i];

> +		res->start = mres->start + info->offset;
> +		res->end = mres->start + info->offset + info->size - 1;
> +		res->flags = mres->flags;
> +		res->name = sc8180x_tiles[i];
> +		/* Add new MEM to resource tree */
> +		insert_resource(mres->parent, res);
> +	}
> +
> +	/* Remove old MEM from resource tree */
> +	remove_resource(mres);
> +
> +	/* Free old resources and install new ones */
> +	ret = platform_device_add_resources(pdev, nres, nres_num);

> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to add new resources: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;

if (ret)
	dev_err(...);
return ret;

?

> +}
> +
>  static int sc8180x_pinctrl_probe(struct platform_device *pdev)
>  {
> -	return msm_pinctrl_probe(pdev, &sc8180x_pinctrl);
> +	const struct msm_pinctrl_soc_data *soc_data;
> +	int ret;
> +
> +	soc_data = device_get_match_data(&pdev->dev);
> +	if (!soc_data)
> +		return -EINVAL;
> +
> +	ret = sc8180x_pinctrl_add_tile_resources(pdev);
> +	if (ret)
> +		return ret;
> +
> +	return msm_pinctrl_probe(pdev, soc_data);
>  }
>  
> +static const struct acpi_device_id sc8180x_pinctrl_acpi_match[] = {
> +	{
> +		.id = "QCOM040D",
> +		.driver_data = (kernel_ulong_t) &sc8180x_acpi_pinctrl,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, sc8180x_pinctrl_acpi_match);
> +
>  static const struct of_device_id sc8180x_pinctrl_of_match[] = {
> -	{ .compatible = "qcom,sc8180x-tlmm", },
> +	{
> +		.compatible = "qcom,sc8180x-tlmm",
> +		.data = &sc8180x_pinctrl,
> +	},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, sc8180x_pinctrl_of_match);
> @@ -1603,6 +1721,7 @@ static struct platform_driver sc8180x_pinctrl_driver = {
>  	.driver = {
>  		.name = "sc8180x-pinctrl",
>  		.of_match_table = sc8180x_pinctrl_of_match,
> +		.acpi_match_table = sc8180x_pinctrl_acpi_match,
>  	},
>  	.probe = sc8180x_pinctrl_probe,
>  	.remove = msm_pinctrl_remove,
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[Linus Walleij]

On Thu, Mar 11, 2021 at 3:41 AM Shawn Guo <shawn.guo@linaro.org> wrote:

> It adds ACPI probe support for pinctrl-sc8180x driver.  We have one
> problem with ACPI table, i.e. GIO0 (TLMM) block has one single memory
> resource to cover 3 tiles defined by SC8180X.  To follow the hardware
> layout of 3 tiles which is already supported DT probe, it adds one
> function to replace the original single memory resource with 3 named
> ones for tiles.  With that, We can map memory for ACPI in the same way
> as DT.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes for v5:
> - Keep .ngpios number as 190 to match SoC spec.
> - Add comments for sc8180x_pinctrl_add_tile_resources().
> - Drop redundant error message.

This v5 version applied!
Thanks for your perseverance and excellent work as always Shawn!

Special thanks to Andy for helping out in getting all ACPI details right!

Yours,
Linus Walleij

Re: [PATCH v5] pinctrl: qcom: sc8180x: add ACPI probe support

[patchwork-bot+linux-arm-msm]

Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Thu, 11 Mar 2021 10:41:02 +0800 you wrote:
> It adds ACPI probe support for pinctrl-sc8180x driver.  We have one
> problem with ACPI table, i.e. GIO0 (TLMM) block has one single memory
> resource to cover 3 tiles defined by SC8180X.  To follow the hardware
> layout of 3 tiles which is already supported DT probe, it adds one
> function to replace the original single memory resource with 3 named
> ones for tiles.  With that, We can map memory for ACPI in the same way
> as DT.
> 
> [...]

Here is the summary with links:
  - [v5] pinctrl: qcom: sc8180x: add ACPI probe support
    https://git.kernel.org/qcom/c/6d8d67988b1a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html