[PATCH v6 0/6] FWU: Add support for mtd backed feature on DeveloperBox

[Michal Simek]

Hi Jassi,

On 5/31/23 07:28, jaswinder.singh at linaro.org wrote:
> From: Jassi Brar <jaswinder.singh at linaro.org>
> 
> Introduce support for mtd backed storage for FWU feature and enable it on
> Synquacer platform based DeveloperBox.
> 
> This revision is rebased onto patchset that trims the FWU api
>   https://lore.kernel.org/u-boot/20230306231747.1888513-1-jassisinghbrar@gmail.com/
> 
> Changes since v5:
> 		* Some of the typo fixes and cosmetic changes suggested by Etienne
> 
> Changes since v4:
>          * Provide default/weak implementations of fwu_plat_get_alt_num and fwu_plat_get_bootidx
>          * Provide man page for mkfwumdata
>          * Misc typo fixes and cosmetic changes
> 
> Changes since v3:
>          * Fix and Update documentation to also build optee for FWU FIP image.
>          * Fixed checkpatch warnings
>          * Made local functions static.
>          * Split config changes to a separate patch
>          * Fix authorship of three patches.
> 
> 
> Jassi Brar (4):
>    dt: fwu: developerbox: enable fwu banks and mdata regions
>    configs: move to new flash layout and boot flow
>    fwu: DeveloperBox: add support for FWU
>    fwu: provide default fwu_plat_get_bootidx
> 
> Masami Hiramatsu (2):
>    FWU: Add FWU metadata access driver for MTD storage regions
>    tools: Add mkfwumdata tool for FWU metadata image
> 
>   .../synquacer-sc2a11-developerbox-u-boot.dtsi |  49 ++-
>   board/socionext/developerbox/Makefile         |   1 +
>   board/socionext/developerbox/developerbox.c   |   8 +
>   board/socionext/developerbox/fwu_plat.c       |  37 ++
>   configs/synquacer_developerbox_defconfig      |  12 +-
>   doc/board/socionext/developerbox.rst          | 154 +++++++-
>   doc/mkfwumdata.1                              |  89 +++++
>   drivers/fwu-mdata/Kconfig                     |  15 +
>   drivers/fwu-mdata/Makefile                    |   1 +
>   drivers/fwu-mdata/raw_mtd.c                   | 269 ++++++++++++++
>   include/configs/synquacer.h                   |  10 +
>   include/fwu.h                                 |  32 ++
>   lib/fwu_updates/Makefile                      |   1 +
>   lib/fwu_updates/fwu.c                         |  18 +
>   lib/fwu_updates/fwu_mtd.c                     | 185 ++++++++++
>   tools/Kconfig                                 |   9 +
>   tools/Makefile                                |   4 +
>   tools/mkfwumdata.c                            | 334 ++++++++++++++++++
>   18 files changed, 1217 insertions(+), 11 deletions(-)
>   create mode 100644 board/socionext/developerbox/fwu_plat.c
>   create mode 100644 doc/mkfwumdata.1
>   create mode 100644 drivers/fwu-mdata/raw_mtd.c
>   create mode 100644 lib/fwu_updates/fwu_mtd.c
>   create mode 100644 tools/mkfwumdata.c
> 

I know that Tom has applied this already but I still think there is a work to do 
here.
Firstly I can generate 2 images per one bank which should be pretty much regular 
capsule update for 2 images. I would expect this should still work.

And then I tried 2 banks with 2 images and fwu_gen_alt_info_from_mtd() generated 
this description for DFU

mtd nor0=bank0 raw 2320000 80000;bank1 raw 27a0000 8000&mtd nor0=bank0 raw 
23a0000 4000000;bank1 raw 2820000 4000000

If you look at size in second entry you will see that it is 8000 instead of 
80000 because it is the same image. That's why curious if you have tested any 
scenario like this.

Next part which I want to take a look is practicality of CONFIG_FWU_NUM_BANKS 
and CONFIG_FWU_NUM_IMAGES_PER_BANK because it pretty much enforcing number of 
banks and images for every platform and prevent creating one u-boot which works 
on different boards and just use information from mdata.
DEN0118 doesn't show any field with this information but do you think that would 
be possible to extract that information from there based on for example reserved 
or accepted field?

Thanks,
Michal