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

[Daniel Kochmański]

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?
>>>
>>>> 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.
>
>> 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?
>
> Regards,
>
> Hans

Best regards,
Daniel

-- 
Daniel Kochmański | Poznań, Poland
;; aka jackdaniel

"Be the change that you wish to see in the world." - Mahatma Gandhi