On Fri, Aug 16, 2024 at 3:18 AM Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> 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:
smime.p7s
Description: S/MIME Cryptographic Signature
