[PATCH v3 00/12] PCI: brcnstb: Enable STB 7712 SOC

[PATCH v3 00/12] PCI: brcnstb: Enable STB 7712 SOC

[Jim Quinlan]

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

V3 Changes:
  o Commit "Enable 7712 SOCs"
    -- Move "model" check from outside to inside func (Stan)
  o Commit "Check return value of all reset_control_xxx calls"
    -- Propagate errors up the chain instead of ignoring them (Stan)
  o Commit "Refactor for chips with many regular inbound BARs"
    -- Nine suggestions given, nine implemented (Stan)
  o Commit "Make HARD_DEBUG, INTR2_CPU_BASE offsets SoC-specific"
    -- Drop tab, add parens around macro params in expression (Stan)
  o Commit "Use swinit reset if available"
    -- Treat swinit the same as other reset controllers (Stan)
       Stan suggested to use dev_err_probe() for getting resources
       but I will defer that to future series (if that's okay).
  o Commit "Get resource before we start asserting resets"
    -- Squash this with previous commit (Stan)
  o Commit "Use "clk_out" error path label"
    -- Move clk_prepare_enable() after getting resouurces (Stan)
    -- Change subject to "Use more common error handling code in
       brcm_pcie_probe()" (Markus)
    -- Use imperative commit description (Markus)
    -- "Fixes:" tag added for missing error return. (Markus)
  o Commit "dt-bindings: PCI ..."
    -- Split off maintainer change in separate commit.
    -- Tried to accomodate Krzysztof's requests, I'm not sure I
       have succeeded.  Krzysztof, please see [1] below.
  
  [1] Wrt the YAML of brcmstb PCIe resets, here is what I am trying
      to describe:

      CHIP       NUM_RESETS    NAMES
      ====       ==========    =====
      4908       1             perst
      7216       1             rescal
      7712       3             rescal, bridge, swinit
      Others     0             -


V2 Changes (note: four new commits):
  o Commit "dt-bindings: PCI ..."
    -- s/Adds/Add/, fix spelling error (Bjorn)
    -- Order compatible strings alphabetically (Krzysztof)
    -- Give definitions first then rules (Krzysztof)
    -- Add reason for change in maintainer (Krzysztof)
  o Commit "Use swinit reset if available"
    -- no need for "else" clause (Philipp)
    -- fix improper use of dev_err_probe() (Philipp) 
  o Commit "Use "clk_out" error path label"
    -- Improve commit message (Bjorn)
  o Commit "PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE offsets SoC-specific"
    -- Improve commit subject line (Bjorn)
  o Commit (NEW) -- Change field name from 'type' to 'model'
    -- Added as requested (Stanimir)
  o Commit (NEW) -- Check return value of all reset_control_xxx calls
    -- Added as requested (Stanimir)
  o Commit (NEW) "Get resource before we start asserting reset controllers"
    -- Added as requested (Stanimir)
  o Commit (NEW) -- "Remove two unused constants from driver"


V1:
  This submission is for the Broadcom STB 7712, sibling SOC of the RPi5 chip.
  Stanimir has already submitted a patch "Add PCIe support for bcm2712" for
  the RPi version of the SOC.  It is hoped that Stanimir will allow us to
  submit this series first and subsequently rebase his patch(es).

  The largest commit, "Refactor for chips with many regular inbound BARs"
  affects both the STB and RPi SOCs.  It allows for multiple inbound ranges
  where previously only one was effectively used.  This feature will also
  be present in future STB chips, as well as Broadcom's Cable Modem group.


Jim Quinlan (12):
  dt-bindings: PCI: Change brcmstb YAML maintainer
  dt-bindings: PCI: Cleanup of brcmstb YAML and add 7712 SoC
  PCI: brcmstb: Use common error handling code in brcm_pcie_probe()
  PCI: brcmstb: Use bridge reset if available
  PCI: brcmstb: Use swinit reset if available
  PCI: brcmstb: PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE offsets
    SoC-specific
  PCI: brcmstb: Remove two unused constants from driver
  PCI: brcmstb: Don't conflate the reset rescal with phy ctrl
  PCI: brcmstb: Refactor for chips with many regular inbound BARs
  PCI: brcmstb: Check return value of all reset_control_xxx calls
  PCI: brcmstb: Change field name from 'type' to 'model'
  PCI: brcmstb: Enable 7712 SOCs

 .../bindings/pci/brcm,stb-pcie.yaml           |  48 +-
 drivers/pci/controller/pcie-brcmstb.c         | 484 +++++++++++++-----
 2 files changed, 397 insertions(+), 135 deletions(-)


base-commit: 55027e689933ba2e64f3d245fb1ff185b3e7fc81
-- 
2.17.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4210 bytes --]

