| Message ID | 20210326191906.43567-2-jim2101024@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: brcmstb: add EP regulators and panic handler | expand |
On Fri, 26 Mar 2021 15:18:59 -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 | 6 ++++++ > 1 file changed, 6 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/pci/brcm,stb-pcie.example.dts:48.48-49 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/pci/brcm,stb-pcie.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1380: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1458942 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Fri, Mar 26, 2021 at 03:18:59PM -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 | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > index f90557f6deb8..ea3e6f55e365 100644 > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > @@ -156,5 +156,11 @@ examples: > <0x42000000 0x1 0x80000000 0x3 0x00000000 0x0 0x80000000>; > brcm,enable-ssc; > brcm,scb-sizes = <0x0000000080000000 0x0000000080000000>; > + > + pcie-ep@0,0 { > + reg = <0x0 0x0 0x0 0x0 0x0>; > + compatible = "pci14e4,1688"; > + vpcie12v-supply: <&vreg12>; For other cases, these properties are in the host bridge node. If these are standard PCI rails, then I think that's where they belong unless we define slot nodes. Rob
On Tue, Mar 30, 2021 at 10:08:16AM -0500, Rob Herring wrote: > On Fri, Mar 26, 2021 at 03:18:59PM -0400, Jim Quinlan wrote: > > + pcie-ep@0,0 { > > + reg = <0x0 0x0 0x0 0x0 0x0>; > > + compatible = "pci14e4,1688"; > > + vpcie12v-supply: <&vreg12>; > For other cases, these properties are in the host bridge node. If these > are standard PCI rails, then I think that's where they belong unless we > define slot nodes. For a soldered down part I'd expect we'd want both (if the host even cares) - for anything except a supply that I/O or something else shared is referenced off there's no great reason why it has to be physically the same supply going to every device on the bus so each device should be able to specify separately.
On Tue, Mar 30, 2021 at 11:30 AM Mark Brown <broonie@kernel.org> wrote: >10.22.8.121 > On Tue, Mar 30, 2021 at 10:08:16AM -0500, Rob Herring wrote: > > On Fri, Mar 26, 2021 at 03:18:59PM -0400, Jim Quinlan wrote: > > > > + pcie-ep@0,0 { > > > + reg = <0x0 0x0 0x0 0x0 0x0>; > > > + compatible = "pci14e4,1688"; > > > + vpcie12v-supply: <&vreg12>; > > > For other cases, these properties are in the host bridge node. If these > > are standard PCI rails, then I think that's where they belong unless we10.22.8.121 > > define slot nodes. > > For a soldered down part I'd expect we'd want both (if the host even > cares) - for anything except a supply that I/O or something else shared > is referenced off there's no great reason why it has to be physically > the same supply going to every device on the bus so each device should > be able to specify separately. Our developer and reference boards frequently have Mini and half-mini PCIe sockets (a few exceptions), whereas production boards are mostly soldered down. If I resubmit this pullreq so that it looks for "vpcie12v-supply" and "vpcie3v3-supply" in the host node, will that be acceptable for both of you? Thanks, Jim Quinlan Broadcom STB
On Tue, Mar 30, 2021 at 12:23:35PM -0400, Jim Quinlan wrote: > On Tue, Mar 30, 2021 at 11:30 AM Mark Brown <broonie@kernel.org> wrote: > > For a soldered down part I'd expect we'd want both (if the host even > > cares) - for anything except a supply that I/O or something else shared > > is referenced off there's no great reason why it has to be physically > > the same supply going to every device on the bus so each device should > > be able to specify separately. > Our developer and reference boards frequently have Mini and half-mini > PCIe sockets (a few exceptions), whereas production boards are mostly > soldered down. On reflection I think the above probably also applies to sockets - you'd just have to have a socket visible in the DT. > If I resubmit this pullreq so that it looks for "vpcie12v-supply" > and "vpcie3v3-supply" in the host node, will that be acceptable for > both of you? I think you will need both (assuming the controller actually physically gets the supplies) - like I say the sockets/devices may not all share the same 12V and 3.3V rails.
diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml index f90557f6deb8..ea3e6f55e365 100644 --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml @@ -156,5 +156,11 @@ examples: <0x42000000 0x1 0x80000000 0x3 0x00000000 0x0 0x80000000>; brcm,enable-ssc; brcm,scb-sizes = <0x0000000080000000 0x0000000080000000>; + + pcie-ep@0,0 { + reg = <0x0 0x0 0x0 0x0 0x0>; + compatible = "pci14e4,1688"; + vpcie12v-supply: <&vreg12>; + }; }; };
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 | 6 ++++++ 1 file changed, 6 insertions(+)