Re: [PATCH 1/1 RFC] treewide: Deprecate OF_PRIOR_STAGE

[Simon Glass <sjg@chromium.org>]

Hi Thomas,

On Fri, 15 Oct 2021 at 10:19, Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote:
>
> Hi Tom,
>
> Tom Rini <trini@konsulko.com> writes:
>
> > On Wed, Oct 13, 2021 at 01:36:00PM -0400, Thomas Fitzsimmons wrote:
> >> Simon Glass <sjg@chromium.org> writes:
> >>
> >> [...]
> >>
> >> > On Wed, 13 Oct 2021 at 10:26, Thomas Fitzsimmons <fitzsim@fitzsim.org> wrote:
> >> >>
> >> >> Simon Glass <sjg@chromium.org> writes:
> >> >>
> >> >> [...]
> >> >>
> >> >> >> > I think one option is better than two. I have a slight preference for
> >> >> >> > OF_PRIOR_STAGE because it is board-agnostic, but I'm not sure it
> >> >> >> > matters, since some of these boards are doing strange things anyway
> >> >> >> > and cannot use OF_PRIOR_STAGE. So let's go with this.
> >> >> >>
> >> >> >> For now it's easier getting rid of OF_PRIOR_STAGE than OF_BOARD.
> >> >> >> Once we unify OF_PRIOR_STAGE/OF_BOARD and OF_HOSTFILE, then
> >> >> >> I can send a patch on top of that, which removes the board_fdt_blob_setup()
> >> >> >> and just stores the address in a similar fashion to the removed
> >> >> >> 'prior_stage_fdt_address'.  That way we can get rid of architecture
> >> >> >> specific constructs wrt to DT in gd.  The callback is a bit more of a pain to
> >> >> >> maintain for multiple boards but is more flexible than an address in a
> >> >> >> register.  In any case we can do something along the lines of:
> >> >> >>
> >> >> >> Check register (or blob list or whatever)
> >> >> >> if (valid dtb)
> >> >> >>     fixup/amend/use (depending on what we decide)
> >> >> >> else
> >> >> >>    arch specific callback
> >> >> >>
> >> >> >> That should give us enough flexibility to deal with future boards (famous
> >> >> >> last words).
> >> >> >
> >> >> > SGTM
> >> >>
> >> >> This sounds like a good generalization that would still work for the
> >> >> bcm7445 and bcm7260 boards.  I'll test this approach on the evaluation
> >> >> boards I have.
> >> >>
> >> >> For the BCM7445 I may be able to import the evaluation board device tree
> >> >> that Broadcom publishes as part of stblinux.  At runtime I may need to
> >> >> merge some of the in-memory items generated by BOLT, but I'll try to
> >> >> make this work.
> >> >
> >> > That would be good.
> >> >
> >> >> The BCM7260 DTS is not publicly available though, as far as I know.
> >> >
> >> > Presumably it can be dumped from U-Boot?
> >>
> >> Technically, yes, but I wouldn't want to publish the result for various
> >> reasons; e.g., it would be specific to the evaluation boards I have, and
> >> it may contain vendor-specific fields.  I'd much rather this one remain
> >> a stub, until/unless Broadcom publishes a generic BCM7260 DTS under a
> >> free license.
> >
> > Also note that the notion that the U-Boot source tree _must_ contain a
> > dts for a given board is something we're very much debating still, in
> > another thread, if you're inclined to read and chime in there as well
> > with more details about the broadcom use case and technical/legal
> > limitations.  Thanks!
>
> Sure.  I read through [1]; here are my thoughts from the BCM7445/BCM7260
> perspective.
>
> First of all, for background, BCM7445 and BCM7260 are partial ports of
> U-Boot.  They're meant to allow using nice U-Boot features like hush and
> FIT image loading on these platforms.  But they do not handle low-level
> initialization -- that's done by BOLT, the proprietary
> first-and-second-stage bootloader -- and they don't support configuring
> all of the hardware on these boards.  Instead these ports include a
> small set of drivers (e.g., SPI, eMMC, serial) and configuration that is
> needed to make use of the higher level features.
>
> At the time I contributed the BCM7445 support, README called OF_CONTROL
> an experimental feature, and device driver configuration was
> alternatively allowed to live in board-specific header files.  My
> initial local implementation did that, but then I switched to OF_CONTROL
> before submitting the patches, since then I could get some of U-Boot's
> driver configuration from the prior stage (BOLT) dynamically, instead of
> hard-coding addresses in U-Boot source code.  The proposed new policy
> would require me to (re-)add these hard-coded values, albeit in a DTS,
> not a header file.  IMO that's probably a good/fair requirement for the
> non-technical reasons in [1].
>
> The second section of [1] says: "Every board in U-Boot must include a
> devicetree sufficient to build and boot that board on suitable hardware
> (or emulation)."  I initially read that as "boot to Linux", and so I was
> thinking the device tree checked into the U-Boot tree had to be
> sufficient to boot Linux and configure every device that Linux supports.
> One of Simon's responses [2] clarified for me that the policy proposal
> was about the control DTB for U-Boot.
>
> Now I believe the intent of the proposed policy (stated in the
> "Devicetree source" section of [1]) is something like "each port in
> U-Boot must have an in-tree device tree that is sufficient to boot/run
> *U-Boot itself* on at least one representative board designed around
> that SoC".  That would make sense to me; it would permit not-full-Linux
> device trees that configure only the device drivers that the port needs
> to support a subset of U-Boot features.  This would allow boards like
> BCM7260, which have no publicly available Linux DTS, to have a small,
> generic device tree just for configuring reused, GPL'd U-Boot drivers.
> This is in contrast to the policy mandating or encouraging dumping to
> DTS binary-only proprietary Linux DTBs from prior stage bootloaders or
> ROMs, as a precondition to the port being included in U-Boot.
>
> The policy proposal (assuming I'm understanding it correctly now) would
> have been clearer if one of the first two sections in devicetree.rst
> explicitly mentioned "control DTB for U-Boot", i.e., the fact that the
> policy is about U-Boot's own much simpler DTB usage, not Linux's, even
> though the two projects largely share the same DTSs.

OK thanks for your comments. I have updated the patch to mention the
control devicetree explicitly at the top.

Regards,
Simon


>
> Thomas
>
> 1. http://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-sjg@chromium.org/
>
> 2. https://lists.denx.de/pipermail/u-boot/2021-October/463675.html