[PATCH v3 01/12] dt-bindings: PCI: Change brcmstb YAML maintainer

[Jim Quinlan]

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

Nicolas has not been active for a while.  It also makes
sense for a Broadcom employee to be the maintainer as
many of the details are privy to Broadcom.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 11f8ea33240c..558a18fd2e77 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: Brcmstb PCIe Host Controller
 
 maintainers:
-  - Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+  - Jim Quinlan <james.quinlan@broadcom.com>
 
 properties:
   compatible:
-- 
2.17.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4210 bytes --]

[PATCH v3 02/12] dt-bindings: PCI: Cleanup of brcmstb YAML and add 7712 SoC

[Jim Quinlan]

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

o Change order of the compatible strings to be alphabetical
o Describe resets/reset-names before using them in rules
o Add in new resets for 7712 (swinit, bridge)
o Add in changes for 7712

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 .../bindings/pci/brcm,stb-pcie.yaml           | 46 +++++++++++++++++--
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 558a18fd2e77..dd7c76ec9ceb 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -16,11 +16,12 @@ properties:
           - brcm,bcm2711-pcie # The Raspberry Pi 4
           - brcm,bcm4908-pcie
           - brcm,bcm7211-pcie # Broadcom STB version of RPi4
-          - brcm,bcm7278-pcie # Broadcom 7278 Arm
           - brcm,bcm7216-pcie # Broadcom 7216 Arm
-          - brcm,bcm7445-pcie # Broadcom 7445 Arm
+          - brcm,bcm7278-pcie # Broadcom 7278 Arm
           - brcm,bcm7425-pcie # Broadcom 7425 MIPs
           - brcm,bcm7435-pcie # Broadcom 7435 MIPs
+          - brcm,bcm7445-pcie # Broadcom 7445 Arm
+          - brcm,bcm7712-pcie # STB sibling SOC of Raspberry Pi 5
 
   reg:
     maxItems: 1
@@ -95,6 +96,20 @@ properties:
       minItems: 1
       maxItems: 3
 
+  resets:
+    items:
+      - description: reset for phy reset calibration
+      - description: reset for PCIe/CPU bus bridge
+      - description: reset for soft PCIe core reset
+      - description: reset for external PCIe PERST# signal
+
+  reset-names:
+    items:
+      - const: rescal
+      - const: bridge
+      - const: swinit
+      - const: perst
+
 required:
   - compatible
   - reg
@@ -118,8 +133,8 @@ allOf:
     then:
       properties:
         resets:
-          items:
-            - description: reset controller handling the PERST# signal
+          minItems: 1
+          maxItems: 1
 
         reset-names:
           items:
@@ -136,12 +151,33 @@ allOf:
     then:
       properties:
         resets:
+          minItems: 1
+          maxItems: 1
+
+        reset-names:
           items:
-            - description: phandle pointing to the RESCAL reset controller
+            - const: rescal
+
+      required:
+        - resets
+        - reset-names
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm7712-pcie
+    then:
+      properties:
+        resets:
+          minItems: 3
+          maxItems: 3
 
         reset-names:
           items:
             - const: rescal
+            - const: bridge
+            - const: swinit
 
       required:
         - resets
-- 
2.17.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4210 bytes --]

Re: [PATCH v3 02/12] dt-bindings: PCI: Cleanup of brcmstb YAML and add 7712 SoC

[Krzysztof Kozlowski]

