[PATCH 0/3]Adds status interface for zynqmp-fpga

[PATCH 0/3]Adds status interface for zynqmp-fpga

[Nava kishore Manne]

Adds status interface for zynqmp-fpga, It's a read only
interface which allows the user to get the PL status.

This patch series also update the below status values for
fpga-manager.
 -Device Initialization error.
 -Device internal signal error.
 -All I/Os are placed in High-Z state.
 -Device start-up sequence error.
 -Firmware error.

For more details refer the ug570.
https://docs.xilinx.com/v/u/en-US/ug570-ultrascale-configuration

Nava kishore Manne (3):
  fpga: mgr: Update the status for fpga-manager
  firmware: xilinx: Add pm api function for PL readback
  fpga: zynqmp-fpga: Adds status interface

 drivers/firmware/xilinx/zynqmp.c     | 33 ++++++++++++++++++
 drivers/fpga/fpga-mgr.c              | 13 +++++++
 drivers/fpga/zynqmp-fpga.c           | 52 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 14 ++++++++
 include/linux/fpga/fpga-mgr.h        |  6 ++++
 5 files changed, 118 insertions(+)

-- 
2.25.1

[PATCH 0/3]Adds status interface for zynqmp-fpga

[Nava kishore Manne]

Adds status interface for zynqmp-fpga, It's a read only
interface which allows the user to get the PL status.

This patch series also update the below status values for
fpga-manager.
 -Device Initialization error.
 -Device internal signal error.
 -All I/Os are placed in High-Z state.
 -Device start-up sequence error.
 -Firmware error.

For more details refer the ug570.
https://docs.xilinx.com/v/u/en-US/ug570-ultrascale-configuration

Nava kishore Manne (3):
  fpga: mgr: Update the status for fpga-manager
  firmware: xilinx: Add pm api function for PL readback
  fpga: zynqmp-fpga: Adds status interface

 drivers/firmware/xilinx/zynqmp.c     | 33 ++++++++++++++++++
 drivers/fpga/fpga-mgr.c              | 13 +++++++
 drivers/fpga/zynqmp-fpga.c           | 52 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 14 ++++++++
 include/linux/fpga/fpga-mgr.h        |  6 ++++
 5 files changed, 118 insertions(+)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] fpga: mgr: Update the status for fpga-manager

[Nava kishore Manne]

