On 5/16/23 15:08, Andre Przywara wrote:
This whole memory map is somewhat of a legacy. Apart from a few
addresses for the SPL needs we shouldn't have those defines at all.
Some symbols are needed because there are other macros using them,
although these then are eventually unused.
I have some patches to remove most of the symbols, and patch 14/17
demonstrates some idea how to pin this down to what's really needed.
For this particular case: this was copied from the H6 memory map, some
addresses are just plain wrong for the D1 family. I will try to remove
them as much as possible, leaving only the ones needed in.
I see - the only "tangible" concern I had was the access to
prcm->res_cal_ctrl done in
arch/arm/mach-sunxi/clock_sun50i_h6.c:clock_init_safe
This doesn't appear to upset the silicon but also doesn't seem necessary
either -- and with how tight of a memory footprint SPL has to fit into,
I wanted to check whether this was just something undocumented or dead
code that needed to be removed. It sounds like it's mostly the latter.
So where did you see problems? If you would (wrongly) reference
PortL somewhere in SPL GPIO code, it would use a wrong pointer, but at
least the code would still compile fine, wouldn't it?
The specific patch I had to apply (to arch/arm/mach-sunxi/board.c) was:
/* Update PIO power bias configuration by copy hardware
detected value */
val = readl(SUNXI_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL);
writel(val, SUNXI_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL);
- val = readl(SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL);
- writel(val, SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL);
+ if (SUNXI_R_PIO_BASE) {
+ val = readl(SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL);
+ writel(val, SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL);
+ }
With SUNXI_R_PIO_BASE being 0, this was actually attempting to write to
BROM. This might also be something that doesn't really upset the
silicon, though: my debug environment is a concolic emulator I quickly
hacked up to trace MMIO accesses, and it flagged the write to BROM as an
error. It was easier to patch the SPL than to have the emulator ignore
the error (and verify that the T113 was cool with it).
Since this kind of extraneous/erroneous init code tends to remain
undetected when the symbols they need are dummied-out like this, I
figured I'd give a nudge in the direction of instead *removing* the
symbols where appropriate and fixing whatever breaks -- especially since
we really need to be thrifty about SPL size. But that might also be
something that happens in a later cleanup pass when the patchset is
being prepared for upstream inclusion. :)
P.S. Could you try the github post? Then compiled and booted fine for
me, and includes the DRAM code as well now:
https://github.com/apritzel/u-boot/commits/t113s-mq-r-WIP
Ooh, more up-to-date code, thanks for the link! I'll switch to using
this instead going forward. My pulls from that branch might be
relatively infrequent since I'm also working on some patches for better
Clang compatibility concurrent with the efforts here. Is this email
thread a good venue for feedback against that branch or would you prefer
that I use GitHub issues instead?
Warm regards,
Sam
P.S. My target is the BMC on the Turing Pi 2 board. They have the same
SoC and (apparently) UART console configuration, but the differences end
there: in particular, my target supports boot from either/both
microSD+SPI-NAND. I might have to start pushing for room for SPI drivers
in the SPL soon. :)
