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

[Marek Vasut]

On 12/30/2016 10:28 PM, Lukasz Majewski wrote:
> 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.

But that's what we already do, no ? So what is the point of this patch ?

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

OK

-- 
Best regards,
Marek Vasut