This patch update the below status values for
fpga-manager.
	-Device Initialization error.
	-Device internal signal error.
	-All I/Os are placed in High-Z state.
	-Device start-up sequence error.
	-Firmware error.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
 drivers/fpga/fpga-mgr.c       | 13 +++++++++++++
 include/linux/fpga/fpga-mgr.h |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index a3595ecc3f79..be693fd531ea 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -475,6 +475,19 @@ static ssize_t status_show(struct device *dev,
 		len += sprintf(buf + len, "reconfig IP protocol error\n");
 	if (status & FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR)
 		len += sprintf(buf + len, "reconfig fifo overflow error\n");
+	if (status & FPGA_MGR_STATUS_SECURITY_ERR)
+		len += sprintf(buf + len, "reconfig security error\n");
+	if (status & FPGA_MGR_STATUS_DEVICE_INIT_ERR)
+		len += sprintf(buf + len, "Device Initialization error\n");
+	if (status & FPGA_MGR_STATUS_SIGNAL_ERR)
+		len += sprintf(buf + len, "Device internal signal error\n");
+	if (status & FPGA_MGR_STATUS_HIGH_Z_STATE_ERR)
+		len += sprintf(buf + len,
+			       "All I/Os are placed in High-Z state\n");
+	if (status & FPGA_MGR_STATUS_EOS_ERR)
+		len += sprintf(buf + len, "Device start-up sequence error\n");
+	if (status & FPGA_MGR_STATUS_FIRMWARE_REQ_ERR)
+		len += sprintf(buf + len, "firmware error\n");
 
 	return len;
 }
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 0f9468771bb9..91accba14ba2 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -172,6 +172,12 @@ struct fpga_manager_ops {
 #define FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR	BIT(2)
 #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR		BIT(3)
 #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR	BIT(4)
+#define FPGA_MGR_STATUS_SECURITY_ERR		BIT(5)
+#define FPGA_MGR_STATUS_DEVICE_INIT_ERR		BIT(6)
+#define FPGA_MGR_STATUS_SIGNAL_ERR		BIT(7)
+#define FPGA_MGR_STATUS_HIGH_Z_STATE_ERR	BIT(8)
+#define FPGA_MGR_STATUS_EOS_ERR			BIT(9)
+#define FPGA_MGR_STATUS_FIRMWARE_REQ_ERR	BIT(10)
 
 /**
  * struct fpga_manager - fpga manager structure
-- 
2.25.1

[PATCH 1/3] fpga: mgr: Update the status for fpga-manager

[Nava kishore Manne]

This patch update the below status values for
fpga-manager.
	-Device Initialization error.
	-Device internal signal error.
	-All I/Os are placed in High-Z state.
	-Device start-up sequence error.
	-Firmware error.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
 drivers/fpga/fpga-mgr.c       | 13 +++++++++++++
 include/linux/fpga/fpga-mgr.h |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index a3595ecc3f79..be693fd531ea 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -475,6 +475,19 @@ static ssize_t status_show(struct device *dev,
 		len += sprintf(buf + len, "reconfig IP protocol error\n");
 	if (status & FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR)
 		len += sprintf(buf + len, "reconfig fifo overflow error\n");
+	if (status & FPGA_MGR_STATUS_SECURITY_ERR)
+		len += sprintf(buf + len, "reconfig security error\n");
+	if (status & FPGA_MGR_STATUS_DEVICE_INIT_ERR)
+		len += sprintf(buf + len, "Device Initialization error\n");
+	if (status & FPGA_MGR_STATUS_SIGNAL_ERR)
+		len += sprintf(buf + len, "Device internal signal error\n");
+	if (status & FPGA_MGR_STATUS_HIGH_Z_STATE_ERR)
+		len += sprintf(buf + len,
+			       "All I/Os are placed in High-Z state\n");
+	if (status & FPGA_MGR_STATUS_EOS_ERR)
+		len += sprintf(buf + len, "Device start-up sequence error\n");
+	if (status & FPGA_MGR_STATUS_FIRMWARE_REQ_ERR)
+		len += sprintf(buf + len, "firmware error\n");
 
 	return len;
 }
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 0f9468771bb9..91accba14ba2 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -172,6 +172,12 @@ struct fpga_manager_ops {
 #define FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR	BIT(2)
 #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR		BIT(3)
 #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR	BIT(4)
+#define FPGA_MGR_STATUS_SECURITY_ERR		BIT(5)
+#define FPGA_MGR_STATUS_DEVICE_INIT_ERR		BIT(6)
+#define FPGA_MGR_STATUS_SIGNAL_ERR		BIT(7)
+#define FPGA_MGR_STATUS_HIGH_Z_STATE_ERR	BIT(8)
+#define FPGA_MGR_STATUS_EOS_ERR			BIT(9)
+#define FPGA_MGR_STATUS_FIRMWARE_REQ_ERR	BIT(10)
 
 /**
  * struct fpga_manager - fpga manager structure
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] firmware: xilinx: Add pm api function for PL readback

[Nava kishore Manne]

Adds PM API for performing PL configuration readback.
It provides an interface to the pmufw to readback the
FPGA configuration registers as well as configuration
data.

For more detailed info related to the configuration
registers and configuration data refer ug570.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
 drivers/firmware/xilinx/zynqmp.c     | 33 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 14 ++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 7977a494a651..40b99299b662 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -927,6 +927,39 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_fpga_read - Perform the fpga configuration readback
+ * @reg_numframes: Configuration register offset (or) Number of frames to read
+ * @phys_address: Physical Address of the buffer
+ * @readback_type: Type of fpga readback operation
+ *                 0 - FPGA configuration register readback
+ *                 1 - FPGA configuration data readback
+ * @value: Value to read
+ *
+ * This function provides access to xilfpga library to perform
+ * fpga configuration readback.
+ *
+ * Return:	Returns status, either success or error+reason
+ */
+int zynqmp_pm_fpga_read(const u32 reg_numframes, const phys_addr_t phys_address,
+			bool readback_type, u32 *value)
+{
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	int ret;
+
+	if (!value)
+		return -EINVAL;
+
+	ret = zynqmp_pm_invoke_fn(PM_FPGA_READ, reg_numframes,
+				  lower_32_bits(phys_address),
+				  upper_32_bits(phys_address), readback_type,
+				  ret_payload);
+	*value = ret_payload[1];
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_read);
+
 /**
  * zynqmp_pm_pinctrl_request - Request Pin from firmware
  * @pin: Pin number to request
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 1ec73d5352c3..7dc4981345dc 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -61,6 +61,10 @@
 #define PM_LOAD_PDI	0x701
 #define PDI_SRC_DDR	0xF
 
+/* FPGA readback type */
+#define PM_FPGA_READ_CONFIG_REG		0x0U
+#define PM_FPGA_READ_CONFIG_DATA	0x1U
+
 /*
  * Firmware FPGA Manager flags
  * XILINX_ZYNQMP_PM_FPGA_FULL:	FPGA full reconfiguration
@@ -116,6 +120,7 @@ enum pm_api_id {
 	PM_CLOCK_GETRATE = 42,
 	PM_CLOCK_SETPARENT = 43,
 	PM_CLOCK_GETPARENT = 44,
+	PM_FPGA_READ = 46,
 	PM_SECURE_AES = 47,
 	PM_FEATURE_CHECK = 63,
 };
@@ -468,6 +473,8 @@ int zynqmp_pm_feature(const u32 api_id);
 int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id);
 int zynqmp_pm_set_feature_config(enum pm_feature_config_id id, u32 value);
 int zynqmp_pm_get_feature_config(enum pm_feature_config_id id, u32 *payload);
+int zynqmp_pm_fpga_read(const u32 reg_numframes, const phys_addr_t phys_address,
+			bool readback_type, u32 *value);
 #else
 static inline int zynqmp_pm_get_api_version(u32 *version)
 {
@@ -733,6 +740,13 @@ static inline int zynqmp_pm_get_feature_config(enum pm_feature_config_id id,
 {
 	return -ENODEV;
 }
+
+static int zynqmp_pm_fpga_read(const u32 reg_numframes,
+			       const phys_addr_t phys_address,
+			       bool readback_type, u32 *value);
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __FIRMWARE_ZYNQMP_H__ */
-- 
2.25.1

[PATCH 2/3] firmware: xilinx: Add pm api function for PL readback

[Nava kishore Manne]

Adds PM API for performing PL configuration readback.
It provides an interface to the pmufw to readback the
FPGA configuration registers as well as configuration
data.

For more detailed info related to the configuration
registers and configuration data refer ug570.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
 drivers/firmware/xilinx/zynqmp.c     | 33 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 14 ++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 7977a494a651..40b99299b662 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -927,6 +927,39 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_fpga_read - Perform the fpga configuration readback
+ * @reg_numframes: Configuration register offset (or) Number of frames to read
+ * @phys_address: Physical Address of the buffer
+ * @readback_type: Type of fpga readback operation
+ *                 0 - FPGA configuration register readback
+ *                 1 - FPGA configuration data readback
+ * @value: Value to read
+ *
+ * This function provides access to xilfpga library to perform
+ * fpga configuration readback.
+ *
+ * Return:	Returns status, either success or error+reason
+ */
+int zynqmp_pm_fpga_read(const u32 reg_numframes, const phys_addr_t phys_address,
+			bool readback_type, u32 *value)
+{
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	int ret;
+
+	if (!value)
+		return -EINVAL;
+
+	ret = zynqmp_pm_invoke_fn(PM_FPGA_READ, reg_numframes,
+				  lower_32_bits(phys_address),
+				  upper_32_bits(phys_address), readback_type,
+				  ret_payload);
+	*value = ret_payload[1];
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_read);
+
 /**
  * zynqmp_pm_pinctrl_request - Request Pin from firmware
  * @pin: Pin number to request
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 1ec73d5352c3..7dc4981345dc 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -61,6 +61,10 @@
 #define PM_LOAD_PDI	0x701
 #define PDI_SRC_DDR	0xF
 
+/* FPGA readback type */
+#define PM_FPGA_READ_CONFIG_REG		0x0U
+#define PM_FPGA_READ_CONFIG_DATA	0x1U
+
 /*
  * Firmware FPGA Manager flags
  * XILINX_ZYNQMP_PM_FPGA_FULL:	FPGA full reconfiguration
@@ -116,6 +120,7 @@ enum pm_api_id {
 	PM_CLOCK_GETRATE = 42,
 	PM_CLOCK_SETPARENT = 43,
 	PM_CLOCK_GETPARENT = 44,
+	PM_FPGA_READ = 46,
 	PM_SECURE_AES = 47,
 	PM_FEATURE_CHECK = 63,
 };
@@ -468,6 +473,8 @@ int zynqmp_pm_feature(const u32 api_id);
 int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id);
 int zynqmp_pm_set_feature_config(enum pm_feature_config_id id, u32 value);
 int zynqmp_pm_get_feature_config(enum pm_feature_config_id id, u32 *payload);
+int zynqmp_pm_fpga_read(const u32 reg_numframes, const phys_addr_t phys_address,
+			bool readback_type, u32 *value);
 #else
 static inline int zynqmp_pm_get_api_version(u32 *version)
 {
@@ -733,6 +740,13 @@ static inline int zynqmp_pm_get_feature_config(enum pm_feature_config_id id,
 {
 	return -ENODEV;
 }
+
+static int zynqmp_pm_fpga_read(const u32 reg_numframes,
+			       const phys_addr_t phys_address,
+			       bool readback_type, u32 *value);
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __FIRMWARE_ZYNQMP_H__ */
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] firmware: xilinx: Add pm api function for PL readback

