On Fri, Oct 22, 2021 at 10:31 AM Mark Brown <broonie@xxxxxxxxxx> wrote: > > On Fri, Oct 22, 2021 at 10:06:57AM -0400, Jim Quinlan wrote: > > > +static const char * const supplies[] = { > > + "vpcie3v3-supply", > > + "vpcie3v3aux-supply", > > + "brcm-ep-a-supply", > > + "brcm-ep-b-supply", > > +}; > > Why are you including "-supply" in the names here? That will lead to > a double -supply when we look in the DT which probably isn't what you're > looking for. I'm not sure how this got past testing; will fix. > > Also are you *sure* that the device has supplies with names like > "brcm-ep-a"? That seems rather unidiomatic for electrical engineering, > the names here are supposed to correspond to the names used in the > datasheet for the part. I try to explain this in the commit message of"PCI: allow for callback to prepare nascent subdev". Wrt to the names, "These regulators typically govern the actual power supply to the endpoint chip. Sometimes they may be a the official PCIe socket power -- such as 3.3v or aux-3.3v. Sometimes they are truly the regulator(s) that supply power to the EP chip." Each different SOC./board we deal with may present different ways of making the EP device power on. We are using an abstraction name "brcm-ep-a" to represent some required regulator to make the EP work for a specific board. The RC driver cannot hard code a descriptive name as it must work for all boards designed by us, others, and third parties. The EP driver also doesn't know or care about the regulator name, and this driver is often closed source and often immutable. The EP device itself may come from Brcm, a third party, or sometimes a competitor. Basically, we find using a generic name such as "brcm-ep-a-supply" quite handy and many of our customers embrace this feature. I know that Rob was initially against such a generic name, but I vaguely remember him seeing some merit to this, perhaps a tiny bit :-) Or my memory is shot, which could very well be the case. > > > + /* This is for Broadcom STB/CM chips only */ > > + if (pcie->type == BCM2711) > > + return 0; > > It is a relief that other chips have managed to work out how to avoid > requiring power. I'm not sure that the other Broadcom groups have our customers, our customers' requirements, and the amount and variation of boards that run our PCIe driver on the SOC. Jim