[meta-xilinx] [meta-xilinx-tools][RFC][patch 1/3] xilinx-bootbin: rename BIF_PARTITION_ATTR to BIF_PARTITIONS for clarity

[Jean-Francois Dagenais]

Hi Manju,

> On Jul 9, 2019, at 15:22, Manjukumar Harthikote Matha <MANJUKUM at xilinx.com> wrote:
> 
> Hi JD,
> 
>> -----Original Message-----
>> From: Jean-Francois Dagenais <jeff.dagenais at gmail.com>
>> Sent: Tuesday, July 9, 2019 8:45 AM
>> To: meta-xilinx at yoctoproject.org; git <git at xilinx.com>
>> Cc: Jean-Francois Dagenais <jeff.dagenais at gmail.com>
>> Subject: [meta-xilinx-tools][RFC][patch 1/3] xilinx-bootbin: rename
>> BIF_PARTITION_ATTR to BIF_PARTITIONS for clarity
>> 
>> Using BIF_PARTITION_ATTR main variable as the list of partitions is a
>> reuse of the variable which makes the definition and code much harder to
>> read and understand. Simply renaming the list of partitions as
>> BIF_PARTITIONS alleviates this completely.
>> 
>> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais at gmail.com>
>> ---
>> README.md                                     |  2 +-
>> recipes-bsp/bootbin/machine-xilinx-versal.inc |  2 +-
>> recipes-bsp/bootbin/machine-xilinx-zynq.inc   |  4 ++--
>> recipes-bsp/bootbin/machine-xilinx-zynqmp.inc |  4 ++--
>> recipes-bsp/bootbin/xilinx-bootbin_1.0.bb     | 14 +++++++-------
>> 5 files changed, 13 insertions(+), 13 deletions(-)
>> 
>> diff --git a/README.md b/README.md
>> index 65f6623..e4091e5 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -83,7 +83,7 @@ Examples for adding dependencies
>> 
>> See https://github.com/Xilinx/meta-xilinx-tools/blob/master/recipes-
>> bsp/bootbin/machine-xilinx-zynq.inc
>> 
>> -BIF_PARTITION_ATTR= "fsbl u-boot"
>> +BIF_PARTITIONS= "fsbl u-boot"
>> 
> 
> The reason to keep the variable name as "BIF partition attributes" is to match with the user guide of bootgen.
> https://www.xilinx.com/support/documentation/sw_manuals/xilinx2019_1/ug1283-bootgen-user-guide.pdf

That's fine for the "partition attributes" of the definitions, which I have left as is. I just thought putting the list of partitions in the variable BIF_PARTITION_ATTR was a arbitrary, or am I not seeing something correctly?

Why not put the list of partitions (used as flag names on the other vars as well) on the BIF_PARTITION_DEPENDS or BIF_PARTITION_IMAGE then? Why is the _ATTR the one carrying the "top level" list of partition "names"?

After spending minutes analyzing the whole picture, it just seemed to me like there was such a distinct thing called "List of BIF partitions" and that the BIF_PARTITION_ATTR variable which, like it's brothers _IMAGE and _DEPENDS, is mostly used as storage for the varFlags (with partition name as key), seemed to have been arbitrarily chosen as the storage (getVar instead of getVarFlag) for the list of partitions.

My 2 cents. ;)

> 
>> BIF_PARTITION_ATTR[fsbl]="bootloader"
>> 
>> diff --git a/recipes-bsp/bootbin/machine-xilinx-versal.inc b/recipes-
>> bsp/bootbin/machine-xilinx-versal.inc
>> index 2cdaee7..8304448 100644
>> --- a/recipes-bsp/bootbin/machine-xilinx-versal.inc
>> +++ b/recipes-bsp/bootbin/machine-xilinx-versal.inc
>> @@ -8,7 +8,7 @@ DEPENDS += "virtual/cdo"
>> BIF_COMMON_ATTR ?= ""