On Mon, 17 Apr 2023 at 09:29, Michal Simek michal.simek@amd.com wrote:
On 4/17/23 15:48, Jassi Brar wrote:
On Mon, 17 Apr 2023 at 01:38, Michal Simek michal.simek@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.
Those structures already existed before my patchset. And I definitely remember they were suggested to be marked as packed but were not. So now you have to read to code to understand what they are.
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.
you mean redeclare, right? That is what the two define's do effectively.
-j