[PATCH] pinctrl: intel: use FAM for communities

[PATCH] pinctrl: intel: use FAM for communities

[Rosen Penev]

Use a flexible array member to combine allocations into one.

Add __counted_by for extra runtime analysis. Move counting variable
assignments to after allocation as required by __counted_by.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/pinctrl/intel/pinctrl-baytrail.c   | 43 +++++++---------------
 drivers/pinctrl/intel/pinctrl-cherryview.c | 11 ++----
 drivers/pinctrl/intel/pinctrl-intel.c      | 15 ++------
 drivers/pinctrl/intel/pinctrl-intel.h      |  2 +-
 drivers/pinctrl/intel/pinctrl-lynxpoint.c  |  9 +----
 5 files changed, 25 insertions(+), 55 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
index b733ec31ad9d..e73419498857 100644
--- a/drivers/pinctrl/intel/pinctrl-baytrail.c
+++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
@@ -1558,31 +1558,6 @@ static int byt_gpio_probe(struct intel_pinctrl *vg)
 	return 0;
 }
 
-static int byt_set_soc_data(struct intel_pinctrl *vg,
-			    const struct intel_pinctrl_soc_data *soc)
-{
-	struct platform_device *pdev = to_platform_device(vg->dev);
-	int i;
-
-	vg->soc = soc;
-
-	vg->ncommunities = vg->soc->ncommunities;
-	vg->communities = devm_kmemdup_array(vg->dev, vg->soc->communities, vg->ncommunities,
-					     sizeof(*vg->soc->communities), GFP_KERNEL);
-	if (!vg->communities)
-		return -ENOMEM;
-
-	for (i = 0; i < vg->soc->ncommunities; i++) {
-		struct intel_community *comm = vg->communities + i;
-
-		comm->pad_regs = devm_platform_ioremap_resource(pdev, 0);
-		if (IS_ERR(comm->pad_regs))
-			return PTR_ERR(comm->pad_regs);
-	}
-
-	return 0;
-}
-
 static const struct acpi_device_id byt_gpio_acpi_match[] = {
 	{ "INT33B2", (kernel_ulong_t)byt_soc_data },
 	{ "INT33FC", (kernel_ulong_t)byt_soc_data },
@@ -1595,19 +1570,29 @@ static int byt_pinctrl_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct intel_pinctrl *vg;
 	int ret;
+	int i;
 
 	soc_data = intel_pinctrl_get_soc_data(pdev);
 	if (IS_ERR(soc_data))
 		return PTR_ERR(soc_data);
 
-	vg = devm_kzalloc(dev, sizeof(*vg), GFP_KERNEL);
+	vg = devm_kzalloc(dev, struct_size(vg, communities, soc_data->ncommunities), GFP_KERNEL);
 	if (!vg)
 		return -ENOMEM;
 
+	vg->ncommunities = soc_data->ncommunities;
+	memcpy(vg->communities, soc_data->communities, soc_data->ncommunities * sizeof(*vg->communities));
+
 	vg->dev = dev;
-	ret = byt_set_soc_data(vg, soc_data);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to set soc data\n");
+	vg->soc = soc_data;
+
+	for (i = 0; i < vg->soc->ncommunities; i++) {
+		struct intel_community *comm = vg->communities + i;
+
+		comm->pad_regs = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(comm->pad_regs))
+			return PTR_ERR(comm->pad_regs);
+	}
 
 	vg->pctldesc		= byt_pinctrl_desc;
 	vg->pctldesc.name	= dev_name(dev);
diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index a33eca5eafc4..e9b062b44fea 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1595,19 +1595,16 @@ static int chv_pinctrl_probe(struct platform_device *pdev)
 	if (IS_ERR(soc_data))
 		return PTR_ERR(soc_data);
 
-	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
+	pctrl = devm_kzalloc(dev, struct_size(pctrl, communities, soc_data->ncommunities), GFP_KERNEL);
 	if (!pctrl)
 		return -ENOMEM;
 
