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

Dear Marek,
The following changes since commit 40521a6c90d4adfb7f3041033c8cbbeff087a5ca:
Merge https://gitlab.denx.de/u-boot/custodians/u-boot-fsl-qoriq (2020-01-25 12:20:51 -0500)
are available in the Git repository at:
git@gitlab.denx.de:u-boot/custodians/u-boot-dfu.git
for you to fetch changes up to d34375267f521dc76b1875a905cec914a47b0712:
dfu: Add option to skip empty pages when flashing UBI images to NAND (2020-01-26 11:44:15 +0100)
---------------------------------------------------------------- Guillermo Rodríguez (1): dfu: Add option to skip empty pages when flashing UBI images to NAND
drivers/dfu/Kconfig | 7 +++++++ drivers/dfu/dfu_nand.c | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-)
BRANCH: master https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/commits/master
Merge tag info: - Add option to DFU NAND to skip empty pages when flashing UBI images
Travis-CI: https://travis-ci.org/lmajewski/u-boot-dfu/builds/618507384
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

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.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Marek,
El lun., 27 ene. 2020 a las 8:52, Lukasz Majewski (lukma@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. 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
Best regards,
Guillermo Rodriguez Garcia guille.rodriguez@gmail.com

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@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.
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.
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 ?

Hi,
El lunes, 27 de enero de 2020, Marek Vasut marex@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@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.
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.
BR,
Guillermo

On 1/27/20 2:26 PM, Guillermo Rodriguez wrote:
Hi,
Hi,
El lunes, 27 de enero de 2020, Marek Vasut marex@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@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.
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 ?

El lunes, 27 de enero de 2020, Marek Vasut marex@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@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@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.
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?
Guillermo

On 1/27/20 2:35 PM, Guillermo Rodriguez wrote:
El lunes, 27 de enero de 2020, Marek Vasut marex@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@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@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.
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 ?

El lun., 27 ene. 2020 a las 14:51, Marek Vasut (marex@denx.de) escribió:
On 1/27/20 2:35 PM, Guillermo Rodriguez wrote:
El lunes, 27 de enero de 2020, Marek Vasut marex@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@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@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.
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

On Mon, Jan 27, 2020 at 04:00:28PM +0100, Guillermo Rodriguez wrote:
El lun., 27 ene. 2020 a las 14:51, Marek Vasut (marex@denx.de) escribió:
On 1/27/20 2:35 PM, Guillermo Rodriguez wrote:
El lunes, 27 de enero de 2020, Marek Vasut marex@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@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@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:
- 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
Let me know if you have any other questions.
As I've just been reading along, thanks for the detailed summary of the issues!

On 1/27/20 4:00 PM, Guillermo Rodriguez wrote:
El lun., 27 ene. 2020 a las 14:51, Marek Vasut (marex@denx.de) escribió:
On 1/27/20 2:35 PM, Guillermo Rodriguez wrote:
El lunes, 27 de enero de 2020, Marek Vasut marex@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@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@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:
- 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
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.

On 1/27/20 8:52 AM, Lukasz Majewski wrote:
Hi Marek,
Hi,
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.
Which doesn't really answer my question, it only shows that another piece of code suffers from the same problem.
participants (5)
-
Guillermo Rodriguez
-
Guillermo Rodriguez Garcia
-
Lukasz Majewski
-
Marek Vasut
-
Tom Rini