On Tue, 20 Jun 2023 at 05:05, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Sorry for being late to the party, > > +cc Jose who maintains DEN0118 > > On Mon, Jun 19, 2023 at 11:16:53AM -0500, Jassi Brar wrote: > > Hi Michal, > > > > On Mon, 19 Jun 2023 at 10:02, Michal Simek <michal.si...@amd.com> wrote: > > > > > > Hi Jassi, > > > > > > On 5/31/23 07:28, jaswinder.si...@linaro.org wrote: > > > > From: Jassi Brar <jaswinder.si...@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-jassisinghb...@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. > > This is intentionally left out, as we consider it a platform policy. > For example you could update a single bank and copy over the remaining > firmware images from a different ban. There's nothing on the spec that > prohibits you from doing so, but I personally think it's a bad idea. > Me too.
> Keep in mind there's a document hosted here[0] which explains the update > flow and documents what we have as code in U-Boot. > > As far as individual firmware components go now, do you have any useful > usecase? > No. And I don't think current A/B update implementation in u-boot even has helpers for platforms to implement multiple images per bank. Which is fine, except we pretend we do by having NUM_IMAGES_PER_BANK config -- which will always be set to 1 I predict ;) > The general guidance of [0] is construct a firmware > image, only update the components you want while leaving the rest on the > same revisions and update the entire firmware. The reason is that firmware > > Updating a single image of another bank is not as easy as it sounds. > exactly. > Firmware components nowadays, whether we like it or not, depend on each other. > Since firmware updates are not so often and fast, we didn't see any gains > from > over complicating the spec and explicitly describe that case, while dealing > with firmware component compatibility at the same time. > Totally. As I said, I don't believe in the practicality of more than 1 image/bank. So we are on the same page. > > > > > 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. > > Yes this is unfortunate. We did have similar concerns on when drafting the > spec, however we never had a solid usecase to justify this. Arm and > Linaro and working on a v2 (which we try to make compatible with v1) which > addresses this shortcoming. Maybe Jose can chime in. > OK. If we have, say, reserved[0] as last Image and reserved[1] as last Bank, we could get rid of the two compile-time configs. Thanks.
