Re: [edk2-platforms: PATCH v2 1/1] Platform/Rpi3: Add compatible property to the "usb" Device Tree node

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Fri, Aug 23, 2019 at 01:20:50PM +0100, Pete Batard wrote:
> Some Linux kernels (e.g. Debian) require "bcm,bcm2835-usb" to be present in

Is the typo here or in the code? ('bcm,' vs. 'brcm,')
If here, I can fix up on committing.

/
    Leif

> the list of compatible properties for the "usb" node, else they are unable
> to handle some USB devices.
> 
> This patch ensures that the compatible property is added if not present.
> 
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c | 75 ++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> index 45ffe2e394a2..eb8048930c30 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> @@ -135,6 +135,76 @@ UpdateMacAddress (
>    return EFI_SUCCESS;
>  }
>  
> +//
> +// Add "bcm2835-usb" to the USB compatible property list, if not present.
> +// Required because some Linux kernels can't handle USB devices otherwise.
> +//
> +STATIC
> +EFI_STATUS
> +AddUsbCompatibleProperty (
> +  VOID
> +  )
> +{
> +  CONST CHAR8   Prop[]    = "brcm,bcm2708-usb";
> +  CONST CHAR8   NewProp[] = "brcm,bcm2835-usb";
> +  CONST CHAR8   *List;
> +  CHAR8         *NewList;
> +  INT32         ListSize;
> +  INTN          Node;
> +  INTN          Retval;
> +
> +  // Locate the node that the 'usb' alias refers to
> +  Node = fdt_path_offset (mFdtImage, "usb");
> +  if (Node < 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate 'usb' alias\n", __FUNCTION__));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // Get the property list. This is a list of NUL terminated strings.
> +  List = fdt_getprop (mFdtImage, Node, "compatible", &ListSize);
> +  if (List == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate properties\n", __FUNCTION__));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // Check if the compatible value we plan to add is already present
> +  if (fdt_stringlist_contains (List, ListSize, NewProp)) {
> +    DEBUG ((DEBUG_INFO, "%a: property '%a' is already set.\n",
> +      __FUNCTION__, NewProp));
> +    return EFI_SUCCESS;
> +  }
> +
> +  // Make sure the compatible device is what we expect
> +  if (!fdt_stringlist_contains (List, ListSize, Prop)) {
> +    DEBUG ((DEBUG_ERROR, "%a: property '%a' is missing!\n",
> +      __FUNCTION__, Prop));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // Add the new NUL terminated entry to our list
> +  DEBUG ((DEBUG_INFO, "%a: adding '%a' to the properties\n",
> +    __FUNCTION__, NewProp));
> +
> +  NewList = AllocatePool (ListSize + sizeof (NewProp));
> +  if (NewList == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to allocate memory\n", __FUNCTION__));
> +    return EFI_OUT_OF_RESOURCES;;
> +  }
> +  CopyMem (NewList, List, ListSize);
> +  CopyMem (&NewList[ListSize], NewProp, sizeof (NewProp));
> +
> +  Retval = fdt_setprop (mFdtImage, Node, "compatible", NewList,
> +             ListSize + sizeof (NewProp));
> +  FreePool (NewList);
> +  if (Retval != 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to update properties (%d)\n",
> +      __FUNCTION__, Retval));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  STATIC
>  EFI_STATUS
>  CleanMemoryNodes (
> @@ -486,6 +556,11 @@ FdtDxeInitialize (
>      Print (L"Failed to update MAC address: %r\n", Status);
>    }
>  
> +  Status = AddUsbCompatibleProperty ();
> +  if (EFI_ERROR (Status)) {
> +    Print (L"Failed to update USB compatible properties: %r\n", Status);
> +  }
> +
>    if (Internal) {
>      /*
>       * A GPU-provided DTB already has the full command line.
> -- 
> 2.21.0.windows.1
>