[kernel test robot]

Hi Nava,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220524]
[also build test ERROR on v5.18]
[cannot apply to xilinx-xlnx/master soc/for-next linus/master v5.18 v5.18-rc7 v5.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
base:    09ce5091ff971cdbfd67ad84dc561ea27f10d67a
config: hexagon-randconfig-r011-20220524 (https://download.01.org/0day-ci/archive/20220524/202205242059.n4mNNqLG-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 10c9ecce9f6096e18222a331c5e7d085bd813f75)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
        git checkout cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/spi/spi-zynqmp-gqspi.c:13:
>> include/linux/firmware/xlnx-zynqmp.h:747:1: error: expected identifier or '('
   {
   ^
   1 error generated.


vim +747 include/linux/firmware/xlnx-zynqmp.h

   743	
   744	static int zynqmp_pm_fpga_read(const u32 reg_numframes,
   745				       const phys_addr_t phys_address,
   746				       bool readback_type, u32 *value);
 > 747	{
   748		return -ENODEV;
   749	}
   750	#endif
   751	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/3] firmware: xilinx: Add pm api function for PL readback

[kernel test robot]

Hi Nava,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220524]
[also build test ERROR on v5.18]
[cannot apply to xilinx-xlnx/master soc/for-next linus/master v5.18 v5.18-rc7 v5.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
base:    09ce5091ff971cdbfd67ad84dc561ea27f10d67a
config: hexagon-randconfig-r011-20220524 (https://download.01.org/0day-ci/archive/20220524/202205242059.n4mNNqLG-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 10c9ecce9f6096e18222a331c5e7d085bd813f75)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
        git checkout cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/spi/spi-zynqmp-gqspi.c:13:
>> include/linux/firmware/xlnx-zynqmp.h:747:1: error: expected identifier or '('
   {
   ^
   1 error generated.


vim +747 include/linux/firmware/xlnx-zynqmp.h

   743	
   744	static int zynqmp_pm_fpga_read(const u32 reg_numframes,
   745				       const phys_addr_t phys_address,
   746				       bool readback_type, u32 *value);
 > 747	{
   748		return -ENODEV;
   749	}
   750	#endif
   751	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] firmware: xilinx: Add pm api function for PL readback

[kernel test robot]

Hi Nava,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220524]
[also build test ERROR on v5.18]
[cannot apply to xilinx-xlnx/master soc/for-next linus/master v5.18 v5.18-rc7 v5.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
base:    09ce5091ff971cdbfd67ad84dc561ea27f10d67a
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20220524/202205242222.Rq35goGC-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
        git checkout cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/spi/spi-zynqmp-gqspi.c:13:
>> include/linux/firmware/xlnx-zynqmp.h:747:1: error: expected identifier or '(' before '{' token
     747 | {
         | ^
   include/linux/firmware/xlnx-zynqmp.h:744:12: warning: 'zynqmp_pm_fpga_read' declared 'static' but never defined [-Wunused-function]
     744 | static int zynqmp_pm_fpga_read(const u32 reg_numframes,
         |            ^~~~~~~~~~~~~~~~~~~


vim +747 include/linux/firmware/xlnx-zynqmp.h

   743	
   744	static int zynqmp_pm_fpga_read(const u32 reg_numframes,
   745				       const phys_addr_t phys_address,
   746				       bool readback_type, u32 *value);
 > 747	{
   748		return -ENODEV;
   749	}
   750	#endif
   751	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/3] firmware: xilinx: Add pm api function for PL readback

[kernel test robot]

Hi Nava,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220524]
[also build test ERROR on v5.18]
[cannot apply to xilinx-xlnx/master soc/for-next linus/master v5.18 v5.18-rc7 v5.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
base:    09ce5091ff971cdbfd67ad84dc561ea27f10d67a
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20220524/202205242222.Rq35goGC-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nava-kishore-Manne/fpga-mgr-Update-the-status-for-fpga-manager/20220524-184838
        git checkout cc20fc570e528f1bc378aa2e979e7e2ee7f52863
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/spi/spi-zynqmp-gqspi.c:13:
>> include/linux/firmware/xlnx-zynqmp.h:747:1: error: expected identifier or '(' before '{' token
     747 | {
         | ^
   include/linux/firmware/xlnx-zynqmp.h:744:12: warning: 'zynqmp_pm_fpga_read' declared 'static' but never defined [-Wunused-function]
     744 | static int zynqmp_pm_fpga_read(const u32 reg_numframes,
         |            ^~~~~~~~~~~~~~~~~~~


vim +747 include/linux/firmware/xlnx-zynqmp.h

   743	
   744	static int zynqmp_pm_fpga_read(const u32 reg_numframes,
   745				       const phys_addr_t phys_address,
   746				       bool readback_type, u32 *value);
 > 747	{
   748		return -ENODEV;
   749	}
   750	#endif
   751	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] fpga: zynqmp-fpga: Adds status interface

[Nava kishore Manne]

Adds status interface for zynqmp-fpga, It's a read only
interface which allows the user to get the PL status.

Usage:
To read the PL configuration status
        cat /sys/class/fpga_manager/<fpga>/status

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
 drivers/fpga/zynqmp-fpga.c | 52 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
index c60f20949c47..07c7b7326726 100644
--- a/drivers/fpga/zynqmp-fpga.c
+++ b/drivers/fpga/zynqmp-fpga.c
@@ -14,6 +14,19 @@
 
 /* Constant Definitions */
 #define IXR_FPGA_DONE_MASK	BIT(3)
+#define READ_DMA_SIZE		256U
+
+/* Error Register */
+#define IXR_FPGA_ERR_CRC_ERR		BIT(0)
+#define IXR_FPGA_ERR_SECURITY_ERR	BIT(16)
+
+/* Signal Status Register. For details refer ug570 */
+#define IXR_FPGA_END_OF_STARTUP		BIT(4)
+#define IXR_FPGA_GST_CFG_B		BIT(5)
+#define IXR_FPGA_INIT_B_INTERNAL	BIT(11)
+#define IXR_FPGA_DONE_INTERNAL_SIGNAL	BIT(13)
+
+#define IXR_FPGA_CONFIG_STAT_OFFSET	7U
 
 /**
  * struct zynqmp_fpga_priv - Private data structure
@@ -77,8 +90,47 @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
 	return FPGA_MGR_STATE_UNKNOWN;
 }
 
+static u64 zynqmp_fpga_ops_status(struct fpga_manager *mgr)
+{
+	unsigned int *buf, reg_val;
+	dma_addr_t dma_addr;
+	u64 status = 0;
+	int ret;
+
+	buf = dma_alloc_coherent(mgr->dev.parent, READ_DMA_SIZE,
+				 &dma_addr, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = zynqmp_pm_fpga_read(IXR_FPGA_CONFIG_STAT_OFFSET, dma_addr,
+				  PM_FPGA_READ_CONFIG_REG, &reg_val);
+	if (ret) {
+		status = FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
+		goto free_dmabuf;
+	}
+
+	if (reg_val & IXR_FPGA_ERR_CRC_ERR)
+		status |= FPGA_MGR_STATUS_CRC_ERR;
+	if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
+		status |= FPGA_MGR_STATUS_SECURITY_ERR;
+	if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
+		status |= FPGA_MGR_STATUS_DEVICE_INIT_ERR;
+	if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
+		status |= FPGA_MGR_STATUS_SIGNAL_ERR;
+	if (!(reg_val & IXR_FPGA_GST_CFG_B))
+		status |= FPGA_MGR_STATUS_HIGH_Z_STATE_ERR;
+	if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
+		status |= FPGA_MGR_STATUS_EOS_ERR;
+
+free_dmabuf:
+	dma_free_coherent(mgr->dev.parent, READ_DMA_SIZE, buf, dma_addr);
+
+	return status;
+}
+
 static const struct fpga_manager_ops zynqmp_fpga_ops = {
 	.state = zynqmp_fpga_ops_state,
+	.status = zynqmp_fpga_ops_status,
 	.write_init = zynqmp_fpga_ops_write_init,
 	.write = zynqmp_fpga_ops_write,
 };
-- 
2.25.1

[PATCH 3/3] fpga: zynqmp-fpga: Adds status interface

[Nava kishore Manne]

Adds status interface for zynqmp-fpga, It's a read only
interface which allows the user to get the PL status.

Usage:
To read the PL configuration status
        cat /sys/class/fpga_manager/<fpga>/status

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
 drivers/fpga/zynqmp-fpga.c | 52 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
index c60f20949c47..07c7b7326726 100644
--- a/drivers/fpga/zynqmp-fpga.c
+++ b/drivers/fpga/zynqmp-fpga.c
@@ -14,6 +14,19 @@
 
 /* Constant Definitions */
 #define IXR_FPGA_DONE_MASK	BIT(3)
+#define READ_DMA_SIZE		256U
+
+/* Error Register */
+#define IXR_FPGA_ERR_CRC_ERR		BIT(0)
+#define IXR_FPGA_ERR_SECURITY_ERR	BIT(16)
+
+/* Signal Status Register. For details refer ug570 */
+#define IXR_FPGA_END_OF_STARTUP		BIT(4)
+#define IXR_FPGA_GST_CFG_B		BIT(5)
+#define IXR_FPGA_INIT_B_INTERNAL	BIT(11)
+#define IXR_FPGA_DONE_INTERNAL_SIGNAL	BIT(13)
+
+#define IXR_FPGA_CONFIG_STAT_OFFSET	7U
 
 /**
  * struct zynqmp_fpga_priv - Private data structure
@@ -77,8 +90,47 @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
 	return FPGA_MGR_STATE_UNKNOWN;
 }
 
+static u64 zynqmp_fpga_ops_status(struct fpga_manager *mgr)
+{
+	unsigned int *buf, reg_val;
+	dma_addr_t dma_addr;
+	u64 status = 0;
+	int ret;
+
+	buf = dma_alloc_coherent(mgr->dev.parent, READ_DMA_SIZE,
+				 &dma_addr, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = zynqmp_pm_fpga_read(IXR_FPGA_CONFIG_STAT_OFFSET, dma_addr,
+				  PM_FPGA_READ_CONFIG_REG, &reg_val);
+	if (ret) {
+		status = FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
+		goto free_dmabuf;
+	}
+
+	if (reg_val & IXR_FPGA_ERR_CRC_ERR)
+		status |= FPGA_MGR_STATUS_CRC_ERR;
+	if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
+		status |= FPGA_MGR_STATUS_SECURITY_ERR;
+	if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
+		status |= FPGA_MGR_STATUS_DEVICE_INIT_ERR;
+	if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
+		status |= FPGA_MGR_STATUS_SIGNAL_ERR;
+	if (!(reg_val & IXR_FPGA_GST_CFG_B))
+		status |= FPGA_MGR_STATUS_HIGH_Z_STATE_ERR;
+	if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
+		status |= FPGA_MGR_STATUS_EOS_ERR;
+
+free_dmabuf:
+	dma_free_coherent(mgr->dev.parent, READ_DMA_SIZE, buf, dma_addr);
+
+	return status;
+}
+
 static const struct fpga_manager_ops zynqmp_fpga_ops = {
 	.state = zynqmp_fpga_ops_state,
+	.status = zynqmp_fpga_ops_status,
 	.write_init = zynqmp_fpga_ops_write_init,
 	.write = zynqmp_fpga_ops_write,
 };
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] fpga: zynqmp-fpga: Adds status interface

[Xu Yilun]

On Tue, May 24, 2022 at 03:17:45PM +0530, Nava kishore Manne wrote:
> Adds status interface for zynqmp-fpga, It's a read only
> interface which allows the user to get the PL status.
> 
> Usage:
> To read the PL configuration status
>         cat /sys/class/fpga_manager/<fpga>/status
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
>  drivers/fpga/zynqmp-fpga.c | 52 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> index c60f20949c47..07c7b7326726 100644
> --- a/drivers/fpga/zynqmp-fpga.c
> +++ b/drivers/fpga/zynqmp-fpga.c
> @@ -14,6 +14,19 @@
>  
>  /* Constant Definitions */
>  #define IXR_FPGA_DONE_MASK	BIT(3)
> +#define READ_DMA_SIZE		256U
> +
> +/* Error Register */
> +#define IXR_FPGA_ERR_CRC_ERR		BIT(0)
> +#define IXR_FPGA_ERR_SECURITY_ERR	BIT(16)
> +
> +/* Signal Status Register. For details refer ug570 */
> +#define IXR_FPGA_END_OF_STARTUP		BIT(4)
> +#define IXR_FPGA_GST_CFG_B		BIT(5)
> +#define IXR_FPGA_INIT_B_INTERNAL	BIT(11)
> +#define IXR_FPGA_DONE_INTERNAL_SIGNAL	BIT(13)
> +
> +#define IXR_FPGA_CONFIG_STAT_OFFSET	7U
>  
>  /**
>   * struct zynqmp_fpga_priv - Private data structure
> @@ -77,8 +90,47 @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
>  	return FPGA_MGR_STATE_UNKNOWN;
>  }
>  
> +static u64 zynqmp_fpga_ops_status(struct fpga_manager *mgr)
> +{
> +	unsigned int *buf, reg_val;
> +	dma_addr_t dma_addr;
> +	u64 status = 0;
> +	int ret;
> +
> +	buf = dma_alloc_coherent(mgr->dev.parent, READ_DMA_SIZE,
> +				 &dma_addr, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = zynqmp_pm_fpga_read(IXR_FPGA_CONFIG_STAT_OFFSET, dma_addr,
> +				  PM_FPGA_READ_CONFIG_REG, &reg_val);
> +	if (ret) {
> +		status = FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
> +		goto free_dmabuf;
> +	}
> +
> +	if (reg_val & IXR_FPGA_ERR_CRC_ERR)
> +		status |= FPGA_MGR_STATUS_CRC_ERR;
> +	if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
> +		status |= FPGA_MGR_STATUS_SECURITY_ERR;
> +	if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
> +		status |= FPGA_MGR_STATUS_DEVICE_INIT_ERR;
> +	if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
> +		status |= FPGA_MGR_STATUS_SIGNAL_ERR;
> +	if (!(reg_val & IXR_FPGA_GST_CFG_B))
> +		status |= FPGA_MGR_STATUS_HIGH_Z_STATE_ERR;
> +	if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
> +		status |= FPGA_MGR_STATUS_EOS_ERR;

I have concern about the status interface. Different vendors have
differnt error sets defined by Hardwares. If we always define the
new bits when we cannot find an exact 1:1 mapping. A 64 bits would
soon be used out. Also it's hard to understand the mixture of
different error sets.

I'd rather suggest that each driver define its own error reading
interface.

Thanks,
Yilun

> +
> +free_dmabuf:
> +	dma_free_coherent(mgr->dev.parent, READ_DMA_SIZE, buf, dma_addr);
> +
> +	return status;
> +}
> +
>  static const struct fpga_manager_ops zynqmp_fpga_ops = {
>  	.state = zynqmp_fpga_ops_state,
> +	.status = zynqmp_fpga_ops_status,
>  	.write_init = zynqmp_fpga_ops_write_init,
>  	.write = zynqmp_fpga_ops_write,
>  };
> -- 
> 2.25.1

Re: [PATCH 3/3] fpga: zynqmp-fpga: Adds status interface

[Xu Yilun]

On Tue, May 24, 2022 at 03:17:45PM +0530, Nava kishore Manne wrote:
> Adds status interface for zynqmp-fpga, It's a read only
> interface which allows the user to get the PL status.
> 
> Usage:
> To read the PL configuration status
>         cat /sys/class/fpga_manager/<fpga>/status
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
>  drivers/fpga/zynqmp-fpga.c | 52 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> index c60f20949c47..07c7b7326726 100644
> --- a/drivers/fpga/zynqmp-fpga.c
> +++ b/drivers/fpga/zynqmp-fpga.c
> @@ -14,6 +14,19 @@
>  
>  /* Constant Definitions */
>  #define IXR_FPGA_DONE_MASK	BIT(3)
> +#define READ_DMA_SIZE		256U
> +
> +/* Error Register */
> +#define IXR_FPGA_ERR_CRC_ERR		BIT(0)
> +#define IXR_FPGA_ERR_SECURITY_ERR	BIT(16)
> +
> +/* Signal Status Register. For details refer ug570 */
> +#define IXR_FPGA_END_OF_STARTUP		BIT(4)
> +#define IXR_FPGA_GST_CFG_B		BIT(5)
> +#define IXR_FPGA_INIT_B_INTERNAL	BIT(11)
> +#define IXR_FPGA_DONE_INTERNAL_SIGNAL	BIT(13)
> +
> +#define IXR_FPGA_CONFIG_STAT_OFFSET	7U
>  
>  /**
>   * struct zynqmp_fpga_priv - Private data structure
> @@ -77,8 +90,47 @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
>  	return FPGA_MGR_STATE_UNKNOWN;
>  }
>  
> +static u64 zynqmp_fpga_ops_status(struct fpga_manager *mgr)
> +{
> +	unsigned int *buf, reg_val;
> +	dma_addr_t dma_addr;
> +	u64 status = 0;
> +	int ret;
> +
> +	buf = dma_alloc_coherent(mgr->dev.parent, READ_DMA_SIZE,
> +				 &dma_addr, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = zynqmp_pm_fpga_read(IXR_FPGA_CONFIG_STAT_OFFSET, dma_addr,
> +				  PM_FPGA_READ_CONFIG_REG, &reg_val);
> +	if (ret) {
> +		status = FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
> +		goto free_dmabuf;
> +	}
> +
> +	if (reg_val & IXR_FPGA_ERR_CRC_ERR)
> +		status |= FPGA_MGR_STATUS_CRC_ERR;
> +	if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
> +		status |= FPGA_MGR_STATUS_SECURITY_ERR;
> +	if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
> +		status |= FPGA_MGR_STATUS_DEVICE_INIT_ERR;
> +	if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
> +		status |= FPGA_MGR_STATUS_SIGNAL_ERR;
> +	if (!(reg_val & IXR_FPGA_GST_CFG_B))
> +		status |= FPGA_MGR_STATUS_HIGH_Z_STATE_ERR;
> +	if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
> +		status |= FPGA_MGR_STATUS_EOS_ERR;

I have concern about the status interface. Different vendors have
differnt error sets defined by Hardwares. If we always define the
new bits when we cannot find an exact 1:1 mapping. A 64 bits would
soon be used out. Also it's hard to understand the mixture of
different error sets.

I'd rather suggest that each driver define its own error reading
interface.

Thanks,
Yilun

> +
> +free_dmabuf:
> +	dma_free_coherent(mgr->dev.parent, READ_DMA_SIZE, buf, dma_addr);
> +
> +	return status;
> +}
> +
>  static const struct fpga_manager_ops zynqmp_fpga_ops = {
>  	.state = zynqmp_fpga_ops_state,
> +	.status = zynqmp_fpga_ops_status,
>  	.write_init = zynqmp_fpga_ops_write_init,
>  	.write = zynqmp_fpga_ops_write,
>  };
> -- 
> 2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 3/3] fpga: zynqmp-fpga: Adds status interface

[Nava kishore Manne]

Hi Yilun,

	Please find my response inline.

> -----Original Message-----
> From: Xu Yilun <yilun.xu@intel.com>
> Sent: Saturday, May 28, 2022 8:51 PM
> To: Nava kishore Manne <navam@xilinx.com>
> Cc: Michal Simek <michals@xilinx.com>; mdf@kernel.org;
> hao.wu@intel.com; trix@redhat.com; gregkh@linuxfoundation.org; Ronak
> Jain <ronakj@xilinx.com>; Abhyuday Godhasara <agodhasa@xilinx.com>;
> Rajan Vaja <rajanv@xlnx.xilinx.com>; Sai Krishna Potthuri
> <lakshmis@xilinx.com>; Piyush Mehta <piyushm@xilinx.com>; Harsha
> Harsha <harshah@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-fpga@vger.kernel.org; git <git@xilinx.com>
> Subject: Re: [PATCH 3/3] fpga: zynqmp-fpga: Adds status interface
> 
> On Tue, May 24, 2022 at 03:17:45PM +0530, Nava kishore Manne wrote:
> > Adds status interface for zynqmp-fpga, It's a read only interface
> > which allows the user to get the PL status.
> >
> > Usage:
> > To read the PL configuration status
> >         cat /sys/class/fpga_manager/<fpga>/status
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > ---
> >  drivers/fpga/zynqmp-fpga.c | 52
> > ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> > index c60f20949c47..07c7b7326726 100644
> > --- a/drivers/fpga/zynqmp-fpga.c
> > +++ b/drivers/fpga/zynqmp-fpga.c
> > @@ -14,6 +14,19 @@
> >
> >  /* Constant Definitions */
> >  #define IXR_FPGA_DONE_MASK	BIT(3)
> > +#define READ_DMA_SIZE		256U
> > +
> > +/* Error Register */
> > +#define IXR_FPGA_ERR_CRC_ERR		BIT(0)
> > +#define IXR_FPGA_ERR_SECURITY_ERR	BIT(16)
> > +
> > +/* Signal Status Register. For details refer ug570 */
> > +#define IXR_FPGA_END_OF_STARTUP		BIT(4)
> > +#define IXR_FPGA_GST_CFG_B		BIT(5)
> > +#define IXR_FPGA_INIT_B_INTERNAL	BIT(11)
> > +#define IXR_FPGA_DONE_INTERNAL_SIGNAL	BIT(13)
> > +
> > +#define IXR_FPGA_CONFIG_STAT_OFFSET	7U
> >
> >  /**
> >   * struct zynqmp_fpga_priv - Private data structure @@ -77,8 +90,47
> > @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct
> fpga_manager *mgr)
> >  	return FPGA_MGR_STATE_UNKNOWN;
> >  }
> >
> > +static u64 zynqmp_fpga_ops_status(struct fpga_manager *mgr) {
> > +	unsigned int *buf, reg_val;
> > +	dma_addr_t dma_addr;
> > +	u64 status = 0;
> > +	int ret;
> > +
> > +	buf = dma_alloc_coherent(mgr->dev.parent, READ_DMA_SIZE,
> > +				 &dma_addr, GFP_KERNEL);
> > +	if (!buf)
> > +		return -ENOMEM;
> > +
> > +	ret = zynqmp_pm_fpga_read(IXR_FPGA_CONFIG_STAT_OFFSET,
> dma_addr,
> > +				  PM_FPGA_READ_CONFIG_REG, &reg_val);
> > +	if (ret) {
> > +		status = FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
> > +		goto free_dmabuf;
> > +	}
> > +
> > +	if (reg_val & IXR_FPGA_ERR_CRC_ERR)
> > +		status |= FPGA_MGR_STATUS_CRC_ERR;
> > +	if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
> > +		status |= FPGA_MGR_STATUS_SECURITY_ERR;
> > +	if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
> > +		status |= FPGA_MGR_STATUS_DEVICE_INIT_ERR;
> > +	if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
> > +		status |= FPGA_MGR_STATUS_SIGNAL_ERR;
> > +	if (!(reg_val & IXR_FPGA_GST_CFG_B))
> > +		status |= FPGA_MGR_STATUS_HIGH_Z_STATE_ERR;
> > +	if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
> > +		status |= FPGA_MGR_STATUS_EOS_ERR;
> 
> I have concern about the status interface. Different vendors have differnt
> error sets defined by Hardwares. If we always define the new bits when we
> cannot find an exact 1:1 mapping. A 64 bits would soon be used out. Also it's
> hard to understand the mixture of different error sets.
> 
> I'd rather suggest that each driver define its own error reading interface.
> 
I agree Ideally, the core file should contain only the generic stuff and each vendor has its own set of status messages.
So status related messages should be part of the vendor specific files(not in the core files).
Will update the zynqmp_fpga_ops_status() API to popup the status messages (for ZynqMP FPGA related status messages).

Regards,
Navakishore.

RE: [PATCH 3/3] fpga: zynqmp-fpga: Adds status interface

[Nava kishore Manne]

Hi Yilun,

	Please find my response inline.

> -----Original Message-----
> From: Xu Yilun <yilun.xu@intel.com>
> Sent: Saturday, May 28, 2022 8:51 PM
> To: Nava kishore Manne <navam@xilinx.com>
> Cc: Michal Simek <michals@xilinx.com>; mdf@kernel.org;
> hao.wu@intel.com; trix@redhat.com; gregkh@linuxfoundation.org; Ronak
> Jain <ronakj@xilinx.com>; Abhyuday Godhasara <agodhasa@xilinx.com>;
> Rajan Vaja <rajanv@xlnx.xilinx.com>; Sai Krishna Potthuri
> <lakshmis@xilinx.com>; Piyush Mehta <piyushm@xilinx.com>; Harsha
> Harsha <harshah@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-fpga@vger.kernel.org; git <git@xilinx.com>
> Subject: Re: [PATCH 3/3] fpga: zynqmp-fpga: Adds status interface
> 
> On Tue, May 24, 2022 at 03:17:45PM +0530, Nava kishore Manne wrote:
> > Adds status interface for zynqmp-fpga, It's a read only interface
> > which allows the user to get the PL status.
> >
> > Usage:
> > To read the PL configuration status
> >         cat /sys/class/fpga_manager/<fpga>/status
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > ---
> >  drivers/fpga/zynqmp-fpga.c | 52
> > ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> > index c60f20949c47..07c7b7326726 100644
> > --- a/drivers/fpga/zynqmp-fpga.c
> > +++ b/drivers/fpga/zynqmp-fpga.c
> > @@ -14,6 +14,19 @@
> >
> >  /* Constant Definitions */
> >  #define IXR_FPGA_DONE_MASK	BIT(3)
> > +#define READ_DMA_SIZE		256U
> > +
> > +/* Error Register */
> > +#define IXR_FPGA_ERR_CRC_ERR		BIT(0)
> > +#define IXR_FPGA_ERR_SECURITY_ERR	BIT(16)
> > +
> > +/* Signal Status Register. For details refer ug570 */
> > +#define IXR_FPGA_END_OF_STARTUP		BIT(4)
> > +#define IXR_FPGA_GST_CFG_B		BIT(5)
> > +#define IXR_FPGA_INIT_B_INTERNAL	BIT(11)
> > +#define IXR_FPGA_DONE_INTERNAL_SIGNAL	BIT(13)
> > +
> > +#define IXR_FPGA_CONFIG_STAT_OFFSET	7U
> >
> >  /**
> >   * struct zynqmp_fpga_priv - Private data structure @@ -77,8 +90,47
> > @@ static enum fpga_mgr_states zynqmp_fpga_ops_state(struct
> fpga_manager *mgr)
> >  	return FPGA_MGR_STATE_UNKNOWN;
> >  }
> >
> > +static u64 zynqmp_fpga_ops_status(struct fpga_manager *mgr) {
> > +	unsigned int *buf, reg_val;
> > +	dma_addr_t dma_addr;
> > +	u64 status = 0;
> > +	int ret;
> > +
> > +	buf = dma_alloc_coherent(mgr->dev.parent, READ_DMA_SIZE,
> > +				 &dma_addr, GFP_KERNEL);
> > +	if (!buf)
> > +		return -ENOMEM;
> > +
> > +	ret = zynqmp_pm_fpga_read(IXR_FPGA_CONFIG_STAT_OFFSET,
> dma_addr,
> > +				  PM_FPGA_READ_CONFIG_REG, &reg_val);
> > +	if (ret) {
> > +		status = FPGA_MGR_STATUS_FIRMWARE_REQ_ERR;
> > +		goto free_dmabuf;
> > +	}
> > +
> > +	if (reg_val & IXR_FPGA_ERR_CRC_ERR)
> > +		status |= FPGA_MGR_STATUS_CRC_ERR;
> > +	if (reg_val & IXR_FPGA_ERR_SECURITY_ERR)
> > +		status |= FPGA_MGR_STATUS_SECURITY_ERR;
> > +	if (!(reg_val & IXR_FPGA_INIT_B_INTERNAL))
> > +		status |= FPGA_MGR_STATUS_DEVICE_INIT_ERR;
> > +	if (!(reg_val & IXR_FPGA_DONE_INTERNAL_SIGNAL))
> > +		status |= FPGA_MGR_STATUS_SIGNAL_ERR;
> > +	if (!(reg_val & IXR_FPGA_GST_CFG_B))
> > +		status |= FPGA_MGR_STATUS_HIGH_Z_STATE_ERR;
> > +	if (!(reg_val & IXR_FPGA_END_OF_STARTUP))
> > +		status |= FPGA_MGR_STATUS_EOS_ERR;
> 
> I have concern about the status interface. Different vendors have differnt
> error sets defined by Hardwares. If we always define the new bits when we
> cannot find an exact 1:1 mapping. A 64 bits would soon be used out. Also it's
> hard to understand the mixture of different error sets.
> 
> I'd rather suggest that each driver define its own error reading interface.
> 
I agree Ideally, the core file should contain only the generic stuff and each vendor has its own set of status messages.
So status related messages should be part of the vendor specific files(not in the core files).
Will update the zynqmp_fpga_ops_status() API to popup the status messages (for ZynqMP FPGA related status messages).

Regards,
Navakishore.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel