Hi Ahmad,
On Mon, May 16, 2022 at 01:15:42PM +0200, Ahmad Fatoum wrote:
> Hello Sam,
Thanks for your feedback - very appreciated!
>
> On 15.05.22 21:38, Sam Ravnborg wrote:
> > This updates skov-arm9cpu with xload support, and we can now
> > use barebox as a replacment for at91bootstrap.
> >
> > Only boot via SD card is supported.
> >
> > NOTE: Actual status
> > [x] dbgu support in pbl works (can print)
> > [x] Other init stuff ifdeffed out - from at91bootstrap
> > [ ] Check what the original code used for div/mul - there is some confusion
> > [ ] load barebox.bin and boots it. Right now mount fails
>
> Is your SD-Card perhaps 2G or smaller? The AT91 PBL MCI functions
> assume high capacity (> 2G). It's a quite ugly thing, but
> finding out whether a card is High-Capacity or not happens
> during init phase and as we don't redo init in PBL...
>From the at91sam9263 datasheet:
"Boot ROM does not support high capacity SDCards."
Sounds like a very plausible explanation - and gives me something to go
after.
> High Capacity cards reference start block offset in sectors, while
> older cards use bytes. On i.MX, barebox just reads at offset 0
> and all is good, but on AT91, we need to do random access, so
> we need to decide whether to use sectors or bytes. Currently,
> the driver hardcodes the sector assumption. I found this to be
> the lesser evil to the alternative: having a full MMC stack in PBL.
>
> If that's indeed your issue, there's a heuristic possible:
> Try to mount for High-Capacity, if that fails, assume non-high
> capacity and try again. It's not 100%, but it's better than status quo.
I will try to find a nice way to tell that we shall go for a non-high
capacity in the first place.
And then I will dig into the sector versus byte thing afterwards.
>
> > -static void __bare_init skov_arm9cpu_init(void *fdt)
> > +ENTRY_FUNCTION(start_skov_arm9cpu_xload_mmc, r0, r1, r2)
> > {
> > - struct at91sam926x_board_cfg cfg;
> > + const struct sam92_pmc_config sam92_pmc_config = {
> > + /* X-tal is 16.000 MHz so 16 / 4 * (31 + 1) = 200 */
> > + .diva = 14,
> > + .mula = 171,
> > + };
> > + sam9263_lowlevel_init(&sam92_pmc_config);
>
> This is needlessly fragile. Compiler is within rights to never push
> this to stack and to regenerate a relocation entry here that points
> at .data, which has not yet been relocated.
>
> > + arm_setup_stack(AT91SAM9263_SRAM0_BASE + AT91SAM9263_SRAM0_SIZE);
>
> Both sam9263_lowlevel_init and arm_cpu_lowlevel_init are global functions,
> so LR will be pushed to stack in-between, yet stack is only initialized here
> after.
>
> Also ENTRY_FUNCTION is __naked on ARM32, so it's a bad idea to do more
> than the absolutely necessary stuff here as GCC can generate very unexpected
> code when it decides to spill to stack inside a naked function.
>
> We have ENTRY_FUNCTION_WITHSTACK now that removes this footgun.
> Please use that instead.
>
> >
> > - cfg.pio = IOMEM(AT91SAM9263_BASE_PIOD);
> > - cfg.sdramc = IOMEM(AT91SAM9263_BASE_SDRAMC0);
> > - cfg.ebi_pio_is_peripha = true;
> > - cfg.matrix_csa = IOMEM(AT91SAM9263_BASE_MATRIX + AT91SAM9263_MATRIX_EBI0CSA);
> > + relocate_to_current_adr();
> > + setup_c();
>
> My preference would be set up ENTRY_FUNCTION_WITHSTACK, so you don't have
> to write naked code. Call arm_cpu_lowlevel_init() first thing, so you have
> active I-Cache. Then relocate and setup_c and only then do the SoC-specific setup.
Thanks - again very useful feedback. I will go along these lines.
>
> > +pblb-$(CONFIG_MACH_SKOV_ARM9CPU) += start_skov_arm9cpu_xload_mmc
> > +FILE_barebox-skov-arm9cpu-xload-mmc.img = start_skov_arm9cpu_xload_mmc.pblb
> > +MAX_PBL_MEMORY_SIZE_start_skov_arm9cpu = 0x12000
> > +image-$(CONFIG_MACH_SKOV_ARM9CPU) += barebox-skov-arm9cpu-xload-mmc.img
> > +
> > pblb-$(CONFIG_MACH_SKOV_ARM9CPU) += start_skov_arm9cpu
> > FILE_barebox-skov-arm9cpu.img = start_skov_arm9cpu.pblb
> > MAX_PBL_MEMORY_SIZE_start_skov_arm9cpu = 0x12000
>
> Unrelated to your patch, but you might know the answer: Why is there a max PBL memory size here?
> AFAIK, you use at91bootstrap to chainload barebox into DRAM, why do you need
> a PBL size limit then?
The limit is from the old days where we tried to squeeze barebox into
the SRAM, so it could be used as the only bootloader - dropping the need
for at91bootstrap.
The new approach where we do a PBL barebox and then a full barebox is
much easier when you have first understood the concept.
I will drop the size restriction in my next patch stack.
We see other at91sam boards with the same restrictions due to the same
history. I think we can safely assume there is no use for barebox as
at91bootstrap and we can drop the size restrictions.
But then I am not happy to edit old boards that I cannot tests.
I have an at91sam9263-ek board and will update the board support
when I have skov-arm9cpu done. Focus is on the skov board first, as I
have more HW to play with here.
Sam
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
