mbox series

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

Message ID 20230327211458.498879-1-jaswinder.singh@linaro.org
Headers show
Series FWU: Add support for mtd backed feature on DeveloperBox | expand

Message

Jassi Brar March 27, 2023, 9:14 p.m. UTC
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

Comments

Michal Simek March 29, 2023, 1:14 p.m. UTC | #1
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/

When I build it on the top of this series for our SOC I am getting

    aarch64:  +   xilinx_zynqmp_virt
+lib/fwu_updates/fwu.o: In function `fwu_boottime_checks':
+build/../lib/fwu_updates/fwu.c:633: undefined reference to `fwu_plat_get_bootidx'
+lib/fwu_updates/fwu.o: In function `fwu_get_image_index':
+build/../lib/fwu_updates/fwu.c:381: undefined reference to `fwu_plat_get_alt_num'
+make[1]: *** [Makefile:1754: u-boot] Error 139
+make[1]: *** Deleting file 'u-boot'
+make: *** [Makefile:177: sub-make] Error 2

That means that I can select these options and I will get compilation error.
I understand the error but if anybody can select it you should use weak 
functions or so to be able to get it at least build.

The series doesn't look bad but still the biggest issue remains which is that DT 
binding is not approved by DT guys which pretty much means that any platform 
which starts to use this code can't pass DT yaml checking enforced by SR 2.0.

Thanks,
Michal
Michal Simek March 30, 2023, 1:35 p.m. UTC | #2
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