Re: [PATCH v1 8/8] ARM: at91: Add xload support to skov-arm9cpu

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Sam,

On 16.05.22 17:28, Sam Ravnborg wrote:
> Hi Ahmad,
> 
> On Mon, May 16, 2022 at 01:15:42PM +0200, Ahmad Fatoum wrote:
>> Hello Sam,
> 
> Thanks for your feedback - very appreciated!

:)

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

Try toggling priv->highcapacity_card = 1; in atmel_mci_pbl.c

The original in atmel-sdhci-pbl.c had a comment:

 // FIXME can we determine this without leaving SD transfer mode? 
 priv->highcapacity_card = 1;                                  

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

It's probably best we move this setting into the _bio_init functions,
so callers are aware of the limitation and can e.g. for the 9263
always hardcode it as false. Heuristic would be nice to have,
but apparently it's not required for your use case.

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

Ye, doing the change for your board only is fine by me. I just
wondered when I looked at the code. Thanks for clearing this up.

Cheers,
Ahmad

> 
> 	Sam
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox