From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=sd9i9zGl; spf=pass (domain: linaro.org, ip: 209.85.128.67, mailfrom: leif.lindholm@linaro.org) Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by groups.io with SMTP; Fri, 30 Aug 2019 08:32:44 -0700 Received: by mail-wm1-f67.google.com with SMTP id p13so7931782wmh.1 for ; Fri, 30 Aug 2019 08:32:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=oYYlot8zUt5Vbq/n2uehx/HX/uN2rollv30xQXp2HeI=; b=sd9i9zGla0f9ODkEMEn/szig4YYT8Y3NATx3kYfAAYQ8ng2as1p44AglwS2mOYDheu xj8LSTdcOMJCHYk5DxWRj5Ilnp/NPMoPD6cuJ/3F1U18LqVC3HuGf3w037pfoAt25CeP gZBgBDH9Vyf2hLOp/zwQdNO5PfdvH1sA8cI4KdALK3IIbyAb+Sqaa/kEZITQEut6tSZl KIF9nrCs62jYw1huD6wSKH1c277R1lZ4op73RDvpklCViHd+OKp7xgUeHeVlUCROValP K2kAObdvWWMazq0zCAUL7ccz4LsPklXPypQRzXdNDRf7POvDOUEDR//HjSBiDBnfCYdD a4Wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=oYYlot8zUt5Vbq/n2uehx/HX/uN2rollv30xQXp2HeI=; b=edG8afmm3Hh0O+/7ZEHROJXfhdiOwUQyhsRIoCTKWq6kQHT1eEVF9KUAV92DFu5LQF 5HA+tMUpzRwT1IyeqTBoxZarDSLAJsw8ktFKX4XuTFnweh5wXzd9IN8H7eOkBHrKJ0kk TCEgSNb5EI3jZgoi8r9uhvy92C3c9zpoEwaNw4WIhrAdTzCNKpuw1kzjwQ6kVj877HGk z95EmOzugwdZ0wyWgv7OgXwY0Pq0kbxcAv1PmmNkzcZTrwShxAmbUcmoDOqqKoBMM60S scXK9UkRj6UkSYWwy1elyJRSvTIyKSCIDF9yzD1FVDrSZF/jAsu6vrBOdw9jwCE07uRL 6ysw== X-Gm-Message-State: APjAAAXtUHOZYvcbSpjvC65H3wLXfVdhKQ2lWQws7pwKT1hqUGZcGB1D brKV5LoTAf0A+j8/0jZ3Y3I36Q== X-Google-Smtp-Source: APXvYqzKGQ7UzyDncjXJxyA2yVbAhv86B1hQMDc59bsiZzw3sHx+m2MvtpulrGYdlTgr/MLo1o02+Q== X-Received: by 2002:a1c:f512:: with SMTP id t18mr4106622wmh.165.1567179163152; Fri, 30 Aug 2019 08:32:43 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id i5sm8559195wrn.48.2019.08.30.08.32.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 30 Aug 2019 08:32:42 -0700 (PDT) Date: Fri, 30 Aug 2019 16:32:41 +0100 From: "Leif Lindholm" To: Pete Batard Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org Subject: Re: [edk2-platforms: PATCH v2 1/1] Platform/Rpi3: Add compatible property to the "usb" Device Tree node Message-ID: <20190830153241.GW29255@bivouac.eciton.net> References: <20190823122050.5244-1-pete@akeo.ie> <20190823122050.5244-2-pete@akeo.ie> <20190829135459.GS29255@bivouac.eciton.net> <981c8b57-1be1-0cab-7019-969ce4c6fa9a@akeo.ie> MIME-Version: 1.0 In-Reply-To: <981c8b57-1be1-0cab-7019-969ce4c6fa9a@akeo.ie> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Aug 29, 2019 at 04:26:58PM +0100, Pete Batard wrote: > Hi Leif, > > On 2019.08.29 14:54, Leif Lindholm wrote: > > 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,') > > Sorry, should have been 'brcm,bcm2835-usb' above. > > As mentioned in the cover letter, we're basically adding to the compatible > string list so that: > compatible = "brcm,bcm2708-usb"; > becomes: > compatible = "brcm,bcm2708-usb", "brcm,bcm2835-usb"; > > So the part before the comma should always be "brcm", and it's only the part > after that should be "bcm" (which, alas, makes it easy to introduce typos). > > In other words, the typo is in the commit message only. > > I did validate the changes proposed by this patch on real hardware, so I can > vouch for the code being correct, insofar as the compatible strings there > are concerned. Thanks. I was 99% sure that was the case, but I prefer having these things explicitly clarified before I commit. > > If here, I can fix up on committing. > > If you can fix the typo in the commit message, that would be great. Reviewed-by: Leif Lindholm Pushed as 5f003136c2bf. > Regards, > > /Pete > > > > > / > > 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 > > > --- > > > 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 > > > >