On Mon, May 19, 2025 at 2:25 PM Jim Quinlan <james.quinlan@xxxxxxxxxxxx> wrote: > > On Mon, May 19, 2025 at 1:28 PM Manivannan Sadhasivam > <manivannan.sadhasivam@xxxxxxxxxx> wrote: > > > > On Mon, May 19, 2025 at 09:05:57AM -0500, Bjorn Helgaas wrote: > > > On Mon, May 05, 2025 at 01:39:39PM -0400, Jim Quinlan wrote: > > > > Hello, > > > > > > > > I recently rebased to the latest Linux master > > > > > > > > ebd297a2affa Linus.Torvalds Merge tag 'net-6.15-rc5' of > > > > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net > > > > > > > > and noticed that PCI is broken for > > > > "drivers/pci/controller/pcie-brcmstb.c" I've bisected this to the > > > > following commit > > > > > > > > 2489eeb777af PCI/pwrctrl: Skip scanning for the device further if > > > > pwrctrl device is created > > > > > > > > which is part of the series [1]. The driver in pcie-brcmstb.c is > > > > expecting the add_bus() method to be invoked twice per boot-up, but > > > > the second call does not happen. Not only does this code in > > > > brcm_pcie_add_bus() turn on regulators, it also subsequently initiates > > > > PCIe linkup. > > > > > > > > If I revert the aforementioned commit, all is well. > > > > > > > > FWIW, I have included the relevant sections of the PCIe DT we use at [2]. > > > > > > Mani, Bartosz, where are we at with this? The 2489eeb777af commit log > > > doesn't mention a problem fixed by that commit; it sounds more like an > > > optimization -- just avoiding an unnecessary scan. > > > > > > 2489eeb777af appeared in v6.15-rc1, so there's still time to revert it > > > before v6.15 if that's the right way to fix this regression. > > > > > > > We need to find out what is happening in the pcie-brcmstb driver first. From > > Jim's report, it looks like the driver expects add_bus() callback to be invoked > > twice, which seems weird. > > Hi Mani, > > The pci_ops add_bus() method is invoked once for bus 0 and once for > bus 1. Note that our controller only has one port. If I do "lspci" I > get (our controller is on pci domain==1): > > 0001:00:00.0 PCI bridge: Broadcom Inc. and subsidiaries Device 7712 (rev 20) > 0001:01:00.0 Ethernet controller: Broadcom Inc. ... > > It is the second invocation of add_bus() that has the brcmstb driver > turning on the regulators and the subsequent link-up, and this call > never happens with the 2489eeb777aff9 commit. Hello Mani, Actually, I don't think it is sufficient to just revert that one commit. If I checkout 6.14-rc1 and just bring in 06bf05d7349c PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device() I get the following after getting the Linux prompt: pci 0001:00:00.0: deferred probe pending: pci: supplier 1000110000.pcie:pci@0,0 not ready pci 0001:00:00.0: deferred probe pending: pci: supplier 1000110000.pcie:pci@0,0 not ready Basically, brcmstb already picks up and controls the regulator nodes under the port driver node. You folks are adding a new generic system and we are stepping on one another's toes. The problem here is that I cannot seem to turn your system off using CONFIG_PWR* settings. We would certainly be open to adopting your system when it meets our requirements; such as turning off/on on regulators @suspend/resume, and the ability to not do that if the downstream device has device_may_wakeup(dev)==true. But until then we need a way to disable your system or allow brcmstb to "opt out". AFAICT this regression does not affect the RaspberryPi SoCs, so it is not a big deal for us if we take our time to fix this. But if so, it is incumbent on you folks to help me get past this regression. Is that reasonable? Regards, Jim Quinlan Broadcom STB/CM > > Regards, > Jim Quinlan > Broadcom STB/CM > > > > > > If the fix takes time, then we can revert 2489eeb777af in the meantime. > > > > - Mani > > > > -- > > மணிவண்ணன் சதாசிவம்
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
