Re: [PATCH v2 2/6] fpga: dfl: Move the DFH definitions

matthew.gerlach@xxxxxxxxxxxxxxx · Fri, 30 Sep 2022 07:09:57 -0700 (PDT)

On Sat, 24 Sep 2022, Tom Rix wrote:


On 9/23/22 5:17 AM, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
From: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@xxxxxxxxxxxxxxx>

Moving the DFH register offset and register definitions from
drivers/fpga/dfl.h to include/linux/dfl.h. These definitions
need to be accessed by dfl drivers that are outside of
drivers/fpga.

This comment does not match what is done.

A move, a change in names and the introduction new defines.

I am not sure if moving these #defines is the best approach, the later use of 
the in the uart with FIELD_GET's i think should be wrapped as functions and 
these functions exported rather than the #defines.

So split this patch and justify why #defines are added to the user's 
includes.

Tom

I agree the original intent "diverged in v2".  I will minimize moving and 
make helper functions to simplify things.




Signed-off-by: Basheer Ahmed Muddebihal 
<basheer.ahmed.muddebihal@xxxxxxxxxxxxxxx>
Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
---
v2: remove extra space in commit
     use uniform number of digits in constants
     don't change copyright date because of removed content
---
  drivers/fpga/dfl-afu-main.c |  4 ++--
  drivers/fpga/dfl.c          |  2 +-
  drivers/fpga/dfl.h          | 20 +-------------------
  include/linux/dfl.h         | 33 ++++++++++++++++++++++++++++++++-
  4 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 7f621e96d3b8..c26961ee33db 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -468,8 +468,8 @@ afu_id_show(struct device *dev, struct device_attribute 
*attr, char *buf)
  		return -EBUSY;
  	}
  -	guidl = readq(base + GUID_L);
-	guidh = readq(base + GUID_H);
+	guidl = readq(base + DFH_GUID_L);
+	guidh = readq(base + DFH_GUID_H);
  	mutex_unlock(&pdata->lock);
    	return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n", guidh, guidl);
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index b9aae85ba930..1132f3c10440 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1163,7 +1163,7 @@ static int parse_feature_fiu(struct 
build_feature_devs_info *binfo,
  	 * find and parse FIU's child AFU via its NEXT_AFU register.
  	 * please note that only Port has valid NEXT_AFU pointer per spec.
  	 */
-	v = readq(binfo->ioaddr + NEXT_AFU);
+	v = readq(binfo->ioaddr + DFH_NEXT_AFU);
    	offset = FIELD_GET(NEXT_AFU_NEXT_DFH_OFST, v);
  	if (offset)
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 06cfcd5e84bb..e620fcb02b5a 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -17,6 +17,7 @@
  #include <linux/bitfield.h>
  #include <linux/cdev.h>
  #include <linux/delay.h>
+#include <linux/dfl.h>
  #include <linux/eventfd.h>
  #include <linux/fs.h>
  #include <linux/interrupt.h>
@@ -53,28 +54,9 @@
  #define PORT_FEATURE_ID_UINT		0x12
  #define PORT_FEATURE_ID_STP		0x13
  -/*
- * Device Feature Header Register Set
- *
- * For FIUs, they all have DFH + GUID + NEXT_AFU as common header 
registers.
- * For AFUs, they have DFH + GUID as common header registers.
- * For private features, they only have DFH register as common header.
- */
-#define DFH			0x0
-#define GUID_L			0x8
-#define GUID_H			0x10
-#define NEXT_AFU		0x18
-
-#define DFH_SIZE		0x8
-
  /* Device Feature Header Register Bitfield */
-#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID 
*/
  #define DFH_ID_FIU_FME		0
  #define DFH_ID_FIU_PORT		1
-#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision 
*/
-#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH 
*/
-#define DFH_EOL			BIT_ULL(40)		/* End of 
list */
-#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
  #define DFH_TYPE_AFU		1
  #define DFH_TYPE_PRIVATE	3
  #define DFH_TYPE_FIU		4
diff --git a/include/linux/dfl.h b/include/linux/dfl.h
index 431636a0dc78..33d167c53b09 100644
--- a/include/linux/dfl.h
+++ b/include/linux/dfl.h
@@ -2,7 +2,7 @@
  /*
   * Header file for DFL driver and device API
   *
- * Copyright (C) 2020 Intel Corporation, Inc.
+ * Copyright (C) 2020-2022 Intel Corporation, Inc.
   */
    #ifndef __LINUX_DFL_H
@@ -11,6 +11,37 @@
  #include <linux/device.h>
  #include <linux/mod_devicetable.h>
  +/*
+ * Device Feature Header Register Set
+ *
+ * For FIUs, they all have DFH + GUID + NEXT_AFU as common header 
registers.
+ * For AFUs, they have DFH + GUID as common header registers.
+ * For private features, they only have DFH register as common header.
+ */
+#define DFH			0x00
+#define DFH_GUID_L		0x08
+#define DFH_GUID_H		0x10
+#define DFH_NEXT_AFU		0x18
+
+/*
+ * DFHv1 Register Offset definitons
+ * In DHFv1, DFH + GUID + CSR_START + CSR_SIZE_GROUP + PARAM_HDR + 
PARAM_DATA
+ * as common header registers
+ */
+#define DFHv1_CSR_ADDR		0x18  /* CSR Register start address 
*/
+#define DFHv1_CSR_SIZE_GRP	0x20  /* Size of Reg Block and Group/tag */
+#define DFHv1_PARAM_HDR		0x28  /* Optional First Param header 
*/
+#define DFHv1_PARAM_DATA	0x08  /* Offset of Param data from Param 
header */
+
+#define DFH_SIZE		0x08
+
+/* Device Feature Header Register Bitfield */
+#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID 
*/
+#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision 
*/
+#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH 
*/
+#define DFH_EOL			BIT_ULL(40)		/* End of 
list */
+#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
+
  /**
   * enum dfl_id_type - define the DFL FIU types
   */








