On Mon, Oct 25, 2021 at 03:04:34PM -0700, Florian Fainelli wrote:

On 10/25/21 7:58 AM, Mark Brown wrote:

quoted

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.

This doesn't really sound terribly unusual frankly, which means it
shouldn't be insurmountable.  It also doesn't sound like a great
approach to base ABIs around.

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".

If more than one board needs the same accomodation, for example if it's
common for a reset line to be GPIO controlled, then a common property
could be used by many different boards rather than requiring each
individual board to have board specific code.  This is on some level
what most DT properties boil down to.

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.

I'm fairly sure that for example phone vendors pay a bit of attention to
power consumption.

quoted

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?

The binding for a consumer should reflect what's going into that
consumer, this means that if you have 12V and 3.3V supplies then the
device should have two distinct supplies for that.  The device binding
should not change based on how those supplies are provided or any
relationship between the supplies outside the device, there should
definitely be no reason to define any new supplies for this purpose -
that would reflect a fundamental misunderstanding of the abstractions in
the API.

If (as it sounds) you've got systems with two supplies with GPIO enables
controlled by a single GPIO then you should just describe that directly
in device tree, this is quite common and there is support in there
already for identifying and reference counting the shared GPIO in that
case.

quoted

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.

Well, it's a bit of a shared responsibility where the thing being
provided is a standards conforming connector rather than there being an
embedded device with much more potential for variation.

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. 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.

I would expect it's possible to make a PCI binding for a standard
physical layer bus connection as part of the generic PCI bindings, for
example by using some standard invalid ID for the device ID that can't
exist in a physical system or just omitting the device ID.  That seems
like a fairly clear case of being able to describe actual hardware that
physically exists - you can see the physical socket on the board.

quoted

In any case whatever gets done needs to be documented in the bindings
documents.

Is not that what patch #1 does?

It just updated the example, it didn't document any new properties.  The
standard supplies are documented in the patch to the core standard that
was referenced so they're fine but not the extra Broadcom specific ones
that I've raised concerns with.