[PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup

[Nicolas Frattaroli]

On Monday, 27 October 2025 17:31:23 Central European Standard Time Anand Moon wrote:
> Hi Nicolas,
> 
> Thanks for your review comments.
> 
> On Mon, 27 Oct 2025 at 20:42, Nicolas Frattaroli
> <nicolas.frattaroli at collabora.com> wrote:
> >
> > On Monday, 27 October 2025 15:55:29 Central European Standard Time Anand Moon wrote:
> > > Introduce a .remove() callback to the Rockchip DesignWare PCIe
> > > controller driver to ensure proper resource deinitialization during
> > > device removal. This includes disabling clocks and deinitializing the
> > > PCIe PHY.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon at gmail.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > index 87dd2dd188b4..b878ae8e2b3e 100644
> > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > > @@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
> > >       return ret;
> > >  }
> > >
> > > +static void rockchip_pcie_remove(struct platform_device *pdev)
> > > +{
> > > +     struct device *dev = &pdev->dev;
> > > +     struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> > > +
> > > +     /* Perform other cleanups as necessary */
> > > +     clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
> > > +     rockchip_pcie_phy_deinit(rockchip);
> >
> > You may want to add a
> >
> >     if (rockchip->vpcie3v3)
> >             regulator_disable(rockchip->vpcie3v3);
> >
> > here, since it's enabled in the probe function if it's found.
> >
> > Not doing so means the regulator core will produce a warning
> > splat when devres removes it I'm fairly sure.
> >
> I've removed the dependency on vpcie3v3 in the following commit:
>  c930b10f17c0 ("PCI: dw-rockchip: Simplify regulator setup with
> devm_regulator_get_enable_optional()")
> Please review this commit and confirm if everything looks good.

I see. In that case, your code is indeed correct, thank you for
pointing this out.

Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli at collabora.com>

Kind regards,
Nicolas Frattaroli

> 
> > Kind regards,
> > Nicolas Frattaroli
> >
> Thanks
> -Anand
>