On 2026-04-28 12:07 -0600, Simon Glass wrote: > Hi Harsimran, > > On 2026-04-24T17:31:50, Harsimran Singh Tungal > <[email protected]> wrote: > > sandbox: ffa: share synthetic partition metadata via macros > > > > Reuse sandbox FF-A partition constants in emulator tests > > > > Allow the sandbox FF-A emulator test to use execution-context and property > > constants defined in sandbox_arm_ffa.h > > > > Signed-off-by: Harsimran Singh Tungal <[email protected]> > > > > drivers/firmware/arm-ffa/ffa-emul-uclass.c | 36 > > +++++++++++++++++++++--------- > > 1 file changed, 26 insertions(+), 10 deletions(-) > > > diff --git a/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > b/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > @@ -19,41 +19,57 @@ > > /* The partitions (SPs) table */ > > static struct ffa_partition_desc > > sandbox_partitions[SANDBOX_PARTITIONS_CNT] = { > > { > > - .info = { .id = SANDBOX_SP1_ID, .exec_ctxt = 0x5687, > > .properties = 0x89325621 }, > > + .info = { > > + .id = SANDBOX_SP1_ID, > > + .exec_ctxt = SANDBOX_SP1_EXEC_CTXT, > > + .properties = SANDBOX_SP1_PROPERTIES, > > + }, > > .sp_uuid = { > > .a1 = SANDBOX_SERVICE1_UUID_A1, > > ... > > { > > - .info = { .id = SANDBOX_SP3_ID, .exec_ctxt = 0x7687, > > .properties = 0x23325621 }, > > + .info = { > > + .id = SANDBOX_SP2_ID, > > + .exec_ctxt = SANDBOX_SP2_EXEC_CTXT, > > + .properties = SANDBOX_SP2_PROPERTIES, > > + }, > > .sp_uuid = { > > .a1 = SANDBOX_SERVICE2_UUID_A1, > > ... > > { > > - .info = { .id = SANDBOX_SP2_ID, .exec_ctxt = 0x9587, > > .properties = 0x45325621 }, > > + .info = { > > + .id = SANDBOX_SP3_ID, > > + .exec_ctxt = SANDBOX_SP3_EXEC_CTXT, > > + .properties = SANDBOX_SP3_PROPERTIES, > > + }, > > .sp_uuid = { > > .a1 = SANDBOX_SERVICE1_UUID_A1, > > This is not a pure refactor. Index 1 and 2 swap, so SP2 moves from > SERVICE1 to SERVICE2, and SP3 moves from SERVICE2 to SERVICE1. The > existing test in test/dm/ffa.c queries by UUID and walks the matching > SPs, so the set of IDs returned for each service changes. > > If the reorder is deliberate (e.g. so the table reads SP1..SP4 in > numeric order), please call it out in the commit message and confirm > no downstream test relies on the previous SP-to-UUID mapping. > Otherwise keep the .id fields where they were and only substitute in > the macros. > > > diff --git a/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > b/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > @@ -19,41 +19,57 @@ > > + .info = { > > + .id = SANDBOX_SP1_ID, > > + .exec_ctxt = SANDBOX_SP1_EXEC_CTXT, > > + .properties = SANDBOX_SP1_PROPERTIES, > > + }, > > The macros being substituted in here are added in patch 8, and nothing > else in this patch uses them - they exist solely to be consumed by > this patch. Can you squash 9 into 8 so the macro definitions and their > first user land together. >
As suggested, This patch has been now squashed to previous patch and all the comments for this commit have been addressed in v2. Link to v2: https://lore.kernel.org/u-boot/[email protected]/ Regards Harsimran Singh Tungal > > Author: Harsimran Singh Tungal <[email protected]> > > > > > sandbox: ffa: share synthetic partition metadata via macros > > > > Reuse sandbox FF-A partition constants in emulator tests > > > > Allow the sandbox FF-A emulator test to use execution-context and > > property > > constants defined in sandbox_arm_ffa.h > > The body reads like two competing subject lines followed by a one-line > restatement, and none of it explains the motivation. Also, the file > changed is the emulator itself (ffa-emul-uclass.c), not a test, so 'in > emulator tests' / 'emulator test' is misleading. Please rewrite to say > what problem is being solved (e.g. the runtime tests added in 8 need > to reference the same exec_ctxt/properties values, so the magic > numbers are pulled out into shared macros) and drop the duplicate > sentence. > > Regards, > Simon >
