Re: [PATCH 1/3] PCI/pwrctrl: Fix device leak at registration
From: Johan Hovold
Date: Tue Jul 22 2025 - 08:01:13 EST
On Tue, Jul 22, 2025 at 11:05:26AM +0100, Jonathan Cameron wrote:
> On Mon, 21 Jul 2025 17:36:07 +0200
> Johan Hovold <johan+linaro@xxxxxxxxxx> wrote:
> Perhaps time for
> DEFINE_FREE(put_pdev, struct platform_device *, if (_T) put_device(&_T->dev));
>
> then...
>
> > ---
> > drivers/pci/bus.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> > index 69048869ef1c..0394a9c77b38 100644
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -362,11 +362,15 @@ void pci_bus_add_device(struct pci_dev *dev)
> > * before PCI client drivers.
> > */
> > pdev = of_find_device_by_node(dn);
> > - if (pdev && of_pci_supply_present(dn)) {
> > - if (!device_link_add(&dev->dev, &pdev->dev,
> > - DL_FLAG_AUTOREMOVE_CONSUMER))
> > - pci_err(dev, "failed to add device link to power control device %s\n",
> > - pdev->name);
>
> struct platform_device *pdev __free(put_pdev) =
> of_find_device_by_node(dn);
> > + if (pdev) {
> > + if (of_pci_supply_present(dn)) {
> > + if (!device_link_add(&dev->dev, &pdev->dev,
> > + DL_FLAG_AUTOREMOVE_CONSUMER)) {
> > + pci_err(dev, "failed to add device link to power control device %s\n",
> > + pdev->name);
> > + }
> > + }
> > + put_device(&pdev->dev);
>
> and no need for any explicit put.
>
> We already do this extensively in some subsystems (e.g. CXL) and it
> greatly simplifies code.
No, I'm no fan of those kind of changes which I find leads to less
readable code (e.g. with those in-code declarations).
Johan