On Fri, Jun 27, 2025 at 05:45:02PM -0500, Bjorn Helgaas wrote: > On Mon, Jun 16, 2025 at 11:02:09AM +0530, Manivannan Sadhasivam wrote: > > pci_pwrctrl_create_device() is a PWRCTRL framework API. So it should be > > built only when CONFIG_PWRCTRL is enabled. Currently, it is built > > independently of CONFIG_PWRCTRL. This creates enumeration failure on > > platforms like brcmstb using out-of-tree devicetree that describes the > > power supplies for endpoints in the PCIe child node, but doesn't use > > PWRCTRL framework to manage the supplies. The controller driver itself > > manages the supplies. > > > > But in any case, the API should be built only when CONFIG_PWRCTRL is > > enabled. So move its definition to drivers/pci/pwrctrl/core.c and provide > > a stub in drivers/pci/pci.h when CONFIG_PWRCTRL is not enabled. This also > > fixes the enumeration issues on the affected platforms. > > Finally circling back to this since I think brcmstb is broken since > v6.15 and we should fix it for v6.16 final. > Yes! Sorry for the delay. The fact that I switched the job and had to attend OSS NA prevented me from reworking this patch. > IIUC, v3 is the current patch and needs at least a fix for the build > issue [1], and I guess the options are: > > 1) Make CONFIG_PCI_PWRCTRL bool. On my x86-64 system > pci-pwrctrl-core.o is 8880 bytes, which seems like kind of a lot > when only a few systems need it. > > 2) Leave pci_pwrctrl_create_device() in probe.c. It gets optimized > away if CONFIG_OF=n because of_pci_find_child_device() returns > NULL, but still a little ugly for readers. > > 3) Put pci_pwrctrl_create_device() in a separate > drivers/pci/pwrctrl/ file that is always compiled even if PWRCTRL > itself is a module. Ugly because then we sort of have two "core" > files (core.c and whatever new file is always compiled). > I guess, we could go with option 3 if you prefer. We could rename the existing pwrctrl/core.c to pwrctrl/pwrctrl.c and move the definition of pci_pwrctrl_create_device() to new pwrctrl/core.c. The new file will depend on HAVE_PWRCTRL, which is bool. > And I guess all of these options still depend on CONFIG_PCI_PWRCTRL > not being enabled in a kernel that has brcmstb enabled? If so, that > seems ugly to me. We should be able to enable both PWRCTRL and > brcmstb at the same time, e.g., for a single kernel image that works > both on a brcmstb system and a system that needs pwrctrl. > Right, that would be the end goal. As I explained in the reply to the bug report [1], this patch will serve as an interim workaround. Once my pwrctrl rework (which I didn't submit yet) is merged, I will move this driver to use the pwrctrl framework. The fact that I missed this driver in the first place during the previous rework of the pwrctrl framework is due to devicetree being kept out-of-tree for this platform. - Mani [1] https://lore.kernel.org/all/vazxuov2hdk5sezrk7a5qfuclv2s3wo5sxhfwuo3o4uedsdlqv@po55ny24ctne/ -- மணிவண்ணன் சதாசிவம்