+	pctrl->ncommunities = soc_data->ncommunities;
+	memcpy(pctrl->communities, soc_data->communities, soc_data->ncommunities * sizeof(*pctrl->communities));
+
 	pctrl->dev = dev;
 	pctrl->soc = soc_data;
 
-	pctrl->ncommunities = pctrl->soc->ncommunities;
-	pctrl->communities = devm_kmemdup_array(dev, pctrl->soc->communities, pctrl->ncommunities,
-						sizeof(*pctrl->soc->communities), GFP_KERNEL);
-	if (!pctrl->communities)
-		return -ENOMEM;
-
 	community = &pctrl->communities[0];
 	community->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(community->regs))
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 97bf5ec78db4..635e4e5ab10c 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1581,24 +1581,17 @@ int intel_pinctrl_probe(struct platform_device *pdev,
 	struct intel_pinctrl *pctrl;
 	int i, ret, irq;
 
-	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
+	pctrl = devm_kzalloc(dev, struct_size(pctrl, communities, soc_data->ncommunities), GFP_KERNEL);
 	if (!pctrl)
 		return -ENOMEM;
 
+	pctrl->ncommunities = soc_data->ncommunities;
+	memcpy(pctrl->communities, soc_data->communities, soc_data->ncommunities * sizeof(*pctrl->communities));
+
 	pctrl->dev = dev;
 	pctrl->soc = soc_data;
 	raw_spin_lock_init(&pctrl->lock);
 
-	/*
-	 * Make a copy of the communities which we can use to hold pointers
-	 * to the registers.
-	 */
-	pctrl->ncommunities = pctrl->soc->ncommunities;
-	pctrl->communities = devm_kmemdup_array(dev, pctrl->soc->communities, pctrl->ncommunities,
-						sizeof(*pctrl->soc->communities), GFP_KERNEL);
-	if (!pctrl->communities)
-		return -ENOMEM;
-
 	for (i = 0; i < pctrl->ncommunities; i++) {
 		struct intel_community *community = &pctrl->communities[i];
 		unsigned short capability_offset[6];
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index b5476b9de0db..c382fe8f08c2 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -260,10 +260,10 @@ struct intel_pinctrl {
 	struct pinctrl_dev *pctldev;
 	struct gpio_chip chip;
 	const struct intel_pinctrl_soc_data *soc;
-	struct intel_community *communities;
 	size_t ncommunities;
 	struct intel_pinctrl_context context;
 	int irq;
+	struct intel_community communities[] __counted_by(ncommunities);
 };
 
 int intel_pinctrl_probe(struct platform_device *pdev,
diff --git a/drivers/pinctrl/intel/pinctrl-lynxpoint.c b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
index 299ee4f22bdc..b3cde7afded9 100644
--- a/drivers/pinctrl/intel/pinctrl-lynxpoint.c
+++ b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
@@ -715,19 +715,14 @@ static int lp_gpio_probe(struct platform_device *pdev)
 	if (!soc)
 		return -ENODEV;
 
-	lg = devm_kzalloc(dev, sizeof(*lg), GFP_KERNEL);
+	lg = devm_kzalloc(dev, struct_size(lg, communities, soc->ncommunities), GFP_KERNEL);
 	if (!lg)
 		return -ENOMEM;
 
+	lg->ncommunities = soc->ncommunities;
 	lg->dev = dev;
 	lg->soc = soc;
 
-	lg->ncommunities = lg->soc->ncommunities;
-	lg->communities = devm_kcalloc(dev, lg->ncommunities,
-				       sizeof(*lg->communities), GFP_KERNEL);
-	if (!lg->communities)
-		return -ENOMEM;
-
 	lg->pctldesc           = lptlp_pinctrl_desc;
 	lg->pctldesc.name      = dev_name(dev);
 	lg->pctldesc.pins      = lg->soc->pins;
-- 
2.53.0

Re: [PATCH] pinctrl: intel: use FAM for communities

[Andy Shevchenko]

On Wed, Mar 25, 2026 at 07:36:27PM -0700, Rosen Penev wrote:
> Use a flexible array member to combine allocations into one.
> 
> Add __counted_by for extra runtime analysis. Move counting variable
> assignments to after allocation as required by __counted_by.

I don't think it improves readability of the code. Also see below.

...

> -	vg = devm_kzalloc(dev, sizeof(*vg), GFP_KERNEL);
> +	vg = devm_kzalloc(dev, struct_size(vg, communities, soc_data->ncommunities), GFP_KERNEL);
>  	if (!vg)
>  		return -ENOMEM;
>  
> +	vg->ncommunities = soc_data->ncommunities;
> +	memcpy(vg->communities, soc_data->communities, soc_data->ncommunities * sizeof(*vg->communities));

This is a mess. The multiplication is used without overflow check and the parts
of it are from different data structures. If we do a such, we need to use
source data for the all of the information.

What I would like to see is rater special devm_kmalloc_and_dup()-like API
instead of doing this in every driver out of dozens of drivers.

...

> -	ret = byt_set_soc_data(vg, soc_data);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "failed to set soc data\n");
> +	vg->soc = soc_data;
> +
> +	for (i = 0; i < vg->soc->ncommunities; i++) {
> +		struct intel_community *comm = vg->communities + i;
> +
> +		comm->pad_regs = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(comm->pad_regs))
> +			return PTR_ERR(comm->pad_regs);
> +	}

Why is this can't be kept in a separate function as it was originally done?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] pinctrl: intel: use FAM for communities

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 12:35:57PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 25, 2026 at 07:36:27PM -0700, Rosen Penev wrote:

...

> What I would like to see is rater special devm_kmalloc_and_dup()-like API
> instead of doing this in every driver out of dozens of drivers.

You may start with providing a common API for all Intel pin control drivers.

Patch series of:
- add a new API and new member
- convert driver-by-driver in separate changes.

Actually why can't it be done like this from the start?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] pinctrl: intel: use FAM for communities

[Rosen Penev]

On Thu, Mar 26, 2026 at 3:41 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Mar 26, 2026 at 12:35:57PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 25, 2026 at 07:36:27PM -0700, Rosen Penev wrote:
>
> ...
>
> > What I would like to see is rater special devm_kmalloc_and_dup()-like API
> > instead of doing this in every driver out of dozens of drivers.
I defer to the hardening people for that. My suggestion was to
introduce a kmemdup_flex macro that does kzalloc_flex + memcpy on the
FAM.
>
> You may start with providing a common API for all Intel pin control drivers.
>
> Patch series of:
> - add a new API and new member
> - convert driver-by-driver in separate changes.
>
> Actually why can't it be done like this from the start?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH] pinctrl: intel: use FAM for communities

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 12:22:20PM -0700, Rosen Penev wrote:
> On Thu, Mar 26, 2026 at 3:41 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Thu, Mar 26, 2026 at 12:35:57PM +0200, Andy Shevchenko wrote:
> > > On Wed, Mar 25, 2026 at 07:36:27PM -0700, Rosen Penev wrote:

...

> > > What I would like to see is rater special devm_kmalloc_and_dup()-like API
> > > instead of doing this in every driver out of dozens of drivers.
> I defer to the hardening people for that. My suggestion was to
> introduce a kmemdup_flex macro that does kzalloc_flex + memcpy on the
> FAM.

Thanks, please come back with a better approach like kmemdup_flex().

> > You may start with providing a common API for all Intel pin control drivers.
> >
> > Patch series of:
> > - add a new API and new member
> > - convert driver-by-driver in separate changes.
> >
> > Actually why can't it be done like this from the start?

-- 
With Best Regards,
Andy Shevchenko