[PATCH v5 0/2] check feature type for DFL irq parsing

[PATCH v5 0/2] check feature type for DFL irq parsing

[Tianfei Zhang]

This patch set adds an extended usage of Feature ID for DFL.
Previously the feature IDs defined are unique, no matter
which feature type. But currently we want to extend its
usage to have a per-type feature ID space.

Patch 1 adds feature type checking before parse the irq info.
Patch 2 adds description about the Feature ID.

v5:
  - Fix documentation from Matthew's comment. 
v4:
  - Fix the git commit from Hao's comments.
  - Split documentation into another patch.
v3:
  - Remove "Fixes" in commit log with Hao's comment, this is a
    extension not a bug fix.
v2:
  - add DFL Feature ID Registry in documentation.

Tianfei zhang (2):
  fpga: dfl: check feature type before parse irq info
  Documentation: fpga: dfl: add description of Feature ID

 Documentation/fpga/dfl.rst | 10 ++++++++++
 drivers/fpga/dfl.c         | 38 ++++++++++++++++++++++----------------
 2 files changed, 32 insertions(+), 16 deletions(-)

-- 
2.26.2

[PATCH v5 1/2] fpga: dfl: check feature type before parse irq info

[Tianfei Zhang]

From: Tianfei zhang <tianfei.zhang@intel.com>

Previously the feature IDs defined are unique, no matter
which feature type. But currently we want to extend its
usage to have a per-type feature ID space, so this patch
adds feature type checking as well just before look into
feature ID for different features which have irq info.

Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
v4:
  - Fix the git commit from Hao's comments.
  - Split documentation into another patch.
v3:
  - Remove "Fixes" in commit log with Hao's comment, this is a
    extension not a bug fix.
v2:
  - add DFL Feature ID Registry in documentation.