On 11/07/2024 00:16, Jim Quinlan wrote:
> o Change order of the compatible strings to be alphabetical

That's a cleanup. You can squash it with previous patch.

> o Describe resets/reset-names before using them in rules

That's a new commit.

> o Add in new resets for 7712 (swinit, bridge)
> o Add in changes for 7712
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>  .../bindings/pci/brcm,stb-pcie.yaml           | 46 +++++++++++++++++--
>  1 file changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> index 558a18fd2e77..dd7c76ec9ceb 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> @@ -16,11 +16,12 @@ properties:
>            - brcm,bcm2711-pcie # The Raspberry Pi 4
>            - brcm,bcm4908-pcie
>            - brcm,bcm7211-pcie # Broadcom STB version of RPi4
> -          - brcm,bcm7278-pcie # Broadcom 7278 Arm
>            - brcm,bcm7216-pcie # Broadcom 7216 Arm
> -          - brcm,bcm7445-pcie # Broadcom 7445 Arm
> +          - brcm,bcm7278-pcie # Broadcom 7278 Arm
>            - brcm,bcm7425-pcie # Broadcom 7425 MIPs
>            - brcm,bcm7435-pcie # Broadcom 7435 MIPs
> +          - brcm,bcm7445-pcie # Broadcom 7445 Arm
> +          - brcm,bcm7712-pcie # STB sibling SOC of Raspberry Pi 5
>  
>    reg:
>      maxItems: 1
> @@ -95,6 +96,20 @@ properties:
>        minItems: 1
>        maxItems: 3
>  
> +  resets:

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

minItems

> +    items:
> +      - description: reset for phy reset calibration
> +      - description: reset for PCIe/CPU bus bridge
> +      - description: reset for soft PCIe core reset
> +      - description: reset for external PCIe PERST# signal
> +
> +  reset-names:

Same here

> +    items:
> +      - const: rescal
> +      - const: bridge
> +      - const: swinit
> +      - const: perst



Best regards,
Krzysztof

Re: [PATCH v3 01/12] dt-bindings: PCI: Change brcmstb YAML maintainer

[Rob Herring (Arm)]

On Wed, 10 Jul 2024 18:16:15 -0400, Jim Quinlan wrote:
> Nicolas has not been active for a while.  It also makes
> sense for a Broadcom employee to be the maintainer as
> many of the details are privy to Broadcom.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v3 00/12] PCI: brcnstb: Enable STB 7712 SOC

[Florian Fainelli]

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



