[U-Boot] [PATCH 66/93] arm: Remove ot1200 board

[Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>]

[I've cut down the CC list a bit due to some gmail warnings]
On Mon, Nov 26, 2018 at 4:00 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Simon,
>
> On Sun, 25 Nov 2018 at 14:09, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > On Thu, Nov 22, 2018 at 9:50 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 22 Nov 2018 at 10:02, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Nov 22, 2018 at 03:44:28PM +0100, Simon Goldschmidt wrote:
> > > > > Am Do., 22. Nov. 2018, 14:44 hat Tom Rini <trini@konsulko.com> geschrieben:
> > > > >
> > > > > > On Thu, Nov 22, 2018 at 02:24:49PM +0100, Marek Vasut wrote:
> > > > > > > On 11/22/2018 01:52 PM, Tom Rini wrote:
> > > > > > > > On Thu, Nov 22, 2018 at 10:25:14AM +0100, Christian Gmeiner wrote:
> > > > > > > >
> > > > > > > >> Am Mo., 19. Nov. 2018 um 16:56 Uhr schrieb Simon Glass <
> > > > > > sjg at chromium.org>:
> > > > > > > >>>
> > > > > > > >>> This board has not been converted to CONFIG_DM_BLK by the deadline.
> > > > > > > >>> Remove it.
> > > > > > > >>>
> > > > > > > >>
> > > > > > > >> As the board is still mainted I will NAK it for the moment. Are there
> > > > > > > >> any hints want needs to be done
> > > > > > > >> to port thie board over to new DM stuff?
> > > > > > > >
> > > > > > > > Yes, as a start you need to switch over to using CONFIG_OF_CONTROL and
> > > > > > > > selecting/providing a dtb file.  I see ot1200 is using DWC_AHSATA which
> > > > > > > > needs more work, but this is the board-level work that needs doing.
> > > > > > >
> > > > > > > Wasn't there a possibility to use platform data in board file instead of
> > > > > > > DT ? Or is DT mandatory now , including the libfdt overhead ?
> > > > > >
> > > > > > In short, DT for U-Boot and platform data for SPL is what's recommended,
> > > > > > yes.
> > > > > >
> > > > >
> > > > > This is a little confusing for me. Socfpga gen5 SPL doesn't do that. And it
> > > > > seems a little strange or outdated overall.
> > > > >
> > > > > Would there be some kind of reference architecture or mach to look at
> > > > > what's the suggested/up-to-date way to implement SPL? Also regarding code
> > > > > flow?
> > > >
> > > > So, SPL is where things get, ahem, fuzzy.  While I don't want to
> > > > encourage boundless growth in U-Boot proper, we aren't exactly size
> > > > constrained (but rather, functional/logical constrained).  But in SPL,
> > > > yes, we have many platforms with 32/64/128 kilobyte hard limits (and
> > > > some smaller) and we can't always shove in a "TPL" before SPL either.
> > > > So in SPL we do make use of platform data instead.  While not the
> > > > smallest size constraint, am335x_hs_evm is a reasonable thing to look at
> > > > in this case.
> > >
> > > Also 'rock' uses CONFIG_OF_PLATDATA which provides a halfway house -
> > > still uses DT, but it gets converted into C structs so saves code
> > > space.
> > >
> > > firefly-rk3288 is a pretty good DM/DT example, including SPL.
> >
> > I've currently got an issue on socfpga gen5 that could be solved best
> > by enabling CONFIG_OF_EMBED (mixing const and non-const sections is a
> > problem for CRC calculation). However, it could probably also solve by
> > using platform data (but that doesn't work out of the box, yet). The
> > problem with CONFIG_OF_EMBED is that I think it's OK to enable this
> > for SPL but I don't like enabling it for U-Boot, so:
> >
> > Would it make sense to duplicate the whole "Provider of DTB for OF
> > control" choice so that it can be OF_EMBED for SPL but different for
> > U-Boot? Or does it make more sense to convert socfpga gen5 to use
> > OF_PLATDATA?
>
> We should not be using OF_EMBED in in-tree boards or production code.

What's the reason for this? I can understand this for U-Boot, and I
can understand that it's at least theoretically a bit cleaner for SPL,
too. But there are some drawbacks when doing this in SPL where code is
not relocated:
- you lose the ability to check total size in linker file (which is
bad for size-constrained platforms: sometimes you notice failure only
when booting)
- you get an inconsistent memory layout regarding read/write: the
linker places bss at the end but then, DTB follows as const data
- binary size "on disk" grows due to this inconsistent memory layout
(since the flat binary includes the DTB, it needs to include the
zeroed-out bss, too)
- "spl/u-boot-spl.hex" created by the default Makefiles does not seem
to include the DTB

> Can you please explain the issue a bit more?

Of course: socfpga gen5 has a feature where the boot rom can jump to
SPL in SRAM on warm boot. To ensure SPL is still valid after a reboot,
the boot rom can check its consistency by calculating a CRC over one
specified range in SRAM. On first boot, SPL stores its start, length
and CRC value to special registers for the boot rom. Since the
contents of bss changes while SPL is running, bss cannot be included
in this CRC range. (Same goes for the '.data' region, but it's
possible to build SPL without actually using it.)

So to ensure the DTB is untouched, I have to make sure it has a lower
address than the bss section. Using OF_EMBED does this for me. And I
expect using platform data would work too. Do you have another idea
how to achieve my goal of combining all write-only sections in SPL
into one block?

Oh, and I currently count 109 defconfig files containing "OF_EMBED",
so I wasn't aware that this should not be used. Maybe these platforms
have similar reasons like I have and would enable OF_EMBED only for
SPL if they could. At least for socfpga_stratix10 that should work.

Regards,
Simon