[PATCH v3 0/6] PCI: brcmstb: add EP regulators and panic handler

[PATCH v3 0/6] PCI: brcmstb: add EP regulators and panic handler

[Jim Quinlan]

v3 -- Driver now searches for EP DT subnode for any regulators to turn on.
      If present, these regulators have the property names
      "vpcie12v-supply" and "vpcie3v3-supply".  The existence of these
      regulators in the EP subnode are currently pending as a pullreq
      in pci-bus.yaml at
      https://github.com/devicetree-org/dt-schema/pull/54
      (MarkB, RobH).
   -- Check return of brcm_set_regulators() (Florian)
   -- Specify one regulator string per line for easier update (Florian)
   -- Author/Committer/Signoff email changed from that of V2 from
      'james.quinlan@broadcom.com' to 'jim2101024@gmail.com'.

v2 -- Use regulator bulk API rather than multiple calls (MarkB).

v1 -- Bindings are added for fixed regulators that may power the EP device.
   -- The brcmstb RC driver is modified to control these regulators
      during probe, suspend, and resume.
   -- 7216 type SOCs have additional error reporting HW and a
      panic handler is added to dump its info.
   -- A missing return value check is added.


Jim Quinlan (6):
  dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators
  PCI: brcmstb: Add control of EP voltage regulators
  PCI: brcmstb: Do not turn off regulators if EP can wake up
  PCI: brcmstb: Give 7216 SOCs their own config type
  PCI: brcmstb: Add panic/die handler to RC driver
  PCI: brcmstb: Check return value of clk_prepare_enable()

 .../bindings/pci/brcm,stb-pcie.yaml           |   6 +
 drivers/pci/controller/pcie-brcmstb.c         | 271 +++++++++++++++++-
 2 files changed, 272 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH v3 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators

[Jim Quinlan]

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>;
+                    };
             };
     };
-- 
2.17.1

Re: [PATCH v3 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators

[Rob Herring]

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.

Re: [PATCH v3 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators

[Rob Herring]

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

Re: [PATCH v3 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators

[Jim Quinlan]

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

Re: [PATCH v3 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]