[Buildroot] [PATCH v1 1/3] add package/versal-firmware

Sun Aug 21 07:27:13 UTC 2022

Hi Yann,

> Le 20 août 2022 à 21:17, Yann E. MORIN <yann.morin.1998 at free.fr> a écrit :
> 
> Neal, All,
> 
> On 2022-08-20 17:52 +0000, Frager, Neal via buildroot spake thusly:
>>> This looks like your personal git tree, not the official one. (Yeah,
>>> I noticed your email address, but I would still have expected something
>>> under https://github.com/Xilinx/).
>> I am working on this and am using my own personal github location in
>> the meantime to figure out how this could work with buildroot.
> 
> Ah, I understand now. Next time, mark your patch as RFC.
> 
>>> +if BR2_PACKAGE_VERSAL_FIRMWARE
>>> +
>>> +config BR2_PACKAGE_VERSAL_FIRMWARE_LOCATION
>>> +     string "versal firmware location"
>>> The prompt should be "Custom git repository"
>> Could you clarify this a bit?  Are you speaking of the BR2 name or the
>> string text when you say the prompt?
> 
> The prompt is the ârt that is displayed to the user, i.e. the string
> between quotes after the option type, in this case:  "versal firmware
> location" should be replace with "Custom git repository".
> 
> And of course, the astute reader you are noticed that the option name
> als needs a rename, BR2_PACKAGE_VERSAL_FIRMWARE_CUSTOM_GIT
> 
> However, this will ultimately be uneeded, because the firmware files
> hould be downloadable login-less from an official location.
> 
> So, the two options that specify the git tree location adn version are
> not needed. Just hard-code them in the .mk file
> 

I will hard-code the location because you are right that selecting it is not necessary.

However, I would like to keep version and board name configurable in the defconfig file.

This way, users can select whichever version of boot firmware they wish to use for the target board they wish to use.

> So, the Config.in file should only contain (skeleton with abreviations,
> extend as/where appropriate):
> 
>    config BR2_PKG_VERSAL_FW
>        bool "versal firmware
>        help
>          Firmware files the the Xilinx Versal familly of boards.
> 
>          https://official.xilinix/location/github/whatever/
> 
>    if BR2_PKG_VERSAL_FW
> 
>    choice
>        bool "Versal board"
> 
>    config BR2_PKG_VERSAL_FW_VCK190
>        bool "vck190
> 
>    config BR2_PKG_VERSAL_FW_SOMETHING_ELSE
>        bool "something else"
> 
>    endchoice
> 
>    config BR2_PKG_VERSAL_FW_BOARD
>        string
>        default "vck190" if BR2_PKG_VERSAL_FW_VCK190
>        default "something-else" if BR2_PKG_VERSAL_FW_SOMETHING_ELSE
> 
>    endif
> 
> and the .mk would look like:
> 
>    VERSAL_FW_VERSION = 076d4b4f02b475dd9c65ad51992bfcad8d98b002
>    VERSAL_FW_SITE = $(call github,nealfrager,versal_boot,$(VERSALE_FW_VERSION))
> 
>    VERSAL_FW_INSTALL_TARGET = NO
>    VERSAL_FIRMWARE_INSTALL_IMAGES = YES
> 
>    define VERSAL_FW_INSTALL_IMAGES_CMDS
>        $(INSTALL) -D -m 0644 $(@D)/$(VERSAL_FW_BOARD)/boot.bin \
>            $(BINARIES_DIR)/boot.bin
>    endef
> 
>    $(eval $(generic-package))
> 

Thanks for this.

>>> +     help
>>> +       Location of a versal firmware boot.bin.
>>> +
>>> +       The value should be a git repository.
>> 
>>> So, there is not even a default location?
>> 
>> The official default solution (aside from petalinux) is to clone the
>> embeddedsw github location and build the plm.elf and psmfw.elf from
>> that with a microblaze compiler.
>> And then use bootgen to generate a boot.bin.
>> While I can build u-boot and atf easily with buildroot, I do not have
>> a clean solution for building the plm and psmfw images and generating
>> the boot.bin.
> 
> Yeah, I can see it being quite cumbersome. But it is not unheard of in
> Buildroot already. For example, we have package/arm-gnu-toolchain/
> that provides a pre-built armv7 toolchain, that is used to buid the
> boot/arm-trusted-firmware/ in certain conditions.
> 
> So, it would be technically possible to package a microblaze compiler
> like we've done for arm-gnu-toolchain, and use it to build the plm.elf
> and psmfw.elf from "the embeddedsw".
> 
> But that is a bit of a stretch to go. Unless...
> 

I will save this for a later version as the effort to make this work properly is significantly greater.

>> This is why I am looking for a short term solution of just downloading
>> a prebuilt boot.bin.
>> At the moment, distribution of pre-built binaries requires the
>> click-wrap downloading procedure which does not lend itself well for
>> an automated buildroot download.
> 
> Above, you seemed to imply you were looking internally to lift this
> click-wrap restriction. If that is not possible, and you can't have the
> pre-built blobs hosted in an official Xilinx location, then downloading
> the "embeddedsw" and building them would be a fallback.
> 

I still have hope for a shorter term solution, but yes, we agree.

>>> Also, no need for a seaprate macro; just add this code to VERSAL_FIRMWARE_INSTALL_IMAGES_CMDS.
>> Same comment as above.  I was working from other examples of firmware
>> downloading to create this patch set, and I appreciate your feedback
>> for making it simpler.
> 
> Looking at other examples is a good refelex! :-) But other may install
> more than one file at a time, so they need this complexity of
> conditional append-assignments. In your case, it is my understanding
> that only one file will ever be copied.
> 
>>> In the end, I don't think we should apply this package, as it looks
>>> like it tries to circumvent the official distrobution restrictions
>>> on these firmware files.
>> I understand, and I hope to be able to come up with a solution.
> 
> Great! In the meantime, as it is a WIP, I've marked the series as
> changes-requested in patchwork. Thanks for the feedback!
> 

I will send a simpler version 2 of this WIP patch set tomorrow for your review.  Thank you for your help!

>> If you have any ideas for how buildroot might be able to generate the
>> boot.bin, I am happy to receive any help.
> 
> I hope the lead I pointed to above will be enough as a starting point
> for you. Do not hesitate to ask further if you have concerns.
> 

Any leads are useful at the moment.  Thank you!

> Thanks!
> 
> Regards,
> Yann E. MORIN.
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'

Best regards,
Neal Frager
AMD