Hi Manju,

> On Jul 9, 2019, at 15:22, Manjukumar Harthikote Matha <manju...@xilinx.com> 
> wrote:
> 
> Hi JD,
> 
>> -----Original Message-----
>> From: Jean-Francois Dagenais <jeff.dagen...@gmail.com>
>> Sent: Tuesday, July 9, 2019 8:45 AM
>> To: meta-xilinx@yoctoproject.org; git <g...@xilinx.com>
>> Cc: Jean-Francois Dagenais <jeff.dagen...@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.dagen...@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 ?= ""


-- 
_______________________________________________
meta-xilinx mailing list
meta-xilinx@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-xilinx

Reply via email to