[PATCH v6 05/13] PCI: brcmstb: Use bridge reset if available

[Jim Quinlan]

On Tue, Aug 20, 2024 at 7:42 PM Stanimir Varbanov <svarbanov at suse.de> wrote:
>
> Hi Jim,
>
> On 8/20/24 00:49, Jim Quinlan wrote:
> > On Mon, Aug 19, 2024 at 3:39 PM Stanimir Varbanov <svarbanov at suse.de> wrote:
> >>
> >> Hi Jim,
> >>
> >> On 8/19/24 21:09, Jim Quinlan wrote:
> >>> On Sat, Aug 17, 2024 at 1:41 PM Stanimir Varbanov <svarbanov at suse.de> wrote:
> >>>>
> >>>> Hi Jim,
> >>>>
> >>>> On 8/16/24 01:57, Jim Quinlan wrote:
> >>>>> 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.
> >>>>>
> >>>>> Signed-off-by: Jim Quinlan <james.quinlan at broadcom.com>
> >>>>> ---
> >>>>>  drivers/pci/controller/pcie-brcmstb.c | 24 +++++++++++++++++++-----
> >>>>>  1 file changed, 19 insertions(+), 5 deletions(-)
> >>>>
> >>>> Reviewed-by: Stanimir Varbanov <svarbanov at suse.de>
> >>>>
> >>>> One problem though on RPi5 (bcm2712).
> >>>>
> >>>> With this series applied + my WIP patches for enablement of PCIe on
> >>>> bcm2712 when enable the pcie1 and pcie2 root ports in dts, I see kernel
> >>>> boot stuck on pcie2 enumeration and I have to add this [1] to make it
> >>>> work again.
> >>>>
> >>>> Some more info about resets used:
> >>>>
> >>>> pcie0 @ 100000:
> >>>>         resets = <&bcm_reset 5>, <&bcm_reset 42>, <&pcie_rescal>;
> >>>>         reset-names = "swinit", "bridge", "rescal";
> >>>>
> >>>> pcie1 @ 110000:
> >>>>         resets = <&bcm_reset 7>, <&bcm_reset 43>, <&pcie_rescal>;
> >>>>         reset-names = "swinit", "bridge", "rescal";
> >>>>
> >>>> pcie2 @ 120000:
> >>>>         resets = <&bcm_reset 9>, <&bcm_reset 44>, <&pcie_rescal>;
> >>>>         reset-names = "swinit", "bridge", "rescal";
> >>>>
> >>>>
> >>>> I changed "swinit" reset for pcie2 to <&bcm_reset 9> (it is 32 in
> >>>> downstream rpi kernel) because otherwise I'm unable to enumerate RP1
> >>>> south bridge at all.
> >>>>
> >>>> Any help will be appreciated.
> >>>
> >>> Hi Stan,
> >>> Let me look into this.  Why is one of the controllers turning off --
> >>> is it not populated with a device?
> >>
> >> Yes, I enabled pcie1 but no PCI endpoint devices attached on the
> >> expansion connector.
> >
> > Hi Stan,
> >
> > I looked at our similar STB chip that has a rescal controller and
> > multiple PCIe controllers and it doesn't have this problem.  Our 7712
> > doesn't  have this problem either, only because it only has one PCIe
> > controller.
> >
> > However, using my 7712 and unbinding the device (invokes
> > brcm_pcie_remove()) shows me behavior similar to yours (2712).  What I
> > do is read the rescal registers after the unbind, and they will either
> > be dead or alive.  If I comment out the
> > "pcie->bridge_sw_init_set(pcie, 1);" call, the rescal is still dead
> > after unbind.  However if I comment out that AND the
> > "clk_disable_unprepare(pcie->clk);" call,  the rescal registers remain
> > alive after unbind.
>
> Thank you. No idea why the clock is not used on 2712 (or at least it is
> not populated on RPi downstream kernel.

Hi Stan,

Most of the clocks on the STB chips come up active so one does not
have to turn them on and off to have the device function.  It helps
power savings to do this although I'm not sure it is significant.
>
> >
> > Perhaps you don't see the dependence on the PCIe clocks if the 2712
> > does not give the PCIe node a clock property and instead keeps its
> > clocks on all of the time.  In that case I would think that your
> > solution would be fine.
>
> What you mean by my solution? The one where avoiding assert of
> bridge_reset at link [1] bellow?

Yes.
>
> If so, I still cannot understand the relation between bridge_reset and
> rescal as the comment mentions:
>
> "Shutting down this bridge on pcie1 means accesses to rescal block will
> hang the chip if another RC wants to assert/deassert rescal".

I was just describing my observations; this should not be happening.
I would say it is a HW bug for the 2712.  I can file a bug against the
2712 but that will not help us right now.  From what I was told by HW,
asserting the PCIe1 bridge reset does not affect the rescal settings,
but it does freeze access to the rescal registers, and that is game
over for the other PCIe controllers accessing the rescal registers.

Regards,
Jim Quinlan
Broadcom STB/CM

>
> ~Stan
> you
> > Regards,
> > Jim Quinlan
> > Broadcom STB/CM
> >
> >
> >
> >>
> >>>
> >>> As you probably know the 7712 only has access to PCIe1.  But we do
> >>> have another chip with two controllers and I will try to reproduce
> >>> your failure and get to the bottom of it.
> >>
> >> Thank you for the help.
> >>
> >> ~Stan
> >>
> >>>
> >>> Regards,
> >>> Jim Quinlan
> >>> Broadcom STB/CM
> >>>>
> >>>> ~Stan
> >>>>
> >>>> [1]
> >>>> https://github.com/raspberrypi/linux/blob/rpi-6.11.y/drivers/pci/controller/pcie-brcmstb.c#L1711
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4210 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240821/822ad72b/attachment.p7s>