Re: [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration

[Andrew Jeffery]

On Fri, 2026-11-06 at 07:59 +0530, Prasanth Kumar Padarthi wrote:
> The shared Aspeed probe helper registers its controller with
> pinctrl_register(), but does not unregister it when the device is
> unbound. 
> 

This is a problem in its own right. IMO the rest of the discussion can
be left out, as it depends on the subsequent change, which still needs
consideration.

> Registration can therefore outlive device-managed callback
> data, leaving a registered controller with pointers to freed memory.
> The G6 handover change in this series embeds that data in a
> device-managed allocation.
> 
> Use devm_pinctrl_register() so controller teardown runs before earlier
> devres allocations are released. This also cleans up the registration
> if later probe processing fails.
> 
> With this change and the G6 handover patch, one unbind/rebind cycle
> passed under generic KASAN in AST2600 QEMU and on B650D4U hardware.

IMO this sentence is enough to describe the testing, the rest reads
like unnecessary detail that tends to be generated by LLMs.

> Controller debugfs entries disappeared on unbind and returned on
> rebind; a subsequent pins read succeeded without observed KASAN
> reports. The unfixed control case has not been runtime-tested.

What is this last sentence referring to? 

> 
> Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@xxxxxxxxx>
> ---
>  drivers/pinctrl/aspeed/pinctrl-aspeed.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> index 7e0ebf11a..d19b4d23c 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> @@ -461,7 +461,8 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
>  
>  	pdata->pinmux.maps[ASPEED_IP_SCU] = pdata->scu;
>  
> -	pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
> +	/* Unregister before earlier devres allocations such as pdata are freed. */

I'd rather the comment is removed.

Cheers,

Andrew

> +	pctl = devm_pinctrl_register(&pdev->dev, pdesc, pdata);
>  
>  	if (IS_ERR(pctl)) {
>  		dev_err(&pdev->dev, "Failed to register pinctrl\n");