Re: [PATCH] pinctrl: pistachio: Use scope-based resource management in pistachio_gpio_register()

[Jonathan Cameron <jic23@xxxxxxxxxx>]

On Mon, 17 Jun 2024 10:32:03 +0200
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

> On Wed, Jun 5, 2024 at 6:02 PM Markus Elfring <Markus.Elfring@xxxxxx> wrote:
> 
> > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> > Date: Wed, 5 Jun 2024 17:46:52 +0200
> >
> > Scope-based resource management became supported also for another
> > programming interface by contributions of Jonathan Cameron on 2024-02-17.
> > See also the commit 59ed5e2d505bf5f9b4af64d0021cd0c96aec1f7c ("device
> > property: Add cleanup.h based fwnode_handle_put() scope based cleanup.").
> >
> > * Thus use the attribute “__free(fwnode_handle)”.
> >
> > * Reduce the scope for the local variable “child”.
> >
> > * Omit explicit fwnode_handle_put() calls accordingly.
> >
> > Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>  
> 
> Looks reasonable but I'd like Jonathan's and Andy's review tags on this.
> 
> Yours,
> Linus Walleij

I took a look and it makes me a little nervous. Markus' patch isn't
changing behaviour (I think) but i is mixing scoped handling with non
scoped handling which I'm not keen on. I'm also struggling to understand
how the code was right prior to this patch.  In particular what cleans
up the fwnode_handle after the point where Markus has used no_free_ptr()?

Whilst it's assigned to the gpio_chip that chip hasn't been added when
girq->parents is allocated so even if there is some magic path to clearing
it up I'm not immediately seeing how that can be working here.

So this feels like there are more gremlins hiding here and Markus' patch
may not help flush them out.

Jonathan