[U-Boot] [PATCH] SPL: NOR: Add CONFIG_SPL_NOR_COPY_ENTIRE_IMAGE define to enable whole image copy from NOR

[Lukasz Majewski]

Hi Marek,

> On 12/29/2016 04:26 PM, Tom Rini wrote:
> > On Thu, Dec 29, 2016 at 12:41:06AM +0100, Marek Vasut wrote:
> >> On 12/28/2016 09:52 AM, Lukasz Majewski wrote:
> >>> Hi Marek,
> >>>
> >>>> On 12/26/2016 05:36 PM, Lukasz Majewski wrote:
> >>>>> Hi Marek,
> >>>>>
> >>>>>> On 11/29/2016 07:18 PM, Tom Rini wrote:
> >>>>>>> On Tue, Nov 29, 2016 at 11:50:34AM +0100, Marek Vasut wrote:
> >>>>>>>> On 11/29/2016 10:11 AM, Lukasz Majewski wrote:
> >>>>>>>>> Hi Marek,
> >>>>>>>>>
> >>>>>>>>>> On 11/28/2016 10:09 PM, Lukasz Majewski wrote:
> >>>>>>>>>>> This define gives the possibility to copy entire image
> >>>>>>>>>>> (including header - e.g. u-boot.img) from NOR parallel
> >>>>>>>>>>> memory to e.g. SDRAM. The current code only supports
> >>>>>>>>>>> loading the raw binary image (the u-boot.bin).
> >>>>>>>>>>>
> >>>>>>>>>>> The legacy behavior is preserved, since other board don't
> >>>>>>>>>>> enabled this option.
> >>>>>>>>>>
> >>>>>>>>>> Sooooo, what's the usecase again ? ;-) 
> >>>>>>>>>
> >>>>>>>>> :-)
> >>>>>>>>>
> >>>>>>>>> The use case is to allow u-boot.img being loaded from
> >>>>>>>>> Parallel NOR. The current code only supports u-boot.bin.
> >>>>>>>>
> >>>>>>>> Why is u-boot.bin (or the payload) not sufficient ? Why do
> >>>>>>>> you need the header ?
> >>>>>>>
> >>>>>>> Well, the general use-case and code flow is that we load
> >>>>>>> u-boot.img (or a FIT image) and if all else fails, fall back
> >>>>>>> to assuming a .bin and a known address).
> >>>>>>>
> >>>>>> And exactly how is that whole image useful in RAM ? Sorry, I
> >>>>>> still do not see it, usually you just need the executable
> >>>>>> payload, although even that can be left in flash most of the
> >>>>>> time.
> >>>>>
> >>>>> The use case is that I do want to boot from SD card/eMMC and NOR
> >>>>> with using u-boot.img.
> >>>>>
> >>>>> I would like to avoid situation when for NOR I must use
> >>>>> u-boot.bin and for eMMC u-boot.img.
> >>>>>
> >>>>> Such approach keeps things as simple as possible :-)
> >>>>
> >>>> Oh, so it allows you to detect bitrot for the content in SPI
> >>>> NOR ?
> >>>
> >>> I do not use SPI NOR, it is parallel NOR.
> >>
> >> Sorry, I meant parallel NOR of course.
> >>
> >>>> It's a bit strange we had to use u-boot.bin with SPL there.
> >>>>
> >>>
> >>> This is how the legacy system behaves. It uses (by default)
> >>> Parallel NOR for booting (with advised/provided NOR memory
> >>> timings). After doing some measurements, it turned out that for
> >>> "tunned" u-boot/SPL there would be the best way to copy it to ram
> >>> and execute it from there (just like eMMC).
> >>>
> >>> Hence, I would like to use u-boot.img in both booting scenarios.
> >>
> >> I think I was mistaken yesterday, I don't think I understand why
> >> copying the image including the header into RAM has any benefit
> >> compared to copying just the image payload to RAM (and yes, we're
> >> getting back to my original question).
> > 
> > Code complexity and forward compatibility?
> 
> This is adding code complexity, but this is not my point.
> 
> > The general case in the SPL
> > framework is that we have either a "legacy" image or a FIT image
> > and we fall back to "well, just run it!".
> 
> Well, this doesn't answer my question, because if I understand this
> patch correctly, it copies the entire legacy image (incl. header) into
> RAM instead of copying just the image payload (which we already do). I
> don't really understand why we want to do this. Or do I misunderstand
> something ?

No, you understood everything correctly. After some thoughts, I think
that only payload should be copied.

I'll prepare test setup and double check this patch usability by Monday.

Best regards,
Łukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161230/8d811192/attachment.sig>