[U-Boot] Proposal to add NAND-boot support for Sunxi SPL

[Hans de Goede]

Hi,

On 25-05-15 11:29, Daniel Kochmański wrote:
> Hello,
>
> sorry for delay. Comments inline.
>
> Hans de Goede writes:
>
>> Hi Roy,
>>
>> It seems you've accidentally dropped the mailinglist
>> from the Cc (I don't see anything private in here),
>> so I've readded it.
>>
>> On 24-05-15 15:30, Roy Spliet wrote:
>>> Hello Hans,
>>>
>>> Comments inline.
>>>
>>> Op 22-05-15 om 15:51 schreef Hans de Goede:
>>>> Hello Roy,
>>>>
>>>> On 22-05-15 12:12, Roy Spliet wrote:
>>>>> Hello Hans,
>>>>>
>>>>> Sorry for ignoring the second half of your question so far. Here's what's on my mind.
>>>>> Op 21-05-15 om 20:08 schreef Hans de Goede:
>>>>>> Hi Roy,
>>>>>>
>>>>>> 2) What is the plan to add support for loading files from nand in u-boot proper,
>>>>>> so that we can get (e.g.) extlinux.conf + kernel +dtb from a /boot on nand ?
>>>>>
>>>>> For the full U-boot I agree we want both MMC and NAND support, regardless of where it was loaded from. From what I can tell U-boot already has UBI support. It sounds like a logical step to try and construct a proper NAND driver for U-boot that either co-exists with this SPL driver or, even better, shares code. That way, I only assume that the UBI and UBIFS layers will take care of all the rest.
>>>>
>>>> Ack.
>>>>
>>>>> The NAND framework in u-boot resembles Linux in many ways. I'm currently in doubt whether we should take Boris' driver as a starting point, or rather use something heavily reduced that re-uses this SPL code. Either way, in U-boot we can perform a clean NAND-chip detection, preferably based on DT definitions as we also use on Linux, and take care of everything proper like PLL settings and a bunch of parameters which are now hard-coded or a configuration option in sunxi-common.h.
>>>>
>>>> Have you seen Yassin Jaffer's work porting Boris' code to u-boot ?
>>>>
>>>> https://github.com/yassinjaffer/u-boot/commits/sunxi-nand
>>>>
>>>> Last time I mailed with Yassin (added to the Cc) he was ok with someone
>>>> else picking this up and continuing with it as the does not have time
>>>> to work on it.
>>> I have seen the pointer to it, but I have the "nasty" habit of preferring to look at simple work rather than complex. Hence I ended up with leveraging Daniel's patches. Considering space limitations, perhaps it would be best if this SPL driver co-exists with a full NAND driver.
>>
>> Yes I think that given the space limitations that will be the way to go.
>>
>>> Even if that means sacrificing code-sharing...?
>>
>> Yes, as much as I dislike that I think dragging the entire mtd work into
>> the SPL simply is not going to fly.
>>
>>> I might have to get back to you on this once I understand the level of code sharing between the NAND framework for SPL and for U-boot.
>>
>> Ok.
>>
>>> If I were to pick up from Yassin's tree: are there strong reasons why this work hasn't been merged already?
>>
>> Not that I can remember, the main problem was lack of time from both the
>> reviewer and submitter side IIRC. ATM I'm quite interested in getting
>> nand working, so the reviewer side should be covered and I've the
>> feeling the same goes for the submitter side, so we should be able
>> to make good progress here.
>>
>>>>
>>>>> SPL is a different story. I don't know the exact size restriction, but for A10 I've heard it might be as little as 30KB.
>>>>> Current SPL with my patches and without MMC is already 23KiB.
>>>>
>>>> The BROM loads the SPL to a 32K sram and the stack sits in that same SRAM. Note
>>>> I've some patches which switch the SPL from using a fill blown malloc to using
>>>> simple-malloc.c which saves a significant amount of space, and the mmc code
>>>> is not really that big, so I think we should be able to cram this all into
>>>> the SPL.
>>> That sounds like a good plan, it's good enough for SPL.
>>>>
>>>>> I personally think we can reduce it slightly by taking out support for reading everything other than the bootloader partition from SPL (so remove non-syndrome mode, remove the random seeds table...),
>>>>
>>>> I agree that removing (#if 0 it for now?) non boot partition support makes
>>>> sense as a space saving measure.
>
> That would disable possibility of preloading multiimage with packed boot
> script, kernel and devicetree, right? I think code which adds support
> for all partitions isn't that big. Or maybe rather loading anything else
> then u-boot from u-boot is undesirable?

I guess you mean "loading anything else then u-boot from the spl is undesirable?"

So what you're talking about using a multi-image directly from the SPL, is
basically what is falcon mode, and given the restrictions we've in the SPL
I do not think that supporting something like that for nand is a good idea,
in order to access the non boot partitions we need to know some more
nand parameters, and as said I would really like to avoid coding things
like ecc size into Kconfig since different production runs of the same board
(e.g. the famous mk802 boards) may very well use different nand chips,
and I do not want users to need know which production run they exactly have ...

This means we need to get info like this from the nand id, and I'm not sure
that that is doable from the spl.

So for starters I suggest we simply #if 0 the code, with a comment that this
maybe useful for falcon mode, and then it the future we may replace the
#if 0 with a #if falcon and enable this on select boards ?





>>>>
>>>>> but it certainly doesn't leave any room for the full NAND framework to do ID-based NAND chip detection.
>>>>
>>>> What info do we need when we're only reading ? If the BROM can get away with a fixed
>>>> way of reading the nand for booting, we should be able to make the SPL get
>>>> away with it too ...  I do really believe that we should be able to deal
>>>> with different nand chips from a single binaries, with cheap chinese
>>>> devices like the mk802 the chances are simply to big that the nand
>>>> will differ from one revision to the next.
>>> If we look solely at functionality I agree. It doesn't even make sense giving SPL broader support than BROM. The technical half of me is complaining about their approach though, because as it stands it does not allow booting from <4GB SLC NAND chips. The parameters they try simply don't match what these chips have to offer in terms of page size and OOB area.
>>
>> I do not think that support things which the BROM does not support makes
>> sense for the FEL code. OTOH no need to cripple it explicitly if the
>> same code can support more.
>>
>> For the full u-boot.bin nand driver supporting more then the BROM does
>> is fine.
>>
>>> Maybe I should try and add an ID read function to at least obtain the page size and access method, this doesn't have to be a lot of code...
>>
>> I've been looking into this on the kernel side so as to get ecc strength / size
>> from the id rather then having to extend the in kernel fixed id table as was done
>> for the cubietruck. I've this working now for samsung nands, but it is not simple
>> as it differs per vendor and generation of nands, You're free to try and solve
>> this anyway you want but getting info from the id may be harder then you think.
>>
>>> I don't think it's wise to re-use the code in the full NAND framework though because it's simply too elaborate and integrated. We don't care about things like vendor strings in SPL, right?
>>
>> Right, strings are way too expensive for in the SPL.
>>
>>> Are there SPL helper functions to parse a chip ID into some of its parameters (page size, oob size, preferred ECC strength...)?
>>
>> I'm not familiar with the u-boot (spl) nand code, maybe someone on the list
>> reading along knows, if not try searching for them in include/* ?
>>
>>>>> I personally think it's acceptable if NAND-SPL does not have MMC support and vice-versa.
>>>>
>>>> Distros already need to build and distribute a u-boot-with-spl.bin per supported
>>>> board. This doubles the number of builds they have to do and the number of
>>>> files they need to distribute. If at all possible I would really like
>>>> to have a unified SPL binary.
>>> Granted. In this case I take it we should try and prioritise MMC0 over NAND always, matching BROM A10/A20s decisions. Agreed?
>>
>> Agreed.
>
> Unified binary would be great. I'll check this week if I can boot spl
> with MMC support (as I mentioned, it was hanging due to some hairy
> initialization error) and try to fix this.

Ack.

>>
>>> Regarding MMC2... I'm not sure if we want a binary that supports both MMC2 and NAND given they share a set of pins.
>>
>> Right some configs will automatically detect whether we are booting from MMC0 or NAND,
>> and others if we're booting from MMC0 or MMC2, I was just pointing to the existing
>> MMC0 / MMC2 decision code as an example of how this can be done.
>>
>>> I don't know how well switching between the two is going to play out.
>>
>> No need to worry about that, that is not something which we want to support.
>>
>>> I'll see if I can free up some time to do this in a follow-up patch, although I hope it's not a blocker for the patches I sent to the ML last Friday.
>>
>> As I already indicated in reply to your v1 series posting this is not a blocker.
>>
>> BTW why is v2 an RFC and not simply a v2 of the series?

About this, I've just read the cover letters to what is exactly v3 of the series,
and it is in there, so Roy, no need to answer this one.

Regards,

Hans