Re: [meta-ti] [dunfell/master PATCH] pruss-fw: am57xx: Update hsr/prp/sw/dual-emac firmware

["Denys Dmytriyenko" <denis@denix.org>]

On Fri, Mar 04, 2022 at 03:24:37PM -0500, Denys Dmytriyenko wrote:
> On Fri, Mar 04, 2022 at 02:06:47PM -0600, Ryan Eatmon wrote:
> > 
> > 
> > On 3/4/2022 13:40, Bajjuri, Praneeth wrote:
> > >
> > >
> > >On 3/4/2022 1:25 PM, Ryan Eatmon wrote:
> > >>>diff --git a/recipes-bsp/prueth-fw/prueth-fw_5.6.15.bb
> > >>>b/recipes-bsp/prueth-fw/prueth-fw.bb

Also, might want to name it prueth-fw_git.bb - not required, just a naming 
convention to indicate it is pulled from git and version is set by recipe.


> > >>>similarity index 88%
> > >>>rename from recipes-bsp/prueth-fw/prueth-fw_5.6.15.bb
> > >>>rename to recipes-bsp/prueth-fw/prueth-fw.bb
> > >>>index f7336fb1..e41bb3c0 100644
> > >>>--- a/recipes-bsp/prueth-fw/prueth-fw_5.6.15.bb
> > >>>+++ b/recipes-bsp/prueth-fw/prueth-fw.bb
> > >>>@@ -2,6 +2,9 @@ SUMMARY = "PRU Ethernet firmware for AM57xx,
> > >>>AM437x and AM335x"
> > >>>  require recipes-bsp/ti-linux-fw/ti-linux-fw.inc
> > >>>+PV = "${PRUETH_FW_AM335X_VERSION}"
> > >>>+PV = "${PRUETH_FW_AM437X_VERSION}"
> > >>>+PV = "${PRUETH_FW_AM57XX_VERSION}"
> > >>>  PR = "${INC_PR}.0"
> > >>
> > >>I think you need to uniquify the PV variable for each platform. 
> > >>Right now you are just resetting each time and not overriding
> > >>it.
> > >>
> > >>PV_am335x-evm = "${PRUETH_FW_AM335X_VERSION}"
> > >>etc...
> > >
> > >Thanks for the quick review,
> > >Does this work instead? PV_ti-soc?
> > >
> > >The FW version for each platform could be different as the
> > >mentioned commit in commitmessage points to updating the firmware
> > >only for am57xx but not for am335x/am437x.
> > 
> > While I think that might technically work, there are only three
> > machines that come into these recipes.  I think it would be more
> > confusing for someone reading the recipe to wonder what all makes up
> > ti-soc, rather than just seeing the three specific variables called
> > out.  Plus you created three variables in the ti-linux-fw.inc.  It
> > would fit more in line with other things in the same recipe to just
> > list them out:
> > 
> > 
> > TARGET = ""
> > TARGET_ti33x = "am335x-pru0-prueth-fw.elf am335x-pru1-prueth-fw.elf"
> > TARGET_ti43x = "am437x-pru0-prueth-fw.elf am437x-pru1-prueth-fw.elf"
> > TARGET_am57xx-evm = "am57xx-pru0-prueth-fw.elf am57xx-pru1-prueth-fw.elf"
> > TARGET_am57xx-hs-evm = "am57xx-pru0-prueth-fw.elf am57xx-pru1-prueth-fw.elf"
> > 
> > and
> > 
> > PV = ""
> > PV_ti33x = "${PRUETH_FW_AM335X_VERSION}"
> > PV_ti43x = "${PRUETH_FW_AM437X_VERSION}"
> > PV_am57xx-evm = "${PRUETH_FW_AM57XX_VERSION}"
> > PV_am57xx-hs-evm = "${PRUETH_FW_AM57XX_VERSION}"
> 
> No, PV_ti-soc won't work, it will be the same as plain PV and won't use the 
> correct value per platform.
> 
> What Ryan explains is correct - need an override per platform.
> 
> BTW, master uses a different override syntax, so this won't apply to master or 
> kirkstone and would need to be updated...

-- 
Regards,
Denys Dmytriyenko <denis@denix.org>
PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964