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

[Guillermo Rodriguez]

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.

Thanks,

Guillermo