[PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

[PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

[Jim Quinlan]

V6 Changes
  o Commit "Refactor for chips with many regular inbound windows"
    -- Use u8 for anything storing/counting # inbound windows (Stan)
    -- s/set_bar/add_inbound_win/g (Manivannan)
    -- Drop use of "inline" (Manivannan)
    -- Change cpu_beg to cpu_start, same with pcie_beg. (Manivannan)
    -- Used writel_relaxed() (Manivannan)
  o Use swinit reset if available
    -- Proper use of dev_err_probe() (Stan)
  o Commit "Use common error handling code in brcm_pcie_probe()"
    -- Rewrite commit msg in paragraph form (Manivannan)
    -- Refactor error path at end of probe func (Manivannan)
    -- Proper use of dev_err_probe() (Stan)
  o New commit "dt-bindings: PCI: Change brcmstb maintainer and cleanup"
    -- Break out maintainer change and small cleanup into a
       separate commit (Krzysztof)

V5 Changes:
  o All commits: Use imperative voice in commit subjects/messages
       (Manivannan)
  o Commit "PCI: brcmstb: Enable 7712 SOCs"
    -- Augment commit message to include PCIe details and revision.
       (Manivannan)
  o Commit "PCI: brcmstb: Change field name from 'type' to 'model'"
    -- Instead of "model" use "soc_base" (Manivannan)
  o Commit "PCI: brcmstb: Refactor for chips with many regular inbound BARs"
    -- Get rid of the confusing "BAR" variable names and types and use
       something like "inbound_win". (Manivannan)
  o Commit "PCI: brcmstb: PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE..."
    -- Mention in the commit message that this change is in preparation
       for the 7712 SoC. (Manivannan)
  o Commit: "PCI: brcmstb: Use swinit reset if available"
    -- Change reset name "swinit" to "swinit_reset" (Manivannan)
    -- Add 1us delay for reset (Manivannan)
    -- Use dev_err_probe() (Multiple reviewers)
  o Commit "PCI: brcmstb: Use bridge reset if available"
    -- Change reset name "bridge" to "bridge_reset" (Manivannan)
    -- The Reset API can take NULL so need need to test variable
       before calling (Manivannan)
    -- Added a call to bridge_sw_init_set() method in probe()
       as some registers cannot be accessed w/o this. (JQ)
  o Commit "PCI: brcmstb: Use common error handling code in ..."
    -- Use more descriptive goto label (Manivannan)
    -- Refactor error paths to be less encumbered (Manivannan)
    -- Use dev_err_probe() (Multiple reviewers)
  o Commits "dt-bindings: PCI: brcmstb: ..."
    -- Specify the "resets" and "reset-names" in the same manner
       as does qcom,ufs.yaml specifies "clocks" and
       "clock-names" (Krzysztof)
    -- Drop reset desccriptions as they were pretty content-free
       anyhow. (Krzysztof)

V4 Changes:
  o Commit "Check return value of all reset_control_xxx calls"
    -- Blank line before "return" (Stan)
  o Commit "Use common error handling code in brcmstb_probe()"
    -- Drop the "Fixes" tag (Stan)
  o Commit "dt-bindings: PCI ..."
    -- Separate the main commit into two: cleanup and adding the
       7712 SoC (Krzysztof)
    -- Fold maintainer change commit into cleanup change (Krzysztof)
    -- Use minItems/maxItems where appropriate (Krzysztof)
    -- Consistent order of resets/reset-names in decl and usage
       (Krzysztof)

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 (13):
  dt-bindings: PCI: Change brcmstb maintainer and cleanup
  dt-bindings: PCI: Use maxItems for reset controllers
  dt-bindings: PCI: brcmstb: Add 7712 SoC description
  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 windows
  PCI: brcmstb: Check return value of all reset_control_xxx calls
  PCI: brcmstb: Change field name from 'type' to 'soc_base'
  PCI: brcmstb: Enable 7712 SOCs

 .../bindings/pci/brcm,stb-pcie.yaml           |  40 +-
 drivers/pci/controller/pcie-brcmstb.c         | 513 +++++++++++++-----
 2 files changed, 412 insertions(+), 141 deletions(-)


base-commit: e724918b3786252b985b0c2764c16a57d1937707
-- 
2.17.1

[PATCH v6 01/13] dt-bindings: PCI: Change brcmstb maintainer and cleanup

[Jim Quinlan]

Change maintainer: 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.

Also, alphabetize the compatible strings.

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

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 11f8ea33240c..a95760357335 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:
@@ -16,11 +16,11 @@ 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
 
   reg:
     maxItems: 1
-- 
2.17.1

[PATCH v6 02/13] dt-bindings: PCI: Use maxItems for reset controllers

[Jim Quinlan]

Provide the maxItem property for the reset controllers and drop their
superfluous descriptions.

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

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index a95760357335..7d2552192153 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -95,6 +95,12 @@ properties:
       minItems: 1
       maxItems: 3
 
+  resets:
+    maxItems: 1
+
+  reset-names:
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -118,8 +124,7 @@ allOf:
     then:
       properties:
         resets:
-          items:
-            - description: reset controller handling the PERST# signal
+          maxItems: 1
 
         reset-names:
           items:
@@ -136,8 +141,7 @@ allOf:
     then:
       properties:
         resets:
-          items:
-            - description: phandle pointing to the RESCAL reset controller
+          maxItems: 1
 
         reset-names:
           items:
-- 
2.17.1

[PATCH v6 03/13] dt-bindings: PCI: brcmstb: Add 7712 SoC description

[Jim Quinlan]

Add description for the 7712 SoC, a Broadcom STB sibling chip of the RPi 5.
The 7712 uses three reset controllers: rescal, for phy reset calibration;
bridge, for the bridge between the PCIe bus and the memory bus; and swinit,
which is a "soft" initialization of the PCIe HW.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/pci/brcm,stb-pcie.yaml           | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 7d2552192153..0925c520195a 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -21,6 +21,7 @@ properties:
           - brcm,bcm7425-pcie # Broadcom 7425 MIPs
           - brcm,bcm7435-pcie # Broadcom 7435 MIPs
           - brcm,bcm7445-pcie # Broadcom 7445 Arm
+          - brcm,bcm7712-pcie # Broadcom STB sibling of Rpi 5
 
   reg:
     maxItems: 1
@@ -96,10 +97,12 @@ properties:
       maxItems: 3
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
 
   reset-names:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
 
 required:
   - compatible
@@ -151,6 +154,27 @@ allOf:
         - 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
+        - reset-names
+
 unevaluatedProperties: false
 
 examples:
-- 
2.17.1

Re: [PATCH v6 01/13] dt-bindings: PCI: Change brcmstb maintainer and cleanup

[Krzysztof Kozlowski]

On 16/08/2024 00:57, Jim Quinlan wrote:
> Change maintainer: 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.
> 
> Also, alphabetize the compatible strings.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v6 02/13] dt-bindings: PCI: Use maxItems for reset controllers

[Krzysztof Kozlowski]

On 16/08/2024 00:57, Jim Quinlan wrote:
> Provide the maxItem property for the reset controllers and drop their
> superfluous descriptions.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

[Manivannan Sadhasivam]

On Thu, Aug 15, 2024 at 06:57:13PM -0400, Jim Quinlan wrote:
> V6 Changes
>   o Commit "Refactor for chips with many regular inbound windows"
>     -- Use u8 for anything storing/counting # inbound windows (Stan)
>     -- s/set_bar/add_inbound_win/g (Manivannan)
>     -- Drop use of "inline" (Manivannan)
>     -- Change cpu_beg to cpu_start, same with pcie_beg. (Manivannan)
>     -- Used writel_relaxed() (Manivannan)
>   o Use swinit reset if available
>     -- Proper use of dev_err_probe() (Stan)
>   o Commit "Use common error handling code in brcm_pcie_probe()"
>     -- Rewrite commit msg in paragraph form (Manivannan)
>     -- Refactor error path at end of probe func (Manivannan)
>     -- Proper use of dev_err_probe() (Stan)
>   o New commit "dt-bindings: PCI: Change brcmstb maintainer and cleanup"
>     -- Break out maintainer change and small cleanup into a
>        separate commit (Krzysztof)
> 

Looks like you've missed adding the review tags...

- Mani

> V5 Changes:
>   o All commits: Use imperative voice in commit subjects/messages
>        (Manivannan)
>   o Commit "PCI: brcmstb: Enable 7712 SOCs"
>     -- Augment commit message to include PCIe details and revision.
>        (Manivannan)
>   o Commit "PCI: brcmstb: Change field name from 'type' to 'model'"
>     -- Instead of "model" use "soc_base" (Manivannan)
>   o Commit "PCI: brcmstb: Refactor for chips with many regular inbound BARs"
>     -- Get rid of the confusing "BAR" variable names and types and use
>        something like "inbound_win". (Manivannan)
>   o Commit "PCI: brcmstb: PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE..."
>     -- Mention in the commit message that this change is in preparation
>        for the 7712 SoC. (Manivannan)
>   o Commit: "PCI: brcmstb: Use swinit reset if available"
>     -- Change reset name "swinit" to "swinit_reset" (Manivannan)
>     -- Add 1us delay for reset (Manivannan)
>     -- Use dev_err_probe() (Multiple reviewers)
>   o Commit "PCI: brcmstb: Use bridge reset if available"
>     -- Change reset name "bridge" to "bridge_reset" (Manivannan)
>     -- The Reset API can take NULL so need need to test variable
>        before calling (Manivannan)
>     -- Added a call to bridge_sw_init_set() method in probe()
>        as some registers cannot be accessed w/o this. (JQ)
>   o Commit "PCI: brcmstb: Use common error handling code in ..."
>     -- Use more descriptive goto label (Manivannan)
>     -- Refactor error paths to be less encumbered (Manivannan)
>     -- Use dev_err_probe() (Multiple reviewers)
>   o Commits "dt-bindings: PCI: brcmstb: ..."
>     -- Specify the "resets" and "reset-names" in the same manner
>        as does qcom,ufs.yaml specifies "clocks" and
>        "clock-names" (Krzysztof)
>     -- Drop reset desccriptions as they were pretty content-free
>        anyhow. (Krzysztof)
> 
> V4 Changes:
>   o Commit "Check return value of all reset_control_xxx calls"
>     -- Blank line before "return" (Stan)
>   o Commit "Use common error handling code in brcmstb_probe()"
>     -- Drop the "Fixes" tag (Stan)
>   o Commit "dt-bindings: PCI ..."
>     -- Separate the main commit into two: cleanup and adding the
>        7712 SoC (Krzysztof)
>     -- Fold maintainer change commit into cleanup change (Krzysztof)
>     -- Use minItems/maxItems where appropriate (Krzysztof)
>     -- Consistent order of resets/reset-names in decl and usage
>        (Krzysztof)
> 
> 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 (13):
>   dt-bindings: PCI: Change brcmstb maintainer and cleanup
>   dt-bindings: PCI: Use maxItems for reset controllers
>   dt-bindings: PCI: brcmstb: Add 7712 SoC description
>   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 windows
>   PCI: brcmstb: Check return value of all reset_control_xxx calls
>   PCI: brcmstb: Change field name from 'type' to 'soc_base'
>   PCI: brcmstb: Enable 7712 SOCs
> 
>  .../bindings/pci/brcm,stb-pcie.yaml           |  40 +-
>  drivers/pci/controller/pcie-brcmstb.c         | 513 +++++++++++++-----
>  2 files changed, 412 insertions(+), 141 deletions(-)
> 
> 
> base-commit: e724918b3786252b985b0c2764c16a57d1937707
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v6 01/13] dt-bindings: PCI: Change brcmstb maintainer and cleanup

