[GIT] Pull request: u-boot-dfu (26.01.2020)

[Marek Vasut]

On 1/27/20 4:00 PM, Guillermo Rodriguez wrote:
> El lun., 27 ene. 2020 a las 14:51, Marek Vasut (<marex at denx.de>) escribió:
>>
>> On 1/27/20 2:35 PM, Guillermo Rodriguez wrote:
>>> El lunes, 27 de enero de 2020, Marek Vasut <marex at denx.de> escribió:
>>>
>>>> On 1/27/20 2:26 PM, Guillermo Rodriguez wrote:
>>>>> Hi,
>>>>
>>>> Hi,
>>>>
>>>>> El lunes, 27 de enero de 2020, Marek Vasut <marex at denx.de> escribió:
>>>>>
>>>>>> On 1/27/20 9:14 AM, Guillermo Rodriguez Garcia wrote:
>>>>>>> Hi Marek,
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>> El lun., 27 ene. 2020 a las 8:52, Lukasz Majewski (<lukma at denx.de>)
>>>>>> escribió:
>>>>>>>>
>>>>>>>> Hi Marek,
>>>>>>>>
>>>>>>>>> On 1/26/20 9:26 PM, Lukasz Majewski wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>> ----------------------------------------------------------------
>>>>>>>>>> Guillermo Rodríguez (1):
>>>>>>>>>>       dfu: Add option to skip empty pages when flashing UBI images
>>>>>>>>>> to
>>>>>>>>>
>>>>>>>>> Can that option be enabled/disabled at runtime instead of being
>>>>>>>>> hardcoded?
>>>>>>>>
>>>>>>>> It has been designed in a similar way to Android's existing
>>>>>>>> FASTBOOT_FLASH_NAND_TRIMFFS option.
>>>>>>>
>>>>>>> Without this option, UBI images need to be built with --space-fixup
>>>>>>> [1] so that the kernel can "fix" the NAND on first mount.
>>>>>>> When this option is used, --space-fixup is no longer necessary because
>>>>>>> dfu knows how to correctly flash UBI images. However, UBI images built
>>>>>>> with --space-fixup will still work fine.
>>>>>>
>>>>>> Does NAND.TRIMFFS preserve UBI erase counters in the NAND ? I don't
>>>>>> think so, so I don't think "correctly flash UBI images" is the correct
>>>>>> formulation here.
>>>>>>
>>>>>> Yes, you are right.
>>>>>
>>>>>
>>>>>>> In other words, enabling this option at buildtime has no
>>>> countereffects.
>>>>>>> So there is no point in making it configurable at runtime, if support
>>>>>>> has been built into U-boot.
>>>>>>>
>>>>>>>  [1]: http://www.linux-mtd.infradead.org/faq/ubifs.html#
>>>>>> L_free_space_fixup
>>>>>>
>>>>>> So what if I want to write raw NAND image without "trimffs" on such a
>>>>>> system via DFU, e.g. a bootloader ? How can I do that ?
>>>>>>
>>>>>
>>>>> You mean on a non-ubi partition? Then you don't need to do anything
>>>>> special. This code is only triggered for ubi partitions.
>>>>
>>>> And what about partitions which were already built with the --space-fixup ?
>>>>
>>>
>>> What do you mean with "partitions built with --space-fixup"? If you mean
>>> ubi *images* built with --space-fixup, these will still work fine.
>>> Otherwise, could you please clarify?
>>
>> I'm just curious whether we're changing behavior here, which might pose
>> a problem. I am also not super fond of hard-coding things this way, but
>> maybe this is fine in this case ?
> 
> OK, let me try to summarize what we're doing here.
> 
> The recommended way to flash UBI images is using ubiformat [1].
> When this is not possible, then the UBI FAQ describes how UBI images
> should be flashed to NAND [2]. Basically you need to make sure that
> any trailing all-0xFF pages in each eraseblock are dropped (i.e. not
> written to flash). This is what "nand write.trimffs" does (see
> description in [3])
> If you cannot do that, then an alternative is to make sure that the
> UBI image is built with --space-fixup [4]; this sets a flag so that
> the Linux kernel will "fix" the NAND on first mount. This is somewhat
> expensive, though, as the kernel may need to rewrite several PEBs.
> 
> In the current u-boot, dfu does not follow the procedure stated in the
> UBI FAQ. This forces you to make sure UBI images are always built with
> --space-fixup.
> With this patch, when dfu writes to a UBI partition it will use the
> procedure outlined in the UBI FAQ. So --space-fixup is no longer
> required.
> 
> Two important things to note here:
> - This only makes a difference for UBI partitions (see check in
> dfu_nand [5]). Behaviour for non-UBI partitions does not change.
> - Images built with --space-fixup will continue to work just fine (the
> kernel will do just its stuff the first time the file system is
> mounted. This is unnecessary, but also harmless).
> 
> So, to summarize:
>  1. Without the patch dfu only works if --space-fixup was used when
> the UBI image was generated; with the patch it will work for images
> with and without --space-fixup
>  2. Nothing changes for non-UBI partitions
> 
>  [1]: http://www.linux-mtd.infradead.org/faq/ubifs.html#L_why_ubiformat
>  [2]: http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
>  [3]: https://gitlab.com/u-boot/u-boot/blob/master/doc/README.nand#L65
>  [4]: http://www.linux-mtd.infradead.org/faq/ubifs.html#L_free_space_fixup
>  [5]: https://gitlab.com/u-boot/u-boot/blob/master/drivers/dfu/dfu_nand.c#L231
> 
> Let me know if you have any other questions.

I think it's OK. I just wanted to be sure we're not breaking anything
there. Thanks!

Applied.