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

[Rob Herring <robh@kernel.org>]

On Mon, Oct 25, 2021 at 03:04:34PM -0700, Florian Fainelli wrote:
> On 10/25/21 7:58 AM, Mark Brown wrote:
> > On Mon, Oct 25, 2021 at 09:50:09AM -0400, Jim Quinlan wrote:
> >> On Fri, Oct 22, 2021 at 3:47 PM Mark Brown <broonie@kernel.org> wrote:
> >>> On Fri, Oct 22, 2021 at 03:15:59PM -0400, Jim Quinlan wrote:
> > 
> >>> That sounds like it just shouldn't be a regulator at all, perhaps the
> >>> board happens to need a regulator there but perhaps it needs a clock,
> >>> GPIO or some specific sequence of actions.  It sounds like you need some
> >>> sort of quirking mechanism to cope with individual boards with board
> >>> specific bindings.
> > 
> >> The boards involved may have no PCIe sockets, or run the gamut of the different
> >> PCIe sockets.  They all offer gpio(s) to turn off/on their power supply(s) to
> >> make their PCIe device endpoint functional.  It is not viable to add
> >> new Linux quirk or DT
> >> code for each board.  First is the volume and variety of the boards
> >> that use our SOCs.. Second, is
> >> our lack of information/control:  often, the board is designed by one
> >> company (not us), and
> >> given to another company as the middleman, and then they want the
> >> features outlined
> >> in my aforementioned commit message.
> > 
> > Other vendors have plenty of variation in board design yet we still have
> > device trees that describe the hardware, I can't see why these systems
> > should be so different.  It is entirely normal for system integrators to
> > collaborate on this and even upstream their own code, this happens all
> > the time, there is no need for everything to be implemented directly the
> > SoC vendor.
> 
> This is all well and good and there is no disagreement here that it
> should just be that way but it does not reflect what Jim and I are
> confronted with on a daily basis. We work in a tightly controlled
> environment using a waterfall approach, whatever we come up with as a
> SoC vendor gets used usually without further modification by the OEMs,
> when OEMs do change things we have no visibility into anyway.
> 
> We have a boot loader that goes into great lengths to tailor the FDT
> blob passed to the kernel to account for board-specific variations, PCIe
> voltage regulators being just one of those variations. This is usually
> how we quirk and deal with any board specific details, so I fail to
> understand what you mean by "quirks that match a common pattern".
> 
> Also, I don't believe other vendors are quite as concerned with
> conserving power as we are, it could be that they are just better at it
> through different ways, or we have a particular sensitivity to the subject.
> 
> > 
> > If there are generic quirks that match a common pattern seen in a lot of
> > board then properties can be defined for those, this is in fact the
> > common case.  This is no reason to just shove in some random thing that
> > doesn't describe the hardware, that's a great way to end up stuck with
> > an ABI that is fragile and difficult to understand or improve.
> 
> I would argue that at least 2 out of the 4 supplies proposed do describe
> hardware signals. The latter two are more abstract to say the least,
> however I believe it is done that way because they are composite
> supplies controlling both the 12V and 3.3V supplies coming into a PCIe
> device (Jim is that right?). So how do we call the latter supply then?
> vpcie12v3v...-supply?
> 
> > Potentially some of these things should be being handled in the bindings
> > and drivers drivers for the relevant PCI devices rather than in the PCI
> > controller.
> 
> The description and device tree binding can be and should be in a PCI
> device binding rather than pci-bus.yaml.
> 
> The handling however goes back to the chicken and egg situation that we
> talked about multiple times before: no supply -> no link UP -> no
> enumeration -> no PCI device, therefore no driver can have a chance to
> control the regulator. These are not hotplug capable systems by the way,
> but even if they were, we would still run into the same problem. Given
> that most reference boards do have mechanical connectors that people can
> plug random devices into, we cannot provide a compatible string
> containing the PCI vendor/device ID ahead of time because we don't know
> what will be plugged in. 

I thought you didn't have connectors or was it just they are 
non-standard? If the latter case, what are the supply rails for the 
connector?

I'd be okay if there's no compatible as long as there's not a continual 
stream of DT properties trying to describe power sequencing 
requirements.

> In the case of a MCM, we would, but then we
> only solved about 15% of the boards we need to support, so we have not
> really progressed much.

MCM is multi-chip module?

Rob