On Sat, Oct 3, 2026 at 5:31 AM Dave Klotz <[email protected]> wrote: > > clk_enable_bulk(), reset_deassert_bulk() and the outbound iATU setup > can all fail, and the probe ignores all three. A controller whose > clocks or resets did not come up is then accessed anyway, and a failed > memory window is reported as a successful probe. > > Fail the probe with the error instead, as Linux's driver does for its > clocks and resets, and undo the clock enable when the resets fail. > > Signed-off-by: Dave Klotz <[email protected]> > --- > drivers/pci/pcie_dw_spacemit.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pcie_dw_spacemit.c b/drivers/pci/pcie_dw_spacemit.c > index b85de73ad3..5e0871986b 100644 > --- a/drivers/pci/pcie_dw_spacemit.c > +++ b/drivers/pci/pcie_dw_spacemit.c > @@ -230,8 +230,18 @@ static int pcie_dw_spacemit_probe(struct udevice *dev) > spacemit_pcie_toggle_soft_reset(pci); > > /* enable pcie clk and deassert resets */ > - clk_enable_bulk(&pci->clks); > - reset_deassert_bulk(&pci->rsts); > + ret = clk_enable_bulk(&pci->clks); > + if (ret) { > + dev_err(dev, "failed to enable clocks: %d\n", ret); > + return ret; > + } > + > + ret = reset_deassert_bulk(&pci->rsts); > + if (ret) { > + dev_err(dev, "failed to deassert resets: %d\n", ret); > + clk_disable_bulk(&pci->clks); > + return ret; > + }
I'm fine with disabling or resetting the clock when it fails. However, the probe function has several other error return paths, so I'd suggest adding the same disable/reset handling to those cases as well. > > reg = spacemit_pcie_readl(pci, PCIECTRL_K1X_CONF_DEVICE_CMD); > reg &= ~LTSSM_EN; > @@ -317,6 +327,8 @@ static int pcie_dw_spacemit_probe(struct udevice *dev) > pci->dw.mem.phys_start, > pci->dw.mem.bus_start, > pci->dw.mem.size); > + if (ret) > + return -EIO; > > return 0; > } > -- > 2.55.0 > >
