[U-Boot] [PATCH v8 4/8] ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading
Marek Vasut
marex at denx.de
Thu Feb 14 10:41:19 UTC 2019
On 2/14/19 7:44 AM, Chee, Tien Fong wrote:
> On Wed, 2019-02-13 at 17:20 +0100, Marek Vasut wrote:
>> On 2/13/19 3:18 PM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>
>>> Add FPGA driver to support program FPGA with FPGA bitstream loading
>>> from
>>> filesystem. The driver are designed based on generic firmware
>>> loader
>>> framework. The driver can handle FPGA program operation from
>>> loading FPGA
>>> bitstream in flash to memory and then to program FPGA.
>>>
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
>>>
>>> ---
>>>
>>> changes for v8
>>> - Added codes to discern bitstream type based on fpga node name.
>>>
>>> changes for v7
>>> - Restructure the FPGA driver to support both peripheral bitstream
>>> and core
>>> bitstream bundled into FIT image.
>>> - Support loadable property for core bitstream. User can set
>>> loadable
>>> in DDR for better performance. This loading would be done in one
>>> large
>>> chunk instead of chunk by chunk loading with small memory buffer.
>>> ---
>>> arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 17 +
>>> .../include/mach/fpga_manager_arria10.h | 39 +-
>>> drivers/fpga/socfpga_arria10.c | 467
>>> ++++++++++++++++++++-
>>> 3 files changed, 500 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>> b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>> index 998d811..14f1967 100644
>>> --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>> +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
>>> @@ -18,6 +18,23 @@
>>> /dts-v1/;
>>> #include "socfpga_arria10_socdk.dtsi"
>>>
>>> +/ {
>>> + chosen {
>>> + firmware-loader = &fs_loader0;
>> Should be a phandle.
> Can we change this label to phandle stage by stage, may be after fpga
> driver? This requires time working on firmware loader.
We should fix this as soon as possible, otherwise people might find this
bad example and wonder why it doesn't work once this is changed.
>>
>>>
>>> + };
>>> +
>>> + fs_loader0: fs-loader at 0 {
>>> + u-boot,dm-pre-reloc;
>>> + compatible = "u-boot,fs-loader";
>>> + phandlepart = <&mmc 1>;
>>> + };
>>> +};
>>> +
>>> +&fpga_mgr {
>>> + u-boot,dm-pre-reloc;
>>> + altr,bitstream = "fit_spl_fpga.itb";
>>> +};
>>> +
>>> &mmc {
>>> u-boot,dm-pre-reloc;
>>> status = "okay";
[...]
>>> - * FPGA Manager to program the FPGA. This is the interface used by
>>> FPGA driver.
>>> - * Return 0 for sucess, non-zero for error.
>>> - */
>>> +char *get_fpga_filename(const void *fdt, int *len)
>>> +{
>>> + char *fpga_filename = NULL;
>>> + int node_offset;
>>> +
>>> + fdtdec_find_aliases_for_id(gd->fdt_blob, "fpga_mgr",
>>> + COMPAT_ALTERA_SOCFPGA_FPGA0,
>>> + &node_offset, 1);
>>> +
>>> + ofnode fpgamgr_node = offset_to_ofnode(node_offset);
>>> +
>>> + if (ofnode_valid(fpgamgr_node))
>>> + fpga_filename = (char
>>> *)ofnode_read_string(fpgamgr_node,
>>> + "altr,bitstream");
>>> +
>>> +
>> Why is the cast needed ?
> The return string would be eventually set to the char *filename in
> common struct fpga_fsinfo. So, the cast here is to avoid the warning
> from compiler.
I presume if the compiler generates a warning, it's for a reason. What
warning is that ?
>> Drop the two newlines.
> Okay.
>>
>>>
>>> + return fpga_filename;
>>> +}
>>> +
>>> +static void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer)
>>> +{
>>> + /*
>>> + * Magic ID starting at:
>>> + * -> 1st dword[15:0] in periph.rbf
>>> + * -> 2nd dword[15:0] in core.rbf
>>> + * Note: dword == 32 bits
>>> + */
>>> + u32 word_reading_max = 2;
>>> + u32 i;
>>> +
>>> + for (i = 0; i < word_reading_max; i++) {
>>> + if (*(buffer + i) ==
>>> FPGA_SOCFPGA_A10_RBF_UNENCRYPTED) {
>>> + rbf->security = unencrypted;
>>> + } else if (*(buffer + i) ==
>>> FPGA_SOCFPGA_A10_RBF_ENCRYPTED) {
>>> + rbf->security = encrypted;
>>> + } else if (*(buffer + i + 1) ==
>>> + FPGA_SOCFPGA_A10_RBF_UNENCRYPTED)
>>> {
>>> + rbf->security = unencrypted;
>>> + } else if (*(buffer + i + 1) ==
>>> + FPGA_SOCFPGA_A10_RBF_ENCRYPTED) {
>>> + rbf->security = encrypted;
>>> + } else {
>>> + rbf->security = invalid;
>>> + continue;
>>> + }
>>> +
>>> + /* PERIPH RBF(buffer + i + 1), CORE RBF(buffer + i
>>> + 2) */
>>> + if (*(buffer + i + 1) ==
>>> FPGA_SOCFPGA_A10_RBF_PERIPH) {
>>> + rbf->section = periph_section;
>>> + break;
>>> + } else if (*(buffer + i + 1) ==
>>> FPGA_SOCFPGA_A10_RBF_CORE) {
>>> + rbf->section = core_section;
>>> + break;
>>> + } else if (*(buffer + i + 2) ==
>>> FPGA_SOCFPGA_A10_RBF_PERIPH) {
>>> + rbf->section = periph_section;
>>> + break;
>>> + } else if (*(buffer + i + 2) ==
>>> FPGA_SOCFPGA_A10_RBF_CORE) {
>>> + rbf->section = core_section;
>>> + break;
>>> + }
>>> +
>>> + rbf->section = unknown;
>>> + break;
>>> +
>>> + WATCHDOG_RESET();
>>> + }
>>> +}
>>> +
>>> +#ifdef CONFIG_FS_LOADER
>>> +static int first_loading_rbf_to_buffer(struct udevice *dev,
>>> + struct fpga_loadfs_info
>>> *fpga_loadfs,
>>> + u32 *buffer, size_t *buffer_bsize)
>>> +{
>>> + u32 *buffer_p = (u32 *)*buffer;
>>> + u32 *loadable = buffer_p;
>>> + size_t buffer_size = *buffer_bsize;
>>> + size_t fit_size;
>>> + int ret, i, count;
>>> + int confs_noffset, images_noffset;
>>> + int rbf_offset;
>>> + int rbf_size;
>>> + const char *fpga_node_name = NULL;
>>> + const char *uname = NULL;
>>> +
>>> + /* Load image header into buffer */
>>> + ret = request_firmware_into_buf(dev,
>>> + fpga_loadfs->fpga_fsinfo-
>>>> filename,
>>> + buffer_p,
>>> + sizeof(struct
>>> image_header),
>>> + 0);
>>> + if (ret < 0) {
>>> + debug("FPGA: Failed to read image header from
>>> flash.\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + if (image_get_magic((struct image_header *)buffer_p) !=
>>> FDT_MAGIC) {
>>> + debug("FPGA: No FDT magic was found.\n");
>>> + return -EBADF;
>>> + }
>>> +
>>> + fit_size = fdt_totalsize(buffer_p);
>>> +
>>> + if (fit_size > buffer_size) {
>>> + debug("FPGA: FIT image is larger than available
>>> buffer.\n");
>>> + debug("Please use FIT external data or increasing
>>> buffer.\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + /* Load entire FIT into buffer */
>>> + ret = request_firmware_into_buf(dev,
>>> + fpga_loadfs->fpga_fsinfo-
>>>> filename,
>>> + buffer_p,
>>> + fit_size,
>>> + 0);
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = fit_check_format(buffer_p);
>>> + if (!ret) {
>>> + debug("FPGA: No valid FIT image was found.\n");
>>> + return -EBADF;
>>> + }
>>> +
>>> + confs_noffset = fdt_path_offset(buffer_p, FIT_CONFS_PATH);
>>> + images_noffset = fdt_path_offset(buffer_p,
>>> FIT_IMAGES_PATH);
>>> + if (confs_noffset < 0 || images_noffset < 0) {
>>> + debug("FPGA: No Configurations or images nodes
>>> were found.\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + /* Get default configuration unit name from default
>>> property */
>>> + confs_noffset = fit_conf_get_node(buffer_p, NULL);
>>> + if (confs_noffset < 0) {
>>> + debug("FPGA: No default configuration was found in
>>> config.\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + count = fit_conf_get_prop_node_count(buffer_p,
>>> confs_noffset,
>>> + FIT_FPGA_PROP);
>>> +
>>> + if (count < 0) {
>>> + debug("FPGA: Invalid configuration format for FPGA
>>> node.\n");
>>> + return count;
>>> + } else {
>>> + debug("FPGA: FPGA node count: %d\n", count);
>>> + }
>>> +
>>> + for (i = 0; i < count; i++) {
>>> + images_noffset =
>>> fit_conf_get_prop_node_index(buffer_p,
>>> + confs
>>> _noffset,
>>> + FIT_F
>>> PGA_PROP, i);
>>> + uname = fit_get_name(buffer_p, images_noffset,
>>> NULL);
>>> + if (uname) {
>>> + debug("FPGA: %s\n", uname);
>>> +
>>> + if (strstr(uname, "fpga-periph") &&
>>> + (!is_fpgamgr_early_user_mode() ||
>>> + is_fpgamgr_user_mode())) {
>>> + fpga_node_name = uname;
>>> + printf("FPGA: Start to program ");
>>> + printf("peripheral/full bitstream
>>> ...\n");
>>> + break;
>>> + } else if (strstr(uname, "fpga-core") &&
>>> + (is_fpgamgr_early_user_mod
>>> e() &&
>>> + !is_fpgamgr_user_mode()))
>>> {
>>> + fpga_node_name = uname;
>>> + printf("FPGA: Start to program
>>> core ");
>>> + printf("bitstream ...\n");
>>> + break;
>>> + }
>>> + }
>>> + WATCHDOG_RESET();
>>> + }
>>> +
>>> + if (!fpga_node_name) {
>>> + debug("FPGA: No suitable bitstream was found,
>>> count: %d.\n", i);
>>> + return 1;
>>> + }
>>> +
>>> + images_noffset = fit_image_get_node(buffer_p,
>>> fpga_node_name);
>>> + if (images_noffset < 0) {
>>> + debug("FPGA: No node '%s' was found in FIT.\n",
>>> + fpga_node_name);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + ret = fit_image_get_data_position(buffer_p,
>>> images_noffset,
>>> + &rbf_offset);
>>> + if (ret < 0) {
>>> + debug("FPGA: No data position was found
>>> (err=%d).\n", ret);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + ret = fit_image_get_data_size(buffer_p, images_noffset,
>>> &rbf_size);
>>> + if (ret < 0) {
>>> + debug("FPGA: No data size was found (err=%d).\n",
>>> ret);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + ret = fit_image_get_load(buffer_p, images_noffset, (ulong
>>> *)loadable);
>>> + if (ret < 0) {
>>> + debug("FPGA: No loadable was found (err=%d).\n",
>>> ret);
>>> + debug("FPGA: Using default buffer and size.\n");
>>> + } else {
>>> + buffer_p = (u32 *)*loadable;
>>> + buffer_size = rbf_size;
>>> + debug("FPGA: Found loadable address = 0x%x.\n",
>>> *loadable);
>>> + }
>>> +
>>> + debug("FPGA: External data: offset = 0x%x, size =
>>> 0x%x.\n",
>>> + rbf_offset, rbf_size);
>>> +
>>> + fpga_loadfs->remaining = rbf_size;
>>> +
>>> + /*
>>> + * Determine buffer size vs bitstream size, and
>>> calculating number of
>>> + * chunk by chunk transfer is required due to smaller
>>> buffer size
>>> + * compare to bitstream
>>> + */
>>> + if (rbf_size <= buffer_size) {
>>> + /* Loading whole bitstream into buffer */
>>> + buffer_size = rbf_size;
>>> + fpga_loadfs->remaining = 0;
>>> + } else {
>>> + fpga_loadfs->remaining -= buffer_size;
>>> + }
>> Shouldn't all this parsing and calculation be done by the firmware
>> loader code ?
> The calculation here is to determine the available memory size can be
> used, it could be size from OCRAM buffer, or DDR.
It seems rather that the code is parsing the fitImage structures ?
How is that related to such calculations ?
>>>
>>> + fpga_loadfs->offset = rbf_offset;
>>> + /* Loading bitstream into buffer */
>>> + ret = request_firmware_into_buf(dev,
>>> + fpga_loadfs->fpga_fsinfo-
>>>> filename,
>>> + buffer_p,
>>> + buffer_size,
>>> + fpga_loadfs->offset);
>>> + if (ret < 0) {
>>> + debug("FPGA: Failed to read bitstream from
>>> flash.\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + /* Getting info about bitstream types */
>>> + get_rbf_image_info(&fpga_loadfs->rbfinfo, (u16
>>> *)buffer_p);
>>> +
>>> + /* Update next reading bitstream offset */
>>> + fpga_loadfs->offset += buffer_size;
>>> +
>>> + /* Update the final addr for bitstream */
>>> + *buffer = (u32)buffer_p;
>>> +
>>> + /* Update the size of bitstream to be programmed into FPGA
>>> */
>>> + *buffer_bsize = buffer_size;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int subsequent_loading_rbf_to_buffer(struct udevice *dev,
>>> + struct fpga_loadfs_info
>>> *fpga_loadfs,
>>> + u32 *buffer, size_t
>>> *buffer_bsize)
>>> +{
>>> + int ret = 0;
>>> + u32 *buffer_p = (u32 *)*buffer;
>>> +
>>> + /* Read the bitstream chunk by chunk. */
>>> + if (fpga_loadfs->remaining > *buffer_bsize) {
>>> + fpga_loadfs->remaining -= *buffer_bsize;
>>> + } else {
>>> + *buffer_bsize = fpga_loadfs->remaining;
>>> + fpga_loadfs->remaining = 0;
>>> + }
>>> +
>>> + ret = request_firmware_into_buf(dev,
>>> + fpga_loadfs->fpga_fsinfo-
>>>> filename,
>>> + buffer_p,
>>> + *buffer_bsize,
>>> + fpga_loadfs->offset);
>>> + if (ret < 0) {
>>> + debug("FPGA: Failed to read bitstream from
>>> flash.\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + /* Update next reading bitstream offset */
>>> + fpga_loadfs->offset += *buffer_bsize;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf,
>>> size_t bsize,
>>> + u32 offset)
>>> +{
>>> + struct fpga_loadfs_info fpga_loadfs;
>>> + int status = 0;
>>> + int ret = 0;
>>> + u32 buffer = (u32)buf;
>> This will fail on arm64 , look at uintptr_t .
> This driver is only used by A10 which is arm32. You want me to use
> (u32)(uintptr_t)buf?
If you want to cast pointer to integer type, yes, that's uintptr_t .
>>
>>>
>>> + size_t buffer_sizebytes = bsize;
>>> + size_t buffer_sizebytes_ori = bsize;
>>> + size_t total_sizeof_image = 0;
>>> + struct udevice *dev;
>>> +
>>> + ret = uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0,
>>> &dev);
>> Shouldn't the firmware loaded instance be obtained via the DT phandle
>> ?
> It just to get the device activated. The firmware loaded itself would
> go to choosen node(default) for getting the label. I can change to the
> phandle, may be after this?
Except this always activates the first device in the list.
[...]
--
Best regards,
Marek Vasut
More information about the U-Boot
mailing list