mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Jeffery <andrew@codeconstruct.com.au>
To: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>,
	 linusw@kernel.org
Cc: joel@jms.id.au, ryan_chen@aspeedtech.com,
	billy_tsai@aspeedtech.com,  brgl@kernel.org,
	tan.siewert@9elements.com, linux-aspeed@lists.ozlabs.org,
	 linux-gpio@vger.kernel.org, openbmc@lists.ozlabs.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration
Date: Fri, 06 Nov 2026 16:43:41 +1030	[thread overview]
Message-ID: <8afcccd35b8a78730c6a9cc46c30dbeb99694e6e.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20261106022907.10735-2-prasanth.padarthi10@gmail.com>

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@gmail.com>
> ---
>  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");

  reply	other threads:[~2026-11-06  6:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15  3:51 [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Prasanth Kumar Padarthi
2026-10-22  2:43 ` [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment Prasanth Kumar Padarthi
2026-10-22  2:43   ` [RFC PATCH v2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
2026-11-05 15:23   ` [PATCH v3] " Prasanth Kumar Padarthi
2026-11-06  2:28     ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Prasanth Kumar Padarthi
2026-11-06  2:29     ` Prasanth Kumar Padarthi
2026-11-06  2:29       ` [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration Prasanth Kumar Padarthi
2026-11-06  6:13         ` Andrew Jeffery [this message]
2026-11-06  2:29       ` [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
2026-11-06 10:54         ` Andrew Jeffery
2026-11-06  5:25       ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Andrew Jeffery
2026-11-04 12:19 ` [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Linus Walleij
2026-11-05  3:36   ` Prasanth
2026-11-05 13:27     ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8afcccd35b8a78730c6a9cc46c30dbeb99694e6e.camel@codeconstruct.com.au \
    --to=andrew@codeconstruct.com.au \
    --cc=billy_tsai@aspeedtech.com \
    --cc=brgl@kernel.org \
    --cc=joel@jms.id.au \
    --cc=linusw@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=prasanth.padarthi10@gmail.com \
    --cc=ryan_chen@aspeedtech.com \
    --cc=tan.siewert@9elements.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®