[PATCH v2 5/6] fpga: dfl: parse the location of the feature's registers from DFHv1

matthew.gerlach@linux.intel.com posted 6 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v2 5/6] fpga: dfl: parse the location of the feature's registers from DFHv1
Posted by matthew.gerlach@linux.intel.com 1 year, 5 months ago
From: Matthew Gerlach <matthew.gerlach@intel.com>

The location of a feature's registers is explicitly
described in DFHv1 and can be relative to the base of the DFHv1
or an absolute address.  Parse the location and pass the information
to DFL driver.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
---
v2: Introduced in v2.
---
 drivers/fpga/dfl.c  | 26 +++++++++++++++++++++++++-
 drivers/fpga/dfl.h  |  4 ++++
 include/linux/dfl.h |  4 ++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index dfd3f563c92d..6fb4f30f93cf 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -381,6 +381,8 @@ dfl_dev_add(struct dfl_feature_platform_data *pdata,
 	ddev->feature_id = feature->id;
 	ddev->revision = feature->revision;
 	ddev->cdev = pdata->dfl_cdev;
+	ddev->csr_start = feature->csr_start;
+	ddev->csr_size = feature->csr_size;
 
 	/* add mmio resource */
 	parent_res = &pdev->resource[feature->resource_index];
@@ -708,18 +710,25 @@ struct build_feature_devs_info {
  * struct dfl_feature_info - sub feature info collected during feature dev build
  *
  * @fid: id of this sub feature.
+ * @revision: revision of this sub feature
+ * @dfh_version: version of Device Feature Header (DFH)
  * @mmio_res: mmio resource of this sub feature.
  * @ioaddr: mapped base address of mmio resource.
  * @node: node in sub_features linked list.
+ * @csr_start: DFHv1 start of feature registers
+ * @csr_size: DFHv1 size of feature registers
  * @irq_base: start of irq index in this sub feature.
  * @nr_irqs: number of irqs of this sub feature.
  */
 struct dfl_feature_info {
 	u16 fid;
 	u8 revision;
+	u8 dfh_version;
 	struct resource mmio_res;
 	void __iomem *ioaddr;
 	struct list_head node;
+	resource_size_t csr_start;
+	resource_size_t csr_size;
 	unsigned int irq_base;
 	unsigned int nr_irqs;
 };
@@ -797,6 +806,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
 		feature->dev = fdev;
 		feature->id = finfo->fid;
 		feature->revision = finfo->revision;
+		feature->csr_start = finfo->csr_start;
+		feature->csr_size = finfo->csr_size;
 
 		/*
 		 * the FIU header feature has some fundamental functions (sriov
@@ -1054,6 +1065,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 {
 	unsigned int irq_base, nr_irqs;
 	struct dfl_feature_info *finfo;
+	u8 dfh_version = 0;
 	u8 revision = 0;
 	int ret;
 	u64 v;
@@ -1061,7 +1073,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 	if (fid != FEATURE_ID_AFU) {
 		v = readq(binfo->ioaddr + ofst);
 		revision = FIELD_GET(DFH_REVISION, v);
-
+		dfh_version = FIELD_GET(DFH_VERSION, v);
 		/* read feature size and id if inputs are invalid */
 		size = size ? size : feature_size(v);
 		fid = fid ? fid : feature_id(v);
@@ -1080,12 +1092,24 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 
 	finfo->fid = fid;
 	finfo->revision = revision;
+	finfo->dfh_version = dfh_version;
 	finfo->mmio_res.start = binfo->start + ofst;
 	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
 	finfo->mmio_res.flags = IORESOURCE_MEM;
 	finfo->irq_base = irq_base;
 	finfo->nr_irqs = nr_irqs;
 
+	if (dfh_version == 1) {
+		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_ADDR);
+		if (v & DFHv1_CSR_ADDR_REL)
+			finfo->csr_start = FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
+		else
+			finfo->csr_start = binfo->start + ofst + FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
+
+		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_SIZE_GRP);
+		finfo->csr_size = FIELD_GET(DFHv1_CSR_SIZE_GRP_SIZE, v);
+	}
+
 	list_add_tail(&finfo->node, &binfo->sub_features);
 	binfo->feature_num++;
 
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index e620fcb02b5a..64cedd00dca4 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -217,6 +217,8 @@ struct dfl_feature_irq_ctx {
  *		    this index is used to find its mmio resource from the
  *		    feature dev (platform device)'s resources.
  * @ioaddr: mapped mmio resource address.
+ * @csr_start: DFHv1 start of feature registers
+ * @csr_size: DFHv1 size of feature registers
  * @irq_ctx: interrupt context list.
  * @nr_irqs: number of interrupt contexts.
  * @ops: ops of this sub feature.
@@ -229,6 +231,8 @@ struct dfl_feature {
 	u8 revision;
 	int resource_index;
 	void __iomem *ioaddr;
+	resource_size_t csr_start;
+	resource_size_t csr_size;
 	struct dfl_feature_irq_ctx *irq_ctx;
 	unsigned int nr_irqs;
 	const struct dfl_feature_ops *ops;
diff --git a/include/linux/dfl.h b/include/linux/dfl.h
index 33e21c360671..7d74ef8d1d20 100644
--- a/include/linux/dfl.h
+++ b/include/linux/dfl.h
@@ -84,6 +84,8 @@ enum dfl_id_type {
  * @type: type of DFL FIU of the device. See enum dfl_id_type.
  * @feature_id: feature identifier local to its DFL FIU type.
  * @mmio_res: mmio resource of this dfl device.
+ * @csr_start: DFHv1 start of feature registers
+ * @csr_size: DFHv1 size of feature registers
  * @irqs: list of Linux IRQ numbers of this dfl device.
  * @num_irqs: number of IRQs supported by this dfl device.
  * @cdev: pointer to DFL FPGA container device this dfl device belongs to.
@@ -96,6 +98,8 @@ struct dfl_device {
 	u16 feature_id;
 	u8 revision;
 	struct resource mmio_res;
+	resource_size_t csr_start;
+	resource_size_t csr_size;
 	int *irqs;
 	unsigned int num_irqs;
 	struct dfl_fpga_cdev *cdev;
-- 
2.25.1
Re: [PATCH v2 5/6] fpga: dfl: parse the location of the feature's registers from DFHv1
Posted by Xu Yilun 1 year, 4 months ago
On 2022-09-23 at 05:17:44 -0700, matthew.gerlach@linux.intel.com wrote:
> From: Matthew Gerlach <matthew.gerlach@intel.com>
> 
> The location of a feature's registers is explicitly
> described in DFHv1 and can be relative to the base of the DFHv1
> or an absolute address.  Parse the location and pass the information
> to DFL driver.
> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> ---
> v2: Introduced in v2.
> ---
>  drivers/fpga/dfl.c  | 26 +++++++++++++++++++++++++-
>  drivers/fpga/dfl.h  |  4 ++++
>  include/linux/dfl.h |  4 ++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index dfd3f563c92d..6fb4f30f93cf 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -381,6 +381,8 @@ dfl_dev_add(struct dfl_feature_platform_data *pdata,
>  	ddev->feature_id = feature->id;
>  	ddev->revision = feature->revision;
>  	ddev->cdev = pdata->dfl_cdev;
> +	ddev->csr_start = feature->csr_start;
> +	ddev->csr_size = feature->csr_size;
>  
>  	/* add mmio resource */
>  	parent_res = &pdev->resource[feature->resource_index];
> @@ -708,18 +710,25 @@ struct build_feature_devs_info {
>   * struct dfl_feature_info - sub feature info collected during feature dev build
>   *
>   * @fid: id of this sub feature.
> + * @revision: revision of this sub feature
> + * @dfh_version: version of Device Feature Header (DFH)
>   * @mmio_res: mmio resource of this sub feature.
>   * @ioaddr: mapped base address of mmio resource.
>   * @node: node in sub_features linked list.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irq_base: start of irq index in this sub feature.
>   * @nr_irqs: number of irqs of this sub feature.
>   */
>  struct dfl_feature_info {
>  	u16 fid;
>  	u8 revision;
> +	u8 dfh_version;
>  	struct resource mmio_res;
>  	void __iomem *ioaddr;
>  	struct list_head node;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;
>  	unsigned int irq_base;
>  	unsigned int nr_irqs;
>  };
> @@ -797,6 +806,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  		feature->dev = fdev;
>  		feature->id = finfo->fid;
>  		feature->revision = finfo->revision;
> +		feature->csr_start = finfo->csr_start;
> +		feature->csr_size = finfo->csr_size;
>  
>  		/*
>  		 * the FIU header feature has some fundamental functions (sriov
> @@ -1054,6 +1065,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  {
>  	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
> +	u8 dfh_version = 0;
>  	u8 revision = 0;
>  	int ret;
>  	u64 v;
> @@ -1061,7 +1073,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	if (fid != FEATURE_ID_AFU) {
>  		v = readq(binfo->ioaddr + ofst);
>  		revision = FIELD_GET(DFH_REVISION, v);
> -
> +		dfh_version = FIELD_GET(DFH_VERSION, v);
>  		/* read feature size and id if inputs are invalid */
>  		size = size ? size : feature_size(v);
>  		fid = fid ? fid : feature_id(v);
> @@ -1080,12 +1092,24 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  
>  	finfo->fid = fid;
>  	finfo->revision = revision;
> +	finfo->dfh_version = dfh_version;
>  	finfo->mmio_res.start = binfo->start + ofst;
>  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>  	finfo->mmio_res.flags = IORESOURCE_MEM;
>  	finfo->irq_base = irq_base;
>  	finfo->nr_irqs = nr_irqs;
>  
> +	if (dfh_version == 1) {
> +		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_ADDR);
> +		if (v & DFHv1_CSR_ADDR_REL)
> +			finfo->csr_start = FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
> +		else
> +			finfo->csr_start = binfo->start + ofst + FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
> +
> +		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_SIZE_GRP);
> +		finfo->csr_size = FIELD_GET(DFHv1_CSR_SIZE_GRP_SIZE, v);
> +	}
> +
>  	list_add_tail(&finfo->node, &binfo->sub_features);
>  	binfo->feature_num++;
>  
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index e620fcb02b5a..64cedd00dca4 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -217,6 +217,8 @@ struct dfl_feature_irq_ctx {
>   *		    this index is used to find its mmio resource from the
>   *		    feature dev (platform device)'s resources.
>   * @ioaddr: mapped mmio resource address.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irq_ctx: interrupt context list.
>   * @nr_irqs: number of interrupt contexts.
>   * @ops: ops of this sub feature.
> @@ -229,6 +231,8 @@ struct dfl_feature {
>  	u8 revision;
>  	int resource_index;
>  	void __iomem *ioaddr;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;
>  	struct dfl_feature_irq_ctx *irq_ctx;
>  	unsigned int nr_irqs;
>  	const struct dfl_feature_ops *ops;
> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> index 33e21c360671..7d74ef8d1d20 100644
> --- a/include/linux/dfl.h
> +++ b/include/linux/dfl.h
> @@ -84,6 +84,8 @@ enum dfl_id_type {
>   * @type: type of DFL FIU of the device. See enum dfl_id_type.
>   * @feature_id: feature identifier local to its DFL FIU type.
>   * @mmio_res: mmio resource of this dfl device.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irqs: list of Linux IRQ numbers of this dfl device.
>   * @num_irqs: number of IRQs supported by this dfl device.
>   * @cdev: pointer to DFL FPGA container device this dfl device belongs to.
> @@ -96,6 +98,8 @@ struct dfl_device {
>  	u16 feature_id;
>  	u8 revision;
>  	struct resource mmio_res;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;

I think these register start & size info could be stored in
struct resource mmio_res. This is the generic way for the driver to
understand the register layout of the device. And it makes the dfl
driver code easier to be understood by other domains reviewers.

Thanks,
Yilun

>  	int *irqs;
>  	unsigned int num_irqs;
>  	struct dfl_fpga_cdev *cdev;
> -- 
> 2.25.1
>
Re: [PATCH v2 5/6] fpga: dfl: parse the location of the feature's registers from DFHv1
Posted by Muddebihal, Basheer Ahmed 1 year, 5 months ago

On 9/23/2022 5:17 AM, matthew.gerlach@linux.intel.com wrote:
> From: Matthew Gerlach <matthew.gerlach@intel.com>
> 
> The location of a feature's registers is explicitly
> described in DFHv1 and can be relative to the base of the DFHv1
> or an absolute address.  Parse the location and pass the information
> to DFL driver.
> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> ---
> v2: Introduced in v2.
> ---
>  drivers/fpga/dfl.c  | 26 +++++++++++++++++++++++++-
>  drivers/fpga/dfl.h  |  4 ++++
>  include/linux/dfl.h |  4 ++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index dfd3f563c92d..6fb4f30f93cf 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -381,6 +381,8 @@ dfl_dev_add(struct dfl_feature_platform_data *pdata,
>  	ddev->feature_id = feature->id;
>  	ddev->revision = feature->revision;
>  	ddev->cdev = pdata->dfl_cdev;
> +	ddev->csr_start = feature->csr_start;
> +	ddev->csr_size = feature->csr_size;
>  
>  	/* add mmio resource */
>  	parent_res = &pdev->resource[feature->resource_index];
> @@ -708,18 +710,25 @@ struct build_feature_devs_info {
>   * struct dfl_feature_info - sub feature info collected during feature dev build
>   *
>   * @fid: id of this sub feature.
> + * @revision: revision of this sub feature
> + * @dfh_version: version of Device Feature Header (DFH)
>   * @mmio_res: mmio resource of this sub feature.
>   * @ioaddr: mapped base address of mmio resource.
>   * @node: node in sub_features linked list.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irq_base: start of irq index in this sub feature.
>   * @nr_irqs: number of irqs of this sub feature.
>   */
>  struct dfl_feature_info {
>  	u16 fid;
>  	u8 revision;
> +	u8 dfh_version;
>  	struct resource mmio_res;
>  	void __iomem *ioaddr;
>  	struct list_head node;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;
>  	unsigned int irq_base;
>  	unsigned int nr_irqs;
>  };
> @@ -797,6 +806,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  		feature->dev = fdev;
>  		feature->id = finfo->fid;
>  		feature->revision = finfo->revision;> +		feature->csr_start = finfo->csr_start;
> +		feature->csr_size = finfo->csr_size;
>  
>  		/*
>  		 * the FIU header feature has some fundamental functions (sriov
> @@ -1054,6 +1065,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  {
>  	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
> +	u8 dfh_version = 0;
>  	u8 revision = 0;
>  	int ret;
>  	u64 v;
> @@ -1061,7 +1073,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	if (fid != FEATURE_ID_AFU) {
>  		v = readq(binfo->ioaddr + ofst);
>  		revision = FIELD_GET(DFH_REVISION, v);
> -
> +		dfh_version = FIELD_GET(DFH_VERSION, v);
>  		/* read feature size and id if inputs are invalid */
>  		size = size ? size : feature_size(v);
>  		fid = fid ? fid : feature_id(v);
> @@ -1080,12 +1092,24 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  
>  	finfo->fid = fid;
>  	finfo->revision = revision;
> +	finfo->dfh_version = dfh_version;
>  	finfo->mmio_res.start = binfo->start + ofst;
>  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>  	finfo->mmio_res.flags = IORESOURCE_MEM;
>  	finfo->irq_base = irq_base;
>  	finfo->nr_irqs = nr_irqs;
>  
> +	if (dfh_version == 1) {
> +		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_ADDR);
> +		if (v & DFHv1_CSR_ADDR_REL)
> +			finfo->csr_start = FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
> +		else
> +			finfo->csr_start = binfo->start + ofst + FIELD_GET(DFHv1_CSR_ADDR_MASK, v);
> +
> +		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_SIZE_GRP);
> +		finfo->csr_size = FIELD_GET(DFHv1_CSR_SIZE_GRP_SIZE, v);
> +	}
> +
>  	list_add_tail(&finfo->node, &binfo->sub_features);
>  	binfo->feature_num++;
>  
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index e620fcb02b5a..64cedd00dca4 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -217,6 +217,8 @@ struct dfl_feature_irq_ctx {
>   *		    this index is used to find its mmio resource from the
>   *		    feature dev (platform device)'s resources.
>   * @ioaddr: mapped mmio resource address.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irq_ctx: interrupt context list.
>   * @nr_irqs: number of interrupt contexts.
>   * @ops: ops of this sub feature.
> @@ -229,6 +231,8 @@ struct dfl_feature {
>  	u8 revision;
>  	int resource_index;
>  	void __iomem *ioaddr;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;
>  	struct dfl_feature_irq_ctx *irq_ctx;
>  	unsigned int nr_irqs;
>  	const struct dfl_feature_ops *ops;
> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> index 33e21c360671..7d74ef8d1d20 100644
> --- a/include/linux/dfl.h
> +++ b/include/linux/dfl.h
> @@ -84,6 +84,8 @@ enum dfl_id_type {
>   * @type: type of DFL FIU of the device. See enum dfl_id_type.
>   * @feature_id: feature identifier local to its DFL FIU type.
>   * @mmio_res: mmio resource of this dfl device.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irqs: list of Linux IRQ numbers of this dfl device.
>   * @num_irqs: number of IRQs supported by this dfl device.
>   * @cdev: pointer to DFL FPGA container device this dfl device belongs to.
> @@ -96,6 +98,8 @@ struct dfl_device {
>  	u16 feature_id;
>  	u8 revision;
> +	u8 dfh_version;

This is already parsed as part of info dfl_feature_info, Can we add dfh_version to store it for further use ? 

>  	struct resource mmio_res;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;
>  	int *irqs;
>  	unsigned int num_irqs;
>  	struct dfl_fpga_cdev *cdev;
Re: [PATCH v2 5/6] fpga: dfl: parse the location of the feature's registers from DFHv1
Posted by Ilpo Järvinen 1 year, 5 months ago
On Fri, 23 Sep 2022, matthew.gerlach@linux.intel.com wrote:

> From: Matthew Gerlach <matthew.gerlach@intel.com>
> 
> The location of a feature's registers is explicitly
> described in DFHv1 and can be relative to the base of the DFHv1
> or an absolute address.  Parse the location and pass the information
> to DFL driver.
> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> ---
> v2: Introduced in v2.
> ---
>  drivers/fpga/dfl.c  | 26 +++++++++++++++++++++++++-
>  drivers/fpga/dfl.h  |  4 ++++
>  include/linux/dfl.h |  4 ++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index dfd3f563c92d..6fb4f30f93cf 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -381,6 +381,8 @@ dfl_dev_add(struct dfl_feature_platform_data *pdata,
>  	ddev->feature_id = feature->id;
>  	ddev->revision = feature->revision;
>  	ddev->cdev = pdata->dfl_cdev;
> +	ddev->csr_start = feature->csr_start;
> +	ddev->csr_size = feature->csr_size;
>  
>  	/* add mmio resource */
>  	parent_res = &pdev->resource[feature->resource_index];
> @@ -708,18 +710,25 @@ struct build_feature_devs_info {
>   * struct dfl_feature_info - sub feature info collected during feature dev build
>   *
>   * @fid: id of this sub feature.
> + * @revision: revision of this sub feature
> + * @dfh_version: version of Device Feature Header (DFH)
>   * @mmio_res: mmio resource of this sub feature.
>   * @ioaddr: mapped base address of mmio resource.
>   * @node: node in sub_features linked list.
> + * @csr_start: DFHv1 start of feature registers
> + * @csr_size: DFHv1 size of feature registers
>   * @irq_base: start of irq index in this sub feature.
>   * @nr_irqs: number of irqs of this sub feature.
>   */
>  struct dfl_feature_info {
>  	u16 fid;
>  	u8 revision;
> +	u8 dfh_version;
>  	struct resource mmio_res;
>  	void __iomem *ioaddr;
>  	struct list_head node;
> +	resource_size_t csr_start;
> +	resource_size_t csr_size;
>  	unsigned int irq_base;
>  	unsigned int nr_irqs;
>  };
> @@ -797,6 +806,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  		feature->dev = fdev;
>  		feature->id = finfo->fid;
>  		feature->revision = finfo->revision;
> +		feature->csr_start = finfo->csr_start;
> +		feature->csr_size = finfo->csr_size;
>  
>  		/*
>  		 * the FIU header feature has some fundamental functions (sriov
> @@ -1054,6 +1065,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  {
>  	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
> +	u8 dfh_version = 0;
>  	u8 revision = 0;
>  	int ret;
>  	u64 v;
> @@ -1061,7 +1073,7 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	if (fid != FEATURE_ID_AFU) {
>  		v = readq(binfo->ioaddr + ofst);
>  		revision = FIELD_GET(DFH_REVISION, v);
> -
> +		dfh_version = FIELD_GET(DFH_VERSION, v);
>  		/* read feature size and id if inputs are invalid */
>  		size = size ? size : feature_size(v);
>  		fid = fid ? fid : feature_id(v);
> @@ -1080,12 +1092,24 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  
>  	finfo->fid = fid;
>  	finfo->revision = revision;
> +	finfo->dfh_version = dfh_version;
>  	finfo->mmio_res.start = binfo->start + ofst;
>  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>  	finfo->mmio_res.flags = IORESOURCE_MEM;
>  	finfo->irq_base = irq_base;
>  	finfo->nr_irqs = nr_irqs;

Ordering here seems slightly odd. If finfo would be built before calling
parse_feature_irqs(), it could be passed into there and there would be no 
need to:
- read version for second time
- pass irq_base & nr_irqs as pointer parameters

-- 
 i.