Am 05.12.2018 um 14:54 schrieb Simon Glass:
Hi Simon,

On Wed, 5 Dec 2018 at 06:38, Simon Goldschmidt
<simon.k.r.goldschm...@gmail.com> wrote:

On Wed, Dec 5, 2018 at 2:21 PM Simon Glass <s...@chromium.org> wrote:

Hi Simon,

On Sun, 25 Nov 2018 at 23:05, Simon Goldschmidt
<simon.k.r.goldschm...@gmail.com> wrote:

[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 <s...@chromium.org> wrote:

Hi Simon,

On Sun, 25 Nov 2018 at 14:09, Simon Goldschmidt
<simon.k.r.goldschm...@gmail.com> wrote:

On Thu, Nov 22, 2018 at 9:50 PM Simon Glass <s...@chromium.org> wrote:

Hi,

On Thu, 22 Nov 2018 at 10:02, Tom Rini <tr...@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 <tr...@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 <
s...@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 can add an SPL size check in Makefile.spl if you like.

That might be required, yes.

- you get an inconsistent memory layout regarding read/write: the
linker places bss at the end but then, DTB follows as const data

This should be handled by the $(SPL-BIN)-pad.bin file (or by binman if
you are using that).

I don't understand that. How does the padding help? I have these
sections (roughly):
- text: readonly
- bss: writable
- DTB: readonly, added as post build step after linking

How does $(SPL-BIN)-pad.bin help?

It covers over the BSS section so that the image ends where the DTB
starts, thus fixing the addressing issue you mentioned. It allows you
to do this:

cat u-boot-spl-nodtb.bin u-boot-spl.dtn >-u-boot-spl.bin


- 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)

Right, but this is a few bytes. Why does it matter?

- "spl/u-boot-spl.hex" created by the default Makefiles does not seem
to include the DTB

That might just be a bug.

It might, yes. The hex file is currently built from the elf file, so
there's no DTB in there.

OK. Could be worth a patch.



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.)

How about calculating that checksum at build time instead? You could
use binman to do that.


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?

Yes, do it at build time. Or calculate your CRC before you write any
BSS variables.

Creating the correct checksum is not the point. I can do that before using bss.

The problem is that on the next boot, this checksum is not valid any
more because bss might have changed.

Right, but just skip the BSS section when checksumming - i.e. checksum
the code and then the DT but omit the BSS.

Yeah, well, good idea but that's not possible. The boot rom checks this checksum on warm restart and it can only check the CRC of one block. And of course I canno change the boot rom.

So the only option I have without using OF_EMBED is to drop the CRC of the DT, which is not really an option...

Regards,
Simon

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.

That is very bad news. I'll see about adding a Makefile warning.

OK. Looking forward to the discussion that starts then :-)

Yes...

Regards,
Simon


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to