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

[Jassi Brar]

Hi Michal,

On Mon, 19 Jun 2023 at 10:02, Michal Simek <michal.simek at amd.com> wrote:
>
> 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/
> >
.....

> 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.
>
I had, and have, strong doubts about the practicality of 2
images/bank. There aren't enough specification details to explain how
only 1-out-of-N images could be updated. And if we always update all
images in a bank together, we might as well have them as one composite
image. I never got satisfactory clarification from designers and
implementers. So, sorry, I can't defend that scenario with my limited
knowledge.

> 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?
>
Unfortunately the DEN0118 spec doesn't leave any 'don't care' bits in
'accepted' or 'reserved' fields, all unused bits Must-Be-Zero. If we
use any bit, we'll be in violation of the spec.

However, we can do CRC32 calculations by varying NUM_IMAGES_PER_BANK
and NUM_BANKS and find the value pair for which the crc32 field
matches!
For limiting check-loops and finding corrupted metadata, the .config
can carry upper limits on both the options, say
MAX_NUM_IMAGES_PER_BANK=5 and MAX_NUM_BANKS=10 for the most special
scenario. If we find the approach acceptable, I can cook a patch as
proof-of-concept.

cheers.