[PATCH v5 4/6] PCI: brcmstb: Add control of subdevice voltage regulators

[Jim Quinlan]

On Fri, Oct 22, 2021 at 10:31 AM Mark Brown <broonie at kernel.org> 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