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

[Michal Simek <michal.simek@amd.com>]

On 3/27/23 23:14, jassisinghbrar@gmail.com wrote:
> From: Jassi Brar <jaswinder.singh@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 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 (3):
>    dt: fwu: developerbox: enable fwu banks and mdata regions
>    configs: move to new flash layout and boot flow
>    fwu: DeveloperBox: add support for FWU
> 
> Masami Hiramatsu (3):
>    FWU: Add FWU metadata access driver for MTD storage regions
>    FWU: mtd: Add helper functions for accessing FWU metadata
>    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       |  57 +++
>   configs/synquacer_developerbox_defconfig      |  12 +-
>   doc/board/socionext/developerbox.rst          | 155 +++++++-
>   drivers/fwu-mdata/Kconfig                     |  15 +
>   drivers/fwu-mdata/Makefile                    |   1 +
>   drivers/fwu-mdata/raw_mtd.c                   | 272 ++++++++++++++
>   include/configs/synquacer.h                   |  10 +
>   include/fwu.h                                 |  34 ++
>   lib/fwu_updates/Makefile                      |   1 +
>   lib/fwu_updates/fwu_mtd.c                     | 164 +++++++++
>   tools/Kconfig                                 |   9 +
>   tools/Makefile                                |   4 +
>   tools/mkfwumdata.c                            | 334 ++++++++++++++++++
>   16 files changed, 1115 insertions(+), 11 deletions(-)
>   create mode 100644 board/socionext/developerbox/fwu_plat.c
>   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 have played with this more and I found other things.

Ilias:
mkeficapsule is accepting only guid and mkfwumdata is accepting only uuid or 
guids. And DT is having uuids only.
I think it will be good to sync it up because you need to have both version for 
images generation which is a little bit painful. It should be possible to list 
only guids or uuids.

And it is not clear to me how u-boot is talking to boot firmware about next boot 
index. fwu_plat_get_bootidx() returns index which booted but I can't see any 
hoook to tell from u-boot to boot firmware what should be image/index to boot 
after capsule update.

I expect it should work like this.
Boot to u-boot - let's say index 0.
Run capsule update which will update index 1.
Inform boot firmware to boot index 1 (this step I am missing)
Call reset
Boot to u-boot index 1.
Apply accept capsule to confirm that image at index 1 is correct

in case of error (watchdog for example)
boot to u-boot index 0
apply revert capsule and disable index 1 image.

Thanks,
Michal