[<prev] [next>] [day] [month] [year] [list]
Message-ID: <1CC272501B5BC543A05DB90AA509DED50B22C6@fmsmsx122.amr.corp.intel.com>
Date: Fri, 24 Feb 2017 18:08:03 +0000
From: "Nadathur, Sundar" <sundar.nadathur@...el.com>
To: "yi1.li@...ux.intel.com" <yi1.li@...ux.intel.com>,
"atull@...nel.org" <atull@...nel.org>,
"moritz.fischer@...us.com" <moritz.fischer@...us.com>,
"jgunthorpe@...idianresearch.com" <jgunthorpe@...idianresearch.com>
CC: "linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: RE: [RFC] Add a parser in fpga_region to decode the tlv meta data
suggested by Sundar
[Adding linux-kernel mailing list]
> -----Original Message-----
> From: yi1.li@...ux.intel.com [mailto:yi1.li@...ux.intel.com]
> Sent: Friday, February 24, 2017 9:29 AM
> To: atull@...nel.org; moritz.fischer@...us.com;
> jgunthorpe@...idianresearch.com
> Cc: linux-fpga@...r.kernel.org; Nadathur, Sundar
> <sundar.nadathur@...el.com>; Yi Li <yi1.li@...ux.intel.com>
> Subject: [RFC] Add a parser in fpga_region to decode the tlv meta data
> suggested by Sundar
>
> From: Yi Li <yi1.li@...ux.intel.com>
>
> The TLV contains 3 fields
> Type --- 4 bytes long, pre-defined
> Length --- 4 bytes long, Length refers to the length in bytes of the Value field
> Value --- variable length, TLV payload
>
> In the patch below, there are 5 types, which can be extended.
> TYPE_MDHEADER --- Meta Data Header TLV, which can contain sub TLV types
> TYPE_ENABLETIMEOUT --- Variable type TLV, with 4 bytes long of Value
> TYPE_DISABLETIMEOUT --- Variable type TLV, with 4 bytes long of Value
> TYPE_PARTIAL_RECONFIG --- Flag type TLV, with Zero Length.
> TYPE_BITSTREAM --- Raw bit stream type TLV.
>
> Example:
> 0x00000001 0x00000014 --- Mata Data header, includes 3 sub-TLVs below
> 0x00000002 0x00000004 0x00000004 --- fpga_image_info.enable_timeout_us
> = 0x4
> 0x00000003 0x00000004 0x00000004 ---
> fpga_image_info.disable_timeout_us = 0x4
> 0x00000005 0x00000000 --- Partial Reconfig Flag
> 0x00000004 0x00100000 --- Bitstream with 1MB long at the end
>
> Signed-off-by: Yi Li <yi1.li@...ux.intel.com>
> ---
> drivers/fpga/fpga-mgr.c | 10 ++++-
> drivers/fpga/fpga-region.c | 93
> +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index
> 3206a53..8d762d328 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -311,12 +311,18 @@ EXPORT_SYMBOL_GPL(fpga_mgr_firmware_load);
>
> int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
> {
> + int ret;
> +
> if (info->firmware_name)
> return fpga_mgr_firmware_load(mgr, info, info-
> >firmware_name);
> if (info->sgt)
> return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
> - if (info->buf && info->count)
> - return fpga_mgr_buf_load(mgr, info, info->buf, info->count);
> + if (info->buf && info->count) {
> + ret = fpga_mgr_buf_load(mgr, info, info->buf, info->count);
> + info->buf = NULL;
> + info->count = 0;
> + return ret;
> + }
> return -EINVAL;
> }
> EXPORT_SYMBOL_GPL(fpga_mgr_load);
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index
> a63bc6c..6d8bf47 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -15,7 +15,7 @@
> * You should have received a copy of the GNU General Public License along
> with
> * this program. If not, see <http://www.gnu.org/licenses/>.
> */
> -
> +#include <linux/firmware.h>
> #include <linux/fpga/fpga-bridge.h>
> #include <linux/fpga/fpga-mgr.h>
> #include <linux/idr.h>
> @@ -389,6 +389,83 @@ static void fpga_region_put(struct fpga_region
> *region)
> mutex_unlock(®ion->mutex);
> }
>
> +#define TYPE_MDHEADER 0x00000001
> +#define TYPE_ENABLETIMEOUT 0x00000002
> +#define TYPE_DISABLETIMEOUT 0x00000003
> +#define TYPE_BITSTREAM 0x00000004
> +#define TYPE_PARTIAL_RECONFIG 0x00000005
> +
> +/**
> + * fpga_region_parse_header - parser FPGA file header
> + * @data: file header start
> + * @size: file size
> + * @fpga_image_info
> + * @buf: raw bitstream start
> + * @count: raw bitstream size
> + *
> + * Parse the FPGA stream header.
> + *
> + * Return: 0 for success or negative error code.
> + */
> +static int fpga_region_parser_header(const char* data, size_t size,
> + struct fpga_image_info *image_info) {
> + int ret = 0;
> + u32 tlv_type;
> + u32 tlv_len;
> + u32 offset = 0;
> +
> + while (size >= 8) {
> + tlv_type = *((u32 *) (data + offset));
> + tlv_len = *((u32 *) (data + offset + 4));
> + offset += 8; size -= 8;
> +
> + if (size < tlv_len)
> + goto tlv_err;
> +
> + switch (tlv_type) {
> + case TYPE_MDHEADER:
> + break;
> + case TYPE_ENABLETIMEOUT:
> + if (tlv_len != sizeof(u32))
> + goto tlv_err;
> + image_info->enable_timeout_us = *((u32 *)
> (data + offset));
> + break;
> + case TYPE_DISABLETIMEOUT:
> + if (tlv_len != sizeof(u32))
> + goto tlv_err;
> + image_info->disable_timeout_us = *((u32 *)
> (data + offset));
> + break;
> + case TYPE_PARTIAL_RECONFIG:
> + if (tlv_len != 0)
> + goto tlv_err;
> + image_info->flags |=
> FPGA_MGR_PARTIAL_RECONFIG;
> + break;
> + case TYPE_BITSTREAM:
> + image_info->buf = data + offset;
> + image_info->count = tlv_len;
> + break;
> + default:
> + pr_err("unknown type\n");
> + ret = -EINVAL;
> + }
> +
> + /* TYPE_STRUCT is a super type, decode types inside of this
> struct */
> + if (tlv_type != TYPE_MDHEADER) {
> + offset += tlv_len;
> + size -= tlv_len;
> + }
> + }
> +
> + image_info->firmware_name = NULL;
> + image_info->sgt = NULL;
> + return ret;
> +
> +tlv_err:
> + pr_err("Error parsing tlv at type = %d offset = 0x%x\n", tlv_type,
> offset);
> + return -EINVAL;
> +}
> +
> /**
> * fpga_region_program_fpga - program FPGA
> * @region: FPGA region that is receiving an overlay @@ -401,6 +478,8 @@
> static void fpga_region_put(struct fpga_region *region) int
> fpga_region_program_fpga(struct fpga_region *region,
> struct fpga_image_info *image_info) {
> + struct device *dev = ®ion->dev;
> + const struct firmware *fw;
> int ret;
>
> region = fpga_region_get(region);
> @@ -415,6 +494,17 @@ int fpga_region_program_fpga(struct fpga_region
> *region,
> goto err_put_region;
> }
>
> + ret = request_firmware(&fw, image_info->firmware_name, dev);
> + if (ret) {
> + dev_err(dev, "Error requesting firmware %s\n", image_info-
> >firmware_name);
> + goto err_put_region;
> + }
> +
> + /* todo: parse the header and leave results in image_info */
> + ret = fpga_region_parser_header(fw->data, fw->size, image_info);
> + if (ret)
> + goto err_put_region;
> +
> /*
> * In some cases, we already have a list of bridges in the
> * fpga region struct. Or we don't have any bridges.
> @@ -434,6 +524,7 @@ int fpga_region_program_fpga(struct fpga_region
> *region,
> }
>
> ret = fpga_mgr_load(region->mgr, image_info);
> + release_firmware(fw);
> if (ret) {
> pr_err("failed to load fpga image\n");
> goto err_put_br;
> --
> 2.1.4
Powered by blists - more mailing lists