On Fri, Jan 26, 2024 at 03:43:39PM -0600, Bjorn Helgaas wrote: > In subject, capitalize "Add ..." to follow historical convention. > Also the other driver/pci/ patches. > > On Fri, Jan 26, 2024 at 03:36:55PM +0100, Thomas Richard wrote: > > From: Théo Lebrun <theo.lebrun@xxxxxxxxxxx> > > > > That function mixes probe structure init and hardware config. > > The whole hardware config part must be done at resume after a suspend to > > ram. > > We therefore pass it a boolean flag determining if we are at probe or at > > resume. > ... > This is pretty similar but slightly different from the DWC pattern: > > imx6_pcie_probe > ... do structure init ... > if (RC) > dw_pcie_host_init > pp->ops->init > imx6_pcie_host_init > > imx6_pcie_resume_noirq > imx6_pcie_host_init > > j721e_pcie_probe > j721e_pcie_ctrl_init > if (RC) > cdns_pcie_host_setup(true) > > j721e_pcie_resume_noirq > j721e_pcie_ctrl_init > if (RC) > cdns_pcie_host_setup(false) > > It'd be super nice to have them the same. Passing in a "probe" flag > works but seems a little harder to read in cdns_pcie_host_setup() and > you have to keep track of what it means in the callers. Maybe a better way to say this is that this patch uses the "probe" flag to select the behavior of cdns_pcie_host_setup(), and I think it would be nicer to split those two behaviors into separate functions. Bjorn
