Message-ID: <CA+-6iNz4+xP4abf6w6bcVwFxvjx8OhDZXNi5bwfaCNvyF2Mtng@mail.gmail.com>
Date: Mon, 19 Aug 2024 13:44:33 -0400
From: Jim Quinlan <james.quinlan@...adcom.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: linux-pci@...r.kernel.org, Nicolas Saenz Julienne <nsaenz@...nel.org>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Lorenzo Pieralisi <lorenzo.pieralisi@....com>, 
	Cyril Brulebois <kibi@...ian.org>, Stanimir Varbanov <svarbanov@...e.de>, 
	Krzysztof Kozlowski <krzk@...nel.org>, bcm-kernel-feedback-list@...adcom.com, 
	jim2101024@...il.com, 
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, 
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>, 
	open list <linux-kernel@...r.kernel.org>, 
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@...ts.infradead.org>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v6 00/13] PCI: brcnstb: Enable STB 7712 SOC

On Fri, Aug 16, 2024 at 3:18 AM Manivannan Sadhasivam
<manivannan.sadhasivam@...aro.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
> >
>
> --
> மணிவண்ணன் சதாசிவம்

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)

Powered by blists - more mailing lists