Re: [PATCH] spi: cs42l43: Clean up of firmware node

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Thu, Feb 1, 2024 at 4:43 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Thu, Feb 1, 2024 at 12:44 PM Charles Keepax
> <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > As we get a child node in the of case, we should also clean up the

OF

> > references to this, add code for this.

...

> > +       if (is_of_node(fwnode))
> > +               fwnode = fwnode_get_named_child_node(fwnode, "spi");
> > +
> > +       device_set_node(&priv->ctlr->dev, fwnode);
> > +
> >         ret = devm_spi_register_controller(priv->dev, priv->ctlr);
> >         if (ret) {
> >                 dev_err(priv->dev, "Failed to register SPI controller: %d\n", ret);
> > +               goto of_node_err;
> >         }
> >
> > +       return 0;
> > +
> > +of_node_err:
> > +       if (is_of_node(fwnode))
> > +               fwnode_handle_put(fwnode);
>
> Wrong order. What we need is to wrap this by
> devm_add_action_or_reset(), perhaps even providing
> devm_fwnode_handle_get() for everyone.
>
> >         return ret;
> >  }

...

> > +static void cs42l43_spi_remove(struct platform_device *pdev)
> > +{
> > +       struct cs42l43_spi *priv = dev_get_drvdata(&pdev->dev);
> > +
> > +       if (is_of_node(priv->ctlr->dev.fwnode))
> > +               fwnode_handle_put(priv->ctlr->dev.fwnode);
> > +}
>
> No need as per above. If you send a new patch series where you add
> that API, I'll review / ack it immediately.

Oh, just realised that this is not a simple fwnode_handle_get()...
So, we need to add an action then with devm_add_action(). No need for
a separate patch.

-- 
With Best Regards,
Andy Shevchenko