diff mbox series

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

Message ID 20220621092833.1057408-3-nava.manne@xilinx.com (mailing list archive)
State New
Headers show
Series Adds status interface for zynqmp-fpga | expand

Commit Message

Nava kishore Manne June 21, 2022, 9:28 a.m. UTC
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>
---
Changes for v2:
              - None.

 drivers/firmware/xilinx/zynqmp.c     | 33 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 14 ++++++++++++
 2 files changed, 47 insertions(+)

Comments

kernel test robot June 21, 2022, 3:21 p.m. UTC | #1
Hi Nava,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.19-rc3 next-20220621]
[cannot apply to xilinx-xlnx/master]
[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-manager-change-status-api-prototype-don-t-use-older/20220621-183524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 78ca55889a549a9a194c6ec666836329b774ab6d
config: riscv-randconfig-r042-20220619 (https://download.01.org/0day-ci/archive/20220621/202206212254.d7Dba7Nm-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 11.3.0
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/6b66792a9e4acd718d58c0e5a33ca2426837be87
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nava-kishore-Manne/fpga-manager-change-status-api-prototype-don-t-use-older/20220621-183524
        git checkout 6b66792a9e4acd718d58c0e5a33ca2426837be87
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/crypto/xilinx/

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/crypto/xilinx/zynqmp-aes-gcm.c:18:
>> 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
kernel test robot June 21, 2022, 3:21 p.m. UTC | #2
Hi Nava,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.19-rc3 next-20220621]
[cannot apply to xilinx-xlnx/master]
[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-manager-change-status-api-prototype-don-t-use-older/20220621-183524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 78ca55889a549a9a194c6ec666836329b774ab6d
config: hexagon-randconfig-r003-20220620 (https://download.01.org/0day-ci/archive/20220621/202206212247.2Ky8WLW2-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
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/6b66792a9e4acd718d58c0e5a33ca2426837be87
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nava-kishore-Manne/fpga-manager-change-status-api-prototype-don-t-use-older/20220621-183524
        git checkout 6b66792a9e4acd718d58c0e5a33ca2426837be87
        # 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/fpga/

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/fpga/zynqmp-fpga.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
Peter Korsgaard June 22, 2022, 12:14 p.m. UTC | #3
>>>>> "Nava" == Nava kishore Manne <nava.manne@xilinx.com> writes:

 > 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>
 > ---
 > Changes for v2:
 >               - None.

 >  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

An offset OR a length? That sounds odd.


 > + * @phys_address: Physical Address of the buffer
 > + * @readback_type: Type of fpga readback operation
 > + *                 0 - FPGA configuration register readback
 > + *                 1 - FPGA configuration data readback

readback_type is a boolean, so how about calling it `bool data` or
something like that?


 > + * @value: Value to read

what is the relation between phys_address and this value output
argument?

 > + *
 > + * 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,

You are adding PM_FPGA_READ_CONFIG_ defines, so how about using them,
E.G.

data ? PM_FPGA_READ_CONFIG_DATA : PM_FPGA_READ_CONFIG_REG
diff mbox series

Patch

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__ */