On Tue, Sep 10, 2024 at 1:59 PM Bjorn Helgaas [off-list ref] wrote:

On Tue, Sep 10, 2024 at 01:30:41PM -0400, Jim Quinlan wrote:

quoted

On Tue, Sep 3, 2024 at 10:26 AM Jim Quinlan [off-list ref] wrote:

quoted

On Mon, Sep 2, 2024 at 3:18 PM Bjorn Helgaas [off-list ref] wrote:

quoted

On Thu, Aug 15, 2024 at 06:57:18PM -0400, Jim Quinlan wrote:

quoted

The 7712 SOC has a bridge reset which can be described in the device tree.
Use it if present.  Otherwise, continue to use the legacy method to reset
the bridge.

quoted

 static void brcm_pcie_bridge_sw_init_set_generic(struct brcm_pcie *pcie, u32 val)
 {
-     u32 tmp, mask =  RGR1_SW_INIT_1_INIT_GENERIC_MASK;
-     u32 shift = RGR1_SW_INIT_1_INIT_GENERIC_SHIFT;
+     if (val)
+             reset_control_assert(pcie->bridge_reset);
+     else
+             reset_control_deassert(pcie->bridge_reset);

-     tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
-     tmp = (tmp & ~mask) | ((val << shift) & mask);
-     writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+     if (!pcie->bridge_reset) {
+             u32 tmp, mask =  RGR1_SW_INIT_1_INIT_GENERIC_MASK;
+             u32 shift = RGR1_SW_INIT_1_INIT_GENERIC_SHIFT;
+
+             tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+             tmp = (tmp & ~mask) | ((val << shift) & mask);
+             writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+     }

This pattern looks goofy:

  reset_control_assert(pcie->bridge_reset);
  if (!pcie->bridge_reset) {
    ...

If we're going to test pcie->bridge_reset at all, it should be first
so it's obvious what's going on and the reader doesn't have to go
verify that reset_control_assert() ignores and returns success for a
NULL pointer:

  if (pcie->bridge_reset) {
    if (val)
      reset_control_assert(pcie->bridge_reset);
    else
      reset_control_deassert(pcie->bridge_reset);

    return;
  }

  u32 tmp, mask =  RGR1_SW_INIT_1_INIT_GENERIC_MASK;
  ...

Will do.

Hi Bjorn,

It is not clear to me if you want a new series -- which would be V7 --
or you are okay with the current series V6.  If the latter, someone
sent in a fixup commit which must be included.
Please advise.

Krzysztof amended this on the branch.  Take a look here and verify
that it makes sense to you:

  https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=controller/brcmstb#n752

If that looks right to you, no need to post a new v7.

I think Krzysztof also integrated an "int num_inbound_wins" fix; is
that the one you mean?  If I'm thinking of the right one, you can
check that at:

  https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=controller/brcmstb#n1034

quoted

quoted

quoted

Krzysztof, can you amend this on the branch?

It will also make the eventual return checking and error message
simpler because we won't have to initialize "ret" first, and we can
"return 0" directly for the legacy case.

Bjorn

Sorry, I didn't see this email until now.  The changes look good,
thanks for making them.

Regards,
Jim