---
 drivers/fpga/dfl.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 599bb21d86af..6bff39ff21a0 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -940,9 +940,12 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo,
 {
 	void __iomem *base = binfo->ioaddr + ofst;
 	unsigned int i, ibase, inr = 0;
+	enum dfl_id_type type;
 	int virq;
 	u64 v;
 
+	type = feature_dev_id_type(binfo->feature_dev);
+
 	/*
 	 * Ideally DFL framework should only read info from DFL header, but
 	 * current version DFL only provides mmio resources information for
@@ -957,22 +960,25 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo,
 	 * code will be added. But in order to be compatible to old version
 	 * DFL, the driver may still fall back to these quirks.
 	 */
-	switch (fid) {
-	case PORT_FEATURE_ID_UINT:
-		v = readq(base + PORT_UINT_CAP);
-		ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
-		inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
-		break;
-	case PORT_FEATURE_ID_ERROR:
-		v = readq(base + PORT_ERROR_CAP);
-		ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
-		inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
-		break;
-	case FME_FEATURE_ID_GLOBAL_ERR:
-		v = readq(base + FME_ERROR_CAP);
-		ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
-		inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
-		break;
+	if (type == PORT_ID) {
+		switch (fid) {
+		case PORT_FEATURE_ID_UINT:
+			v = readq(base + PORT_UINT_CAP);
+			ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
+			inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
+			break;
+		case PORT_FEATURE_ID_ERROR:
+			v = readq(base + PORT_ERROR_CAP);
+			ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
+			inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
+			break;
+		}
+	} else if (type == FME_ID) {
+		if (fid == FME_FEATURE_ID_GLOBAL_ERR) {
+			v = readq(base + FME_ERROR_CAP);
+			ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
+			inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
+		}
 	}
 
 	if (!inr) {
-- 
2.26.2

Re: [PATCH v5 1/2] fpga: dfl: check feature type before parse irq info

[matthew.gerlach]

On Wed, 23 Mar 2022, Tianfei Zhang wrote:

> From: Tianfei zhang <tianfei.zhang@intel.com>

Hi Tianfei,

Your fix is much better than mine.


>
> Previously the feature IDs defined are unique, no matter
> which feature type. But currently we want to extend its
> usage to have a per-type feature ID space, so this patch
> adds feature type checking as well just before look into
> feature ID for different features which have irq info.
>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> ---
> v4:
>  - Fix the git commit from Hao's comments.
>  - Split documentation into another patch.
> v3:
>  - Remove "Fixes" in commit log with Hao's comment, this is a
>    extension not a bug fix.
> v2:
>  - add DFL Feature ID Registry in documentation.
> ---
> drivers/fpga/dfl.c | 38 ++++++++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 599bb21d86af..6bff39ff21a0 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -940,9 +940,12 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo,
> {
> 	void __iomem *base = binfo->ioaddr + ofst;
> 	unsigned int i, ibase, inr = 0;
> +	enum dfl_id_type type;
> 	int virq;
> 	u64 v;
>
> +	type = feature_dev_id_type(binfo->feature_dev);
> +
> 	/*
> 	 * Ideally DFL framework should only read info from DFL header, but
> 	 * current version DFL only provides mmio resources information for
> @@ -957,22 +960,25 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo,
> 	 * code will be added. But in order to be compatible to old version
> 	 * DFL, the driver may still fall back to these quirks.
> 	 */
> -	switch (fid) {
> -	case PORT_FEATURE_ID_UINT:
> -		v = readq(base + PORT_UINT_CAP);
> -		ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> -		inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> -		break;
> -	case PORT_FEATURE_ID_ERROR:
> -		v = readq(base + PORT_ERROR_CAP);
> -		ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> -		inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> -		break;
> -	case FME_FEATURE_ID_GLOBAL_ERR:
> -		v = readq(base + FME_ERROR_CAP);
> -		ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> -		inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> -		break;
> +	if (type == PORT_ID) {
> +		switch (fid) {
> +		case PORT_FEATURE_ID_UINT:
> +			v = readq(base + PORT_UINT_CAP);
> +			ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> +			inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> +			break;
> +		case PORT_FEATURE_ID_ERROR:
> +			v = readq(base + PORT_ERROR_CAP);
> +			ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> +			inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> +			break;
> +		}
> +	} else if (type == FME_ID) {
> +		if (fid == FME_FEATURE_ID_GLOBAL_ERR) {
> +			v = readq(base + FME_ERROR_CAP);
> +			ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> +			inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> +		}
> 	}
>
> 	if (!inr) {
> -- 
> 2.26.2
>
>

[PATCH v5 2/2] Documentation: fpga: dfl: add description of Feature ID

[Tianfei Zhang]

From: Tianfei zhang <tianfei.zhang@intel.com>

This patch adds the description and registration of Feature ID
in documentation.

Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
v5: fix documentation from Matthew's comment.
---
 Documentation/fpga/dfl.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index ef9eec71f6f3..231fe7a4d099 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -502,6 +502,16 @@ Developer only needs to provide a sub feature driver with matched feature id.
 FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
 could be a reference.
 
+Individual DFL drivers are bound DFL devices based on Feature Type and Feature ID.
+The definition of Feature Type and Feature ID can be found:
+
+https://github.com/OPAE/linux-dfl-feature-id/blob/master/dfl-feature-ids.rst
+
+If you want to add a new feature ID for FPGA DFL feature device, you must submit a pull
+request to register a feature ID for DFL. Here is the DFL Feature ID Registry:
+
+https://github.com/OPAE/linux-dfl-feature-id
+
 Location of DFLs on a PCI Device
 ================================
 The original method for finding a DFL on a PCI device assumed the start of the
-- 
2.26.2

Re: [PATCH v5 2/2] Documentation: fpga: dfl: add description of Feature ID

[Bagas Sanjaya]

On 23/03/22 15.51, Tianfei Zhang wrote:
> +Individual DFL drivers are bound DFL devices based on Feature Type and Feature ID.
> +The definition of Feature Type and Feature ID can be found:
> +
> +https://github.com/OPAE/linux-dfl-feature-id/blob/master/dfl-feature-ids.rst
> +

This doesn't answer "What is Feature Type and Feature ID?" question.
I would like to see the answer and the feature list above in the kernel
documentation.

> +If you want to add a new feature ID for FPGA DFL feature device, you must submit a pull
> +request to register a feature ID for DFL. Here is the DFL Feature ID Registry:
> +
> +https://github.com/OPAE/linux-dfl-feature-id
> +

Please explain, in this document, the PR procedure regarding feature ID
registration.

-- 
An old man doll... just what I always wanted! - Clara

RE: [PATCH v5 2/2] Documentation: fpga: dfl: add description of Feature ID

[Zhang, Tianfei]

> -----Original Message-----
> From: Bagas Sanjaya <bagasdotme@gmail.com>
> Sent: Wednesday, March 23, 2022 8:12 PM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao <hao.wu@intel.com>;
> trix@redhat.com; mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>; linux-
> fpga@vger.kernel.org; linux-doc@vger.kernel.org
> Cc: corbet@lwn.net; rdunlap@infradead.org
> Subject: Re: [PATCH v5 2/2] Documentation: fpga: dfl: add description of
> Feature ID
> 
> On 23/03/22 15.51, Tianfei Zhang wrote:
> > +Individual DFL drivers are bound DFL devices based on Feature Type and
> Feature ID.
> > +The definition of Feature Type and Feature ID can be found:
> > +
> > +https://github.com/OPAE/linux-dfl-feature-id/blob/master/dfl-feature-
> > +ids.rst
> > +
> 
> This doesn't answer "What is Feature Type and Feature ID?" question.
> I would like to see the answer and the feature list above in the kernel
> documentation.

Feature Type is the device type of feature device, currently, we only support FME device and Port device now.
Feature ID means the ID of feature device. Individual DFL drivers are bound DFL devices based on Feature Type and Feature ID
after DFL enumeration. I think those detail information are in DFL specification, and it don't need put everything in kernel documentation.
And I think the feature list maintains by an extra git repository will be better, and the users want to add a new feature ID must
submit a pull request for this repo to register the new ID.

> 
> > +If you want to add a new feature ID for FPGA DFL feature device, you
> > +must submit a pull request to register a feature ID for DFL. Here is the DFL
> Feature ID Registry:
> > +
> > +https://github.com/OPAE/linux-dfl-feature-id
> > +
> 
> Please explain, in this document, the PR procedure regarding feature ID
> registration.

In this documentation, it has a chapter talk about the PR, pls see the " Partial Reconfiguration" chapter.
On DFL perspective, PR is a FME private feature device, the Feature Type is "0" (0 means FME), the feature ID is 0x5.
FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c) will be bound if it found a 
PR private feature device after DFL enumeration.

> 
> --
> An old man doll... just what I always wanted! - Clara