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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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