[PATCH v4 3/6] tools: Add mkfwumdata tool for FWU metadata image

Michal Simek michal.simek at amd.com
Mon Apr 17 16:29:26 CEST 2023



On 4/17/23 15:48, Jassi Brar wrote:
> On Mon, 17 Apr 2023 at 01:38, Michal Simek <michal.simek at amd.com> wrote:
>> On 4/14/23 17:02, Jassi Brar wrote:
> 
>>>>>>> +
>>>>>>> +/* This will dynamically allocate the fwu_mdata */
>>>>>>> +#define CONFIG_FWU_NUM_BANKS         0
>>>>>>> +#define CONFIG_FWU_NUM_IMAGES_PER_BANK       0
>>>>>>
>>>>>> These two are completely unused.
>>>>>>
>>>>> these are necessary for include/fwu_mdata.h that comes later.
>>>>
>>>> If that's come later, add it later.
>>>>
>>> Can you please actually look at the code to see the underlying constraint?
>>
>> Ok. I misunderstand your comment.
>>
>> You have it there because of #include <fwu_mdata.h> and using macros there.
>> Can you just allocated that structures at run time instead of statically defined
>> them via array? That would clean that design and also fix checkpatch issue.
>>
> I can't see how dynamically allocating an array of packed structures
> inside an array of packed structures , makes the design any cleaner.

It is not marked as __packed and based on what you are saying it should be 
marked like that.

> 
> I think this is a valid case where we can ignore the checkpatch
> warning because we know what we are doing.

I will let maintainer, who will merge this code, to decide on this.
I would at least make that comment much bigger to explain it better
that you are doing it because you don't want to redefine structures from 
fwu_mdata.h.

Thanks,
Michal


More information about the U-Boot mailing list