Re: [edk2-platforms PATCH v2] Platform/RaspberryPi: fix pci DT node address in SyncPcie()

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 7 Oct 2022 at 22:39, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
> On 10/6/22 12:46, Adrien Thierry wrote:
> > To make sure the XHCI controller does not get reset by Linux in DT mode,
> > we remove its pci parent node from the device tree. However, the pci
> > node address has been updated in the Raspberry Pi 4 device tree [1] and
> > no longer matches the one we are trying to remove in SyncPcie(). This
> > results in the XHCI controller actually being reset by Linux, which
> > leads to errors during USB initialization:
> >
> > [    3.563963] xhci_hcd 0000:01:00.0: xHCI Host Controller
> > [    3.569538] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 1
> > [    3.577452] xhci_hcd 0000:01:00.0: hcc params 0x002841eb hci version 0x100 quirks 0x0000040000000890
> > [    3.587725] xhci_hcd 0000:01:00.0: xHCI Host Controller
> > [    3.593115] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 2
> > [    3.600693] xhci_hcd 0000:01:00.0: Host supports USB 3.0 SuperSpeed
> > [    3.608106] hub 1-0:1.0: USB hub found
> > [    3.612026] hub 1-0:1.0: 1 port detected
> > [    3.616819] hub 2-0:1.0: USB hub found
> > [    3.620726] hub 2-0:1.0: 4 ports detected
> > [    3.875902] usb 1-1: new high-speed USB device number 2 using xhci_hcd
> > [    4.008123] usb 1-1: device descriptor read/64, error -71
> > [    4.256088] usb 1-1: device descriptor read/64, error -71
> > [    4.495882] usb 1-1: new high-speed USB device number 3 using xhci_hcd
> > [    4.628111] usb 1-1: device descriptor read/64, error -71
> > [    4.872083] usb 1-1: device descriptor read/64, error -71
> > [    5.407888] usb 1-1: new high-speed USB device number 4 using xhci_hcd
> > [    6.023964] xhci_hcd 0000:01:00.0: Setup ERROR: setup address command for slot 1.
> > [    6.239977] xhci_hcd 0000:01:00.0: Setup ERROR: setup address command for slot 1.
> >
> > This patch allows matching any address for the pci node, thus working
> > with both legacy and new device trees.
> >
>
> Thanks a lot for looking into this! This looks fine to me, the only
> thing that poped out was that the uefi checkpatch complained about the
> linux log in the commit message, but I think that should be ok.
>
> so:
>
> Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
>
> and I also did some basic testing on a rpi4's so:
>
> Tested-by: Jeremy Linton <jeremy.linton@arm.com>
>
> too.
>
> Thanks again!
>

Pushed as 3b889620a53c..ad00518399fc

Thanks all,

> > [1] https://lore.kernel.org/all/20210831125843.1233488-1-nsaenzju@redhat.com/
> >
> > Fixes: efff29cdcdb7 ("Platform/RaspberryPi: Always use non translating DMA in DT mode")
> > Signed-off-by: Adrien Thierry <athierry@redhat.com>
> > ---
> >
> > v1->v2: Match both pci@1,0 and pci@0,0
> >
> > I chose to remove the address altogether, since there is only one pci
> > node for the Raspberry Pi 4 and thus no ambiguity. Let me know if you
> > prefer explicitly matching pci@1,0 and pci@0,0.
> >
> >   Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> > index e72d132b18..cbbc2ad30d 100644
> > --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> > +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> > @@ -388,14 +388,14 @@ SyncPcie (
> >      * triggering the mailbox by removing the node.
> >      */
> >
> > -  Node = fdt_path_offset (mFdtImage, "/scb/pcie@7d500000/pci@1,0");
> > +  Node = fdt_path_offset (mFdtImage, "/scb/pcie@7d500000/pci");
> >     if (Node < 0) {
> >       // This can happen on CM4/etc which doesn't have an onboard XHCI
> > -    DEBUG ((DEBUG_INFO, "%a: failed to locate /scb/pcie@7d500000/pci@1/usb@1\n", __FUNCTION__));
> > +    DEBUG ((DEBUG_INFO, "%a: failed to locate /scb/pcie@7d500000/pci\n", __FUNCTION__));
> >     } else {
> >       Retval = fdt_del_node (mFdtImage, Node);
> >       if (Retval != 0) {
> > -      DEBUG ((DEBUG_ERROR, "Failed to remove /scb/pcie@7d500000/pci@1/usb@1\n"));
> > +      DEBUG ((DEBUG_ERROR, "Failed to remove /scb/pcie@7d500000/pci\n"));
> >         return EFI_NOT_FOUND;
> >       }
> >     }
> >
> > base-commit: ae75c51f27e21036b6ee021a2d5b9f365f951413
>