Re: [PATCH v2 edk2-platforms 00/20] Platform/Broadcom: Add Raspberry Pi 3 support

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Tue, 11 Dec 2018 at 21:16, Pete Batard <pete@akeo.ie> wrote:
>
> Hi Leif,
>
> On 2018.12.11 18:10, Leif Lindholm wrote:
> > Hi Pete,
> >
> > Many thanks for this.
> > I expect Ard will give more detailed review, since he has some
> > familiarity with the port. But I wanted to make a few comments.
> >
> > Could you cc me as well on any future revisions?
>
> Will do.
>
> > On Mon, Dec 10, 2018 at 12:38:33PM +0000, Pete Batard wrote:
> >> Version History:
> >>
> >> * v2: Break down the content into logical entities of more manageable size.
> >>        Please pay attention to the *NON-OSI* tagged patches, that should be
> >>        applied to edk2-non-osi instead of edk2-platforms.
> >>
> >> Preamble:
> >>
> >> Because of its price point, ease of use and availability, the Raspberry Pi is
> >> undeniably one of the most successful ARM platform in existence today. Its
> >> widespread adoption therefore makes it a perfect fit as an EDK2 platform.
> >>
> >> However, up until now, the Raspberry Pi hasn't been supported as a bona fide
> >> platform in our repository. This series of patches remedies that by introducing
> >> the Raspberry Pi 3 Model B and Model B+ as a viable EDK2 platforms.
> >>
> >> With regards to the latter:
> >> * Even though the ARM Trusted Firmware binary blobs are subject to a
> >>    BSD-3-Clause licence, which may be compatible with the EDK2 one, we chose
> >>    to follow the lead of other platforms that provide ATF binaries in non OSI.
> >
> > I _think_ all of the ATF binaries we have in non-osi are
> > non-upstream. If the port for the rpi3 is upstream, I would be just as
> > happy to have simple build instructions of a known good commit (with
> > notes on toolchain version tested) in the Readme.md - and possibly a
> > placeholder directory with a .inf in to drop a prebuilt image into.
>
> Well, while it is upstream, it is built using a custom rpi3 specific
> option which we had ATF to add, so that we could get a memory mapping
> that works with Windows (default one was okay for Linux but not
> Windows). So I doubt we will ever get upstream binaries that we can use
> as is, if that's what you are alluding to.
>
> For the record, there's a readme located in the directory where the ATF
> binaries are provided, that has the full build command we used, as well
> as the description of the memory mapping.
>
> > (If it isn't upstream, non-osi is the way to go for now.)
> > ((This isn't a "do what I say", this is a "you don't have to".))
>
> I guess that means we'll keep the binaries in non-osi for now then.
>
> However, I have been thinking about renaming the directory that contains
> the ATF blobs from "Binary/" to "Atf/" to make it more explicit. I'll
> probably do for the v3, unless someone has a different idea.
>
> >> * The Device Tree binaries (and source descriptors) are subject to a GPLv2
> >>    license, as per the ones published by the Raspberry Pi Foundation.
> >
> > This feels somewhat suboptimal. Is there someone you could ask whether
> > they'd be willing to dual license?
>
> As per the official Raspberry Pi firmware repo at
> https://github.com/raspberrypi/firmware:
>
>    The dtbs, overlays and associated README are built from Linux kernel
>    sources, released under the GPL (see boot/COPYING.linux)
>
> The part about it being built from kernel sources makes me think that,
> unless we get all the Linux developers who touched on the relevant parts
> of the Device Tree to agree on dual licensing, which would be a
> tremendous effort, it's going to have to remain GPLv2.
>

Looking at the kernel history, it seems those DT files were
contributed with no license at all, and were relicensed as GPL2 when
SPDX tags were added globally to files without a license.

It might make sense to reach out to Eric Anholt to ask him what the
intent was when those files were added.

> As a result, even if it wanted to, I doubt the Raspberry Pi foundation
> would have the authority to dual license the .dtb's/.dts's. But if you
> see it differently, I can try to get in touch with the Foundation.
>
>
> By the way, since I have just seen from Ard's recent OverdriveBoard
> patches that we do have the ability to compile a Device Tree from source
> (which I wasn't aware of), I'm going to point out that we probably don't
> want to do that for this platform.
>

That is fine.

> The reason is: we need to make it easy for users of the Pi3 Model B+ to
> override the default Model B Device Tree, which is what we embed in the
> firmware (since it's the most compatible one), and of course it'll be a
> lot more annoying for people to do that all they have is the .dts. And
> we can't refer users to the official Pi Foundation .dtb's, as they
> produce USB keyboard issues and are missing some of the features we want.
>
> Moreover, even if we were to decide that we'd like to have at least the
> Model B Device Tree built from source, we'd still also need to provide
> the .dtb for that one as, if you want to use one of the many DT overlays
> provided by the Pi Foundation (see
> https://github.com/raspberrypi/firmware/tree/master/boot/overlays) then
> you have no choice but to also provide the base .dtb at the same time,
> which means that Rpi3 users will want to have easy access to a
> precompiled custom bcm2710-rpi-3-b.dtb as well.
>
> > If not, this is certainly yet another argument for an edk2-non-bsd
> > repository or suchlike.
> >
> >> * The DwUsbHostDxe driver is subject to a GPLv2 license
> >
> > And this.
>
> I can look into this.
>
> I'll just point out that out of the 4 developers that officially appear
> to have had a hand with crafting this code, one is listed as... Linaro. ;)
>
> >> * The Logo source code is under an EDK2 license, but the logo itself, which
> >>    we obtained authorisation to use from the Raspberry Pi Foundation itself,
> >>    after detailing our planned usage, is subject to the trademark licensing
> >>    terms put forward by the Raspberry Pi Foundation, and therefore we chose
> >>    to move the whole Logo driver under non OSI.
> >
> > Yes, that's definitely the right thing to do.
> >
> >> Additional Notes:
> >>
> >> * We chose to introduce the platform under Broadcom/Bcm283x/ as we consider
> >>    first, that additional Broadcom platforms may be introduced, and second that
> >>    even though only Bcm2837 (i.e. Pi 3) platforms are supported from the current
> >>    RaspberryPiPkg, support may be added for Bcm2836 (Pi 2) in the future, hence
> >>    our decision to use a generic Bcm283x/ subdirectory.
> >
> > I fully agree with this, but...
> >
> > Now for the bikeshedding: Bcm2837 is the SoC used in Pi 3. It is not
> > an alternative name for the Pi 3. And since the board design is open,
> > it is plausible that there may be derivative boards.
> > So ideally, I would like to see something like:
> >
> > Platform/RaspberryPi/Pi3
> > Silicon/Broadcom/Bcm283x
> >
> > With (if practically possible) a split between SoC and board modules
> > and configuration files.
>
> Okay. I was half expecting such a request, so I'll see what I can do.
>
>
> I'll be waiting to see if Ard has additional feedback before I start
> working on this as part of a v3.
>

I will try to go over each patch tomorrow.

> > I would expect the Pi3.dsc/.fdf to be fairly minimal and including
> > .dsc.inc/.fdf.inc files from Bcm283x.
> >
> >> * The ARM Trusted Firmware being used is a vanilla version built from the
> >>    latest tree, as we worked with that project to get necessary patches
> >>    integrated.
> >
> > Sweet!
> >
> >> * Detailed instructions on how to build and test the platform firmware are
> >>    included in the Readme.md found at the root of the platform.
> >
> > Splendid!
> >
> >> * As detailed in the Readme, the resulting platform firmware has been
> >>    successfully used to install and run Linux OSes, such as Ubuntu 18.10, as
> >>    well as Windows 10 1809 (*full* UI version, not IoT).
> >
> > Very nice!
> >
> > Could you also add an entry to the top-level Readme.md with a link to
> > the rpi3 Readme.md? (But hold off until I push some updates from
> > Nariman.)
>
> Will do.
>
> Regards,
>
> /Pete