[Florian Fainelli]

On 8/15/24 15:57, Jim Quinlan wrote:
> Change maintainer: 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.
> 
> Also, alphabetize the compatible strings.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

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

Re: [PATCH v6 02/13] dt-bindings: PCI: Use maxItems for reset controllers

[Florian Fainelli]

On 8/15/24 15:57, Jim Quinlan wrote:
> Provide the maxItem property for the reset controllers and drop their
> superfluous descriptions.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

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

Re: [PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

[Jim Quinlan]

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

On Fri, Aug 16, 2024 at 3:18 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Thu, Aug 15, 2024 at 06:57:13PM -0400, Jim Quinlan wrote:
> > V6 Changes
> >   o Commit "Refactor for chips with many regular inbound windows"
> >     -- Use u8 for anything storing/counting # inbound windows (Stan)
> >     -- s/set_bar/add_inbound_win/g (Manivannan)
> >     -- Drop use of "inline" (Manivannan)
> >     -- Change cpu_beg to cpu_start, same with pcie_beg. (Manivannan)
> >     -- Used writel_relaxed() (Manivannan)
> >   o Use swinit reset if available
> >     -- Proper use of dev_err_probe() (Stan)
> >   o Commit "Use common error handling code in brcm_pcie_probe()"
> >     -- Rewrite commit msg in paragraph form (Manivannan)
> >     -- Refactor error path at end of probe func (Manivannan)
> >     -- Proper use of dev_err_probe() (Stan)
> >   o New commit "dt-bindings: PCI: Change brcmstb maintainer and cleanup"
> >     -- Break out maintainer change and small cleanup into a
> >        separate commit (Krzysztof)
> >
>
> Looks like you've missed adding the review tags...


I didn't mention this in the cover letter but I update the review tags
at each version.  The problem is that if someone has reviewed a
commit, and then that commit is subsequently changed due to another
reviewer, I have to remove the existing tag of the first reviewer
because the code has changed.

Regards,
Jim Quinlan
Broadcom STB/CM
>
>
> - Mani
>
> > V5 Changes:
> >   o All commits: Use imperative voice in commit subjects/messages
> >        (Manivannan)
> >   o Commit "PCI: brcmstb: Enable 7712 SOCs"
> >     -- Augment commit message to include PCIe details and revision.
> >        (Manivannan)
> >   o Commit "PCI: brcmstb: Change field name from 'type' to 'model'"
> >     -- Instead of "model" use "soc_base" (Manivannan)
> >   o Commit "PCI: brcmstb: Refactor for chips with many regular inbound BARs"
> >     -- Get rid of the confusing "BAR" variable names and types and use
> >        something like "inbound_win". (Manivannan)
> >   o Commit "PCI: brcmstb: PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE..."
> >     -- Mention in the commit message that this change is in preparation
> >        for the 7712 SoC. (Manivannan)
> >   o Commit: "PCI: brcmstb: Use swinit reset if available"
> >     -- Change reset name "swinit" to "swinit_reset" (Manivannan)
> >     -- Add 1us delay for reset (Manivannan)
> >     -- Use dev_err_probe() (Multiple reviewers)
> >   o Commit "PCI: brcmstb: Use bridge reset if available"
> >     -- Change reset name "bridge" to "bridge_reset" (Manivannan)
> >     -- The Reset API can take NULL so need need to test variable
> >        before calling (Manivannan)
> >     -- Added a call to bridge_sw_init_set() method in probe()
> >        as some registers cannot be accessed w/o this. (JQ)
> >   o Commit "PCI: brcmstb: Use common error handling code in ..."
> >     -- Use more descriptive goto label (Manivannan)
> >     -- Refactor error paths to be less encumbered (Manivannan)
> >     -- Use dev_err_probe() (Multiple reviewers)
> >   o Commits "dt-bindings: PCI: brcmstb: ..."
> >     -- Specify the "resets" and "reset-names" in the same manner
> >        as does qcom,ufs.yaml specifies "clocks" and
> >        "clock-names" (Krzysztof)
> >     -- Drop reset desccriptions as they were pretty content-free
> >        anyhow. (Krzysztof)
> >
> > V4 Changes:
> >   o Commit "Check return value of all reset_control_xxx calls"
> >     -- Blank line before "return" (Stan)
> >   o Commit "Use common error handling code in brcmstb_probe()"
> >     -- Drop the "Fixes" tag (Stan)
> >   o Commit "dt-bindings: PCI ..."
> >     -- Separate the main commit into two: cleanup and adding the
> >        7712 SoC (Krzysztof)
> >     -- Fold maintainer change commit into cleanup change (Krzysztof)
> >     -- Use minItems/maxItems where appropriate (Krzysztof)
> >     -- Consistent order of resets/reset-names in decl and usage
> >        (Krzysztof)
> >
> > 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 (13):
> >   dt-bindings: PCI: Change brcmstb maintainer and cleanup
> >   dt-bindings: PCI: Use maxItems for reset controllers
> >   dt-bindings: PCI: brcmstb: Add 7712 SoC description
> >   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 windows
> >   PCI: brcmstb: Check return value of all reset_control_xxx calls
> >   PCI: brcmstb: Change field name from 'type' to 'soc_base'
> >   PCI: brcmstb: Enable 7712 SOCs
> >
> >  .../bindings/pci/brcm,stb-pcie.yaml           |  40 +-
> >  drivers/pci/controller/pcie-brcmstb.c         | 513 +++++++++++++-----
> >  2 files changed, 412 insertions(+), 141 deletions(-)
> >
> >
> > base-commit: e724918b3786252b985b0c2764c16a57d1937707
> > --
> > 2.17.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்

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

Re: [PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

[Florian Fainelli]

On 8/19/24 10:44, Jim Quinlan wrote:
> On Fri, Aug 16, 2024 at 3:18 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
>>
>> On Thu, Aug 15, 2024 at 06:57:13PM -0400, Jim Quinlan wrote:
>>> V6 Changes
>>>    o Commit "Refactor for chips with many regular inbound windows"
>>>      -- Use u8 for anything storing/counting # inbound windows (Stan)
>>>      -- s/set_bar/add_inbound_win/g (Manivannan)
>>>      -- Drop use of "inline" (Manivannan)
>>>      -- Change cpu_beg to cpu_start, same with pcie_beg. (Manivannan)
>>>      -- Used writel_relaxed() (Manivannan)
>>>    o Use swinit reset if available
>>>      -- Proper use of dev_err_probe() (Stan)
>>>    o Commit "Use common error handling code in brcm_pcie_probe()"
>>>      -- Rewrite commit msg in paragraph form (Manivannan)
>>>      -- Refactor error path at end of probe func (Manivannan)
>>>      -- Proper use of dev_err_probe() (Stan)
>>>    o New commit "dt-bindings: PCI: Change brcmstb maintainer and cleanup"
>>>      -- Break out maintainer change and small cleanup into a
>>>         separate commit (Krzysztof)
>>>
>>
>> Looks like you've missed adding the review tags...
> 
> 
> I didn't mention this in the cover letter but I update the review tags
> at each version.  The problem is that if someone has reviewed a
> commit, and then that commit is subsequently changed due to another
> reviewer, I have to remove the existing tag of the first reviewer
> because the code has changed.

It seems to me like you did the right thing here, the changes you did 
between v5 and v6 were substantial enough they invalidated the 
Reviewed-by tag(s) you had gotten previously.
-- 
Florian

Re: [PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

[Krzysztof Wilczyński]

Hello,

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

Applied to controller/brcmstb, thank you!

[01/13] dt-bindings: PCI: brcm,stb-pcie: Change brcmstb maintainer and cleanup
        https://git.kernel.org/pci/pci/c/2cd86a7c2346

[02/13] dt-bindings: PCI: brcm,stb-pcie: Use maxItems for reset controllers
        https://git.kernel.org/pci/pci/c/9014c6b92fbe

[03/13] dt-bindings: PCI: brcm,stb-pcie: Add 7712 SoC description
        https://git.kernel.org/pci/pci/c/154051eae687

[04/13] PCI: brcmstb: Use common error handling code in brcm_pcie_probe()
        https://git.kernel.org/pci/pci/c/5ccf0ade7937

[05/13] PCI: brcmstb: Use bridge reset if available
        https://git.kernel.org/pci/pci/c/996a76b913ab

[06/13] PCI: brcmstb: Use swinit reset if available
        https://git.kernel.org/pci/pci/c/50fd71c2d2fb

[07/13] PCI: brcmstb: PCI: brcmstb: Make HARD_DEBUG, INTR2_CPU_BASE offsets SoC-specific
        https://git.kernel.org/pci/pci/c/e42c556de029

[08/13] PCI: brcmstb: Remove two unused constants from driver
        https://git.kernel.org/pci/pci/c/092001a4ebd0

[09/13] PCI: brcmstb: Don't conflate the reset rescal with PHY ctrl
        https://git.kernel.org/pci/pci/c/1bed07ffeccb

[10/13] PCI: brcmstb: Refactor for chips with many regular inbound windows
        https://git.kernel.org/pci/pci/c/22877c1ac638

[11/13] PCI: brcmstb: Check return value of all reset_control_* calls
        https://git.kernel.org/pci/pci/c/363051fc3ab8

[12/13] PCI: brcmstb: Change field name from 'type' to 'soc_base'
        https://git.kernel.org/pci/pci/c/e78bade02796

[13/13] PCI: brcmstb: Enable 7712 SOCs
        https://git.kernel.org/pci/pci/c/1ae791a877e7

	Krzysztof