On 7/10/2024 3:16 PM, Jim Quinlan wrote:
> V3 Changes:
>    o Commit "Enable 7712 SOCs"
>      -- Move "model" check from outside to inside func (Stan)
>    o Commit "Check return value of all reset_control_xxx calls"
>      -- Propagate errors up the chain instead of ignoring them (Stan)
>    o Commit "Refactor for chips with many regular inbound BARs"
>      -- Nine suggestions given, nine implemented (Stan)
>    o Commit "Make HARD_DEBUG, INTR2_CPU_BASE offsets SoC-specific"
>      -- Drop tab, add parens around macro params in expression (Stan)
>    o Commit "Use swinit reset if available"
>      -- Treat swinit the same as other reset controllers (Stan)
>         Stan suggested to use dev_err_probe() for getting resources
>         but I will defer that to future series (if that's okay).
>    o Commit "Get resource before we start asserting resets"
>      -- Squash this with previous commit (Stan)
>    o Commit "Use "clk_out" error path label"
>      -- Move clk_prepare_enable() after getting resouurces (Stan)
>      -- Change subject to "Use more common error handling code in
>         brcm_pcie_probe()" (Markus)
>      -- Use imperative commit description (Markus)
>      -- "Fixes:" tag added for missing error return. (Markus)
>    o Commit "dt-bindings: PCI ..."
>      -- Split off maintainer change in separate commit.
>      -- Tried to accomodate Krzysztof's requests, I'm not sure I
>         have succeeded.  Krzysztof, please see [1] below.
>    
>    [1] Wrt the YAML of brcmstb PCIe resets, here is what I am trying
>        to describe:
> 
>        CHIP       NUM_RESETS    NAMES
>        ====       ==========    =====
>        4908       1             perst
>        7216       1             rescal
>        7712       3             rescal, bridge, swinit
>        Others     0             -
> 
> 
> V2 Changes (note: four new commits):
>    o Commit "dt-bindings: PCI ..."
>      -- s/Adds/Add/, fix spelling error (Bjorn)
>      -- Order compatible strings alphabetically (Krzysztof)
>      -- Give definitions first then rules (Krzysztof)
>      -- Add reason for change in maintainer (Krzysztof)
>    o Commit "Use swinit reset if available"
>      -- no need for "else" clause (Philipp)
>      -- fix improper use of dev_err_probe() (Philipp)
>    o Commit "Use "clk_out" error path label"
>      -- Improve commit message (Bjorn)
>    o Commit "PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE offsets SoC-specific"
>      -- Improve commit subject line (Bjorn)
>    o Commit (NEW) -- Change field name from 'type' to 'model'
>      -- Added as requested (Stanimir)
>    o Commit (NEW) -- Check return value of all reset_control_xxx calls
>      -- Added as requested (Stanimir)
>    o Commit (NEW) "Get resource before we start asserting reset controllers"
>      -- Added as requested (Stanimir)
>    o Commit (NEW) -- "Remove two unused constants from driver"
> 
> 
> V1:
>    This submission is for the Broadcom STB 7712, sibling SOC of the RPi5 chip.
>    Stanimir has already submitted a patch "Add PCIe support for bcm2712" for
>    the RPi version of the SOC.  It is hoped that Stanimir will allow us to
>    submit this series first and subsequently rebase his patch(es).
> 
>    The largest commit, "Refactor for chips with many regular inbound BARs"
>    affects both the STB and RPi SOCs.  It allows for multiple inbound ranges
>    where previously only one was effectively used.  This feature will also
>    be present in future STB chips, as well as Broadcom's Cable Modem group.

Nit: the subject has a typo s/brcnstb/brcmstb/g, probably does not 
matter since the subject would not be included in the pull request anyway.
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH v3 01/12] dt-bindings: PCI: Change brcmstb YAML maintainer

[Florian Fainelli]

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



On 7/10/2024 3:16 PM, Jim Quinlan wrote:
> Nicolas has not been active for a while.  It also makes
> sense for a Broadcom employee to be the maintainer as
> many of the details are privy to Broadcom.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH v3 00/12] PCI: brcnstb: Enable STB 7712 SOC

[Florian Fainelli]

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

On 7/10/24 15:16, Jim Quinlan wrote:
> V3 Changes:
>    o Commit "Enable 7712 SOCs"
>      -- Move "model" check from outside to inside func (Stan)
>    o Commit "Check return value of all reset_control_xxx calls"
>      -- Propagate errors up the chain instead of ignoring them (Stan)
>    o Commit "Refactor for chips with many regular inbound BARs"
>      -- Nine suggestions given, nine implemented (Stan)
>    o Commit "Make HARD_DEBUG, INTR2_CPU_BASE offsets SoC-specific"
>      -- Drop tab, add parens around macro params in expression (Stan)
>    o Commit "Use swinit reset if available"
>      -- Treat swinit the same as other reset controllers (Stan)
>         Stan suggested to use dev_err_probe() for getting resources
>         but I will defer that to future series (if that's okay).
>    o Commit "Get resource before we start asserting resets"
>      -- Squash this with previous commit (Stan)
>    o Commit "Use "clk_out" error path label"
>      -- Move clk_prepare_enable() after getting resouurces (Stan)
>      -- Change subject to "Use more common error handling code in
>         brcm_pcie_probe()" (Markus)
>      -- Use imperative commit description (Markus)
>      -- "Fixes:" tag added for missing error return. (Markus)
>    o Commit "dt-bindings: PCI ..."
>      -- Split off maintainer change in separate commit.
>      -- Tried to accomodate Krzysztof's requests, I'm not sure I
>         have succeeded.  Krzysztof, please see [1] below.

Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>

Thanks Jim!
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]