Re: [PATCH v4 2/6] dt-bindings: PCI: Add bindings for Brcmstb endpoint device voltage regulators

From: Rob Herring <robh@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Jim Quinlan <jim2101024@gmail.com>,
	linux-pci@vger.kernel.org,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	bcm-kernel-feedback-list@broadcom.com,
	james.quinlan@broadcom.com,
	Florian Fainelli <f.fainelli@gmail.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-rpi-kernel@lists.infradead.org>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/6] dt-bindings: PCI: Add bindings for Brcmstb endpoint device voltage regulators
Date: Thu, 8 Apr 2021 12:41:42 -0500	[thread overview]
Message-ID: <20210408174142.GB1556444@robh.at.kernel.org> (raw)
In-Reply-To: <20210406164708.GM6443@sirena.org.uk>

On Tue, Apr 06, 2021 at 05:47:08PM +0100, Mark Brown wrote:
> On Thu, Apr 01, 2021 at 05:21:42PM -0400, Jim Quinlan wrote:
> > Similar to the regulator bindings found in "rockchip-pcie-host.txt", this
> > allows optional regulators to be attached and controlled by the PCIe RC
> > driver.  That being said, this driver searches in the DT subnode (the EP
> > node, eg pci@0,0) for the regulator property.
> 
> > The use of a regulator property in the pcie EP subnode such as
> > "vpcie12v-supply" depends on a pending pullreq to the pci-bus.yaml
> > file at
> > 
> > https://github.com/devicetree-org/dt-schema/pull/54
> > 
> > Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > index f90557f6deb8..f2caa5b3b281 100644
> > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > @@ -64,6 +64,9 @@ properties:
> >  
> >    aspm-no-l0s: true
> >  
> > +  vpcie12v-supply: true
> > +  vpcie3v3-supply: true
> > +
> 
> No great problem with having these in the controller node (assming it
> accurately describes the hardware) but I do think we ought to also be
> able to describe these per slot.

My hesistancy here is child nodes are already defined to be devices, not 
slots. That's generally the same thing though. However, 'reg' is a bit 
problematic as it includes the bus number which is dynamically 
assigned. (This is a mismatch between true OpenFirmware and FDT as OF 
was designed to populate the DT with what was discovered and that 
includes some runtime config such as bus number assignments.) Maybe we 
just say for FDT, the bus number is always 0 and ignored. In any case, 
there needs to be some thought on this as well as the more complicated 
case of devices needing device specific setup to be enumerated.

Rob