[PATCH v4 0/3] Add Bitstream configuration support for ZynqMP

[PATCH v4 0/3] Add Bitstream configuration support for ZynqMP

[Nava kishore Manne]

Nava kishore Manne (3):
  firmware: xilinx: Add fpga API's
  dt-bindings: fpga: Add bindings for ZynqMP fpga driver
  fpga manager: Adding FPGA Manager support for Xilinx zynqmp

 .../bindings/fpga/xlnx,zynqmp-pcap-fpga.txt   |  25 +++
 drivers/firmware/xilinx/zynqmp.c              |  46 +++++
 drivers/fpga/Kconfig                          |   9 +
 drivers/fpga/Makefile                         |   1 +
 drivers/fpga/zynqmp-fpga.c                    | 159 ++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h          |  10 ++
 6 files changed, 250 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt
 create mode 100644 drivers/fpga/zynqmp-fpga.c

-- 
2.18.0

[PATCH v4 1/3] firmware: xilinx: Add fpga API's

[Nava kishore Manne]

This Patch Adds fpga API's to support the Bitstream loading
by using firmware interface.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v4:
		-None.

Chnages for v3:
		-Created patches on top of 5.0-rc5.
		 No functional changes.

Changes for v2:
		-Added Firmware FPGA Manager flags As suggested by
		 Moritz.

Changes for v1:
		-None.

Changes for RFC-V2:
		-New Patch

 drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 10 ++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 98f936125643..7159a90abc44 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -537,6 +537,50 @@ static int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset,
 	return ret;
 }
 
+/*
+ * zynqmp_pm_fpga_load - Perform the fpga load
+ * @address:	Address to write to
+ * @size:	pl bitstream size
+ * @flags:
+ *	BIT(0) - Bit-stream type.
+ *		 0 - Full Bitstream.
+ *		 1 - Partial Bitstream.
+ *
+ * This function provides access to pmufw. To transfer
+ * the required bitstream into PL.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int zynqmp_pm_fpga_load(const u64 address, const u32 size,
+			       const u32 flags)
+{
+	return zynqmp_pm_invoke_fn(PM_FPGA_LOAD, lower_32_bits(address),
+				   upper_32_bits(address), size, flags, NULL);
+}
+
+/**
+ * zynqmp_pm_fpga_get_status - Read value from PCAP status register
+ * @value: Value to read
+ *
+ * This function provides access to the xilfpga library to get
+ * the PCAP status
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int zynqmp_pm_fpga_get_status(u32 *value)
+{
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	int ret;
+
+	if (!value)
+		return -EINVAL;
+
+	ret = zynqmp_pm_invoke_fn(PM_FPGA_GET_STATUS, 0, 0, 0, 0, ret_payload);
+	*value = ret_payload[1];
+
+	return ret;
+}
+
 /**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *			       master has initialized its own power management
@@ -640,6 +684,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
 	.request_node = zynqmp_pm_request_node,
 	.release_node = zynqmp_pm_release_node,
 	.set_requirement = zynqmp_pm_set_requirement,
+	.fpga_load = zynqmp_pm_fpga_load,
+	.fpga_get_status = zynqmp_pm_fpga_get_status,
 };
 
 /**
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 642dab10f65d..4df226b6ab0f 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -48,6 +48,12 @@
 #define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
 #define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
 
+/*
+ * Firmware FPGA Manager flags
+ * XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration
+ */
+#define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
+
 enum pm_api_id {
 	PM_GET_API_VERSION = 1,
 	PM_REQUEST_NODE = 13,
@@ -56,6 +62,8 @@ enum pm_api_id {
 	PM_RESET_ASSERT = 17,
 	PM_RESET_GET_STATUS,
 	PM_PM_INIT_FINALIZE = 21,
+	PM_FPGA_LOAD = 22,
+	PM_FPGA_GET_STATUS,
 	PM_GET_CHIPID = 24,
 	PM_IOCTL = 34,
 	PM_QUERY_DATA,
@@ -258,6 +266,8 @@ struct zynqmp_pm_query_data {
 struct zynqmp_eemi_ops {
 	int (*get_api_version)(u32 *version);
 	int (*get_chipid)(u32 *idcode, u32 *version);
+	int (*fpga_load)(const u64 address, const u32 size, const u32 flags);
+	int (*fpga_get_status)(u32 *value);
 	int (*query_data)(struct zynqmp_pm_query_data qdata, u32 *out);
 	int (*clock_enable)(u32 clock_id);
 	int (*clock_disable)(u32 clock_id);
-- 
2.18.0

[PATCH v4 2/3] dt-bindings: fpga: Add bindings for ZynqMP fpga driver

[Nava kishore Manne]

Add documentation to describe Xilinx ZynqMP fpga driver
bindings.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Acked-by: Moritz Fischer <mdf@kernel.org>
---
 .../bindings/fpga/xlnx,zynqmp-pcap-fpga.txt   | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt

diff --git a/Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt b/Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt
new file mode 100644
index 000000000000..3052bf619dd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/xlnx,zynqmp-pcap-fpga.txt
@@ -0,0 +1,25 @@
+Devicetree bindings for Zynq Ultrascale MPSoC FPGA Manager.
+The ZynqMP SoC uses the PCAP (Processor configuration Port) to configure the
+Programmable Logic (PL). The configuration uses  the firmware interface.
+
+Required properties:
+- compatible: should contain "xlnx,zynqmp-pcap-fpga"
+
+Example for full FPGA configuration:
+
+	fpga-region0 {
+		compatible = "fpga-region";
+		fpga-mgr = <&zynqmp_pcap>;
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+	};
+
+	firmware {
+		zynqmp_firmware: zynqmp-firmware {
+			compatible = "xlnx,zynqmp-firmware";
+			method = "smc";
+			zynqmp_pcap: pcap {
+				compatible = "xlnx,zynqmp-pcap-fpga";
+			};
+		};
+	};
-- 
2.18.0

[PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Nava kishore Manne]

This patch adds FPGA Manager support for the Xilinx
ZynqMP chip.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v4:
		-Updated the Fpga Mgr registrations call's
		 to 5.0
		-Removed dma_set_mask_and_coherent() As the FW
		 supports only 32-bit address operations.

Changes for v3:
		-Created patches on top of 5.0-rc5.
		 No functional changes.

Changes for v2:
		-Fixed some minor coding issues as suggested by
		 Moritz

Changes for v1:
		-None.

Changes for RFC-V2:
		-Updated the Fpga Mgr registrations call's
		 to 4.18

 drivers/fpga/Kconfig       |   9 +++
 drivers/fpga/Makefile      |   1 +
 drivers/fpga/zynqmp-fpga.c | 159 +++++++++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+)
 create mode 100644 drivers/fpga/zynqmp-fpga.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index c20445b867ae..d892f3efcd76 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -204,4 +204,13 @@ config FPGA_DFL_PCI
 
 	  To compile this as a module, choose M here.
 
+config FPGA_MGR_ZYNQMP_FPGA
+	tristate "Xilinx ZynqMP FPGA"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	help
+	  FPGA manager driver support for Xilinx ZynqMP FPGAs.
+	  This driver uses the processor configuration port(PCAP)
+	  to configure the programmable logic(PL) through PS
+	  on ZynqMP SoC.
+
 endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index c0dd4c82fbdb..312b9371742f 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC)	+= stratix10-soc.o
 obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
 obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
+obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
 obj-$(CONFIG_ALTERA_PR_IP_CORE)         += altera-pr-ip-core.o
 obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)    += altera-pr-ip-core-plat.o
 
diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
new file mode 100644
index 000000000000..f6e35fe95adb
--- /dev/null
+++ b/drivers/fpga/zynqmp-fpga.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Xilinx, Inc.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/string.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+/* Constant Definitions */
+#define IXR_FPGA_DONE_MASK	0X00000008U
+
+/**
+ * struct zynqmp_fpga_priv - Private data structure
+ * @dev:	Device data structure
+ * @flags:	flags which is used to identify the bitfile type
+ */
+struct zynqmp_fpga_priv {
+	struct device *dev;
+	u32 flags;
+};
+
+static int zynqmp_fpga_ops_write_init(struct fpga_manager *mgr,
+				      struct fpga_image_info *info,
+				      const char *buf, size_t size)
+{
+	struct zynqmp_fpga_priv *priv;
+
+	priv = mgr->priv;
+	priv->flags = info->flags;
+
+	return 0;
+}
+
+static int zynqmp_fpga_ops_write(struct fpga_manager *mgr,
+				 const char *buf, size_t size)
+{
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+	struct zynqmp_fpga_priv *priv;
+	dma_addr_t dma_addr;
+	u32 eemi_flags = 0;
+	char *kbuf;
+	int ret;
+
+	if (!eemi_ops || !eemi_ops->fpga_load)
+		return -ENXIO;
+
+	priv = mgr->priv;
+
+	kbuf = dma_alloc_coherent(priv->dev, size, &dma_addr, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	memcpy(kbuf, buf, size);
+
+	wmb(); /* ensure all writes are done before initiate FW call */
+
+	if (priv->flags & FPGA_MGR_PARTIAL_RECONFIG)
+		eemi_flags |= XILINX_ZYNQMP_PM_FPGA_PARTIAL;
+
+	ret = eemi_ops->fpga_load(dma_addr, size, eemi_flags);
+
+	dma_free_coherent(priv->dev, size, kbuf, dma_addr);
+
+	return ret;
+}
+
+static int zynqmp_fpga_ops_write_complete(struct fpga_manager *mgr,
+					  struct fpga_image_info *info)
+{
+	return 0;
+}
+
+static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
+{
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+	u32 status;
+
+	if (!eemi_ops || !eemi_ops->fpga_get_status)
+		return FPGA_MGR_STATE_UNKNOWN;
+
+	eemi_ops->fpga_get_status(&status);
+	if (status & IXR_FPGA_DONE_MASK)
+		return FPGA_MGR_STATE_OPERATING;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static const struct fpga_manager_ops zynqmp_fpga_ops = {
+	.state = zynqmp_fpga_ops_state,
+	.write_init = zynqmp_fpga_ops_write_init,
+	.write = zynqmp_fpga_ops_write,
+	.write_complete = zynqmp_fpga_ops_write_complete,
+};
+
+static int zynqmp_fpga_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct zynqmp_fpga_priv *priv;
+	struct fpga_manager *mgr;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager",
+				   &zynqmp_fpga_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
+	if (ret) {
+		dev_err(dev, "unable to register FPGA manager");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int zynqmp_fpga_remove(struct platform_device *pdev)
+{
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
+
+	return 0;
+}
+
+static const struct of_device_id zynqmp_fpga_of_match[] = {
+	{ .compatible = "xlnx,zynqmp-pcap-fpga", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, zynqmp_fpga_of_match);
+
+static struct platform_driver zynqmp_fpga_driver = {
+	.probe = zynqmp_fpga_probe,
+	.remove = zynqmp_fpga_remove,
+	.driver = {
+		.name = "zynqmp_fpga_manager",
+		.of_match_table = of_match_ptr(zynqmp_fpga_of_match),
+	},
+};
+
+module_platform_driver(zynqmp_fpga_driver);
+
+MODULE_AUTHOR("Nava kishore Manne <navam@xilinx.com>");
+MODULE_DESCRIPTION("Xilinx ZynqMp FPGA Manager");
+MODULE_LICENSE("GPL");
-- 
2.18.0

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Moritz Fischer]

Hi Nava,

looks mostly good to me. One minor nit below:

On Tue, Apr 02, 2019 at 06:01:23PM +0530, Nava kishore Manne wrote:

[..]
> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> new file mode 100644
> index 000000000000..f6e35fe95adb
> --- /dev/null
> +++ b/drivers/fpga/zynqmp-fpga.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Xilinx, Inc.
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/string.h>
> +#include <linux/firmware/xlnx-zynqmp.h>
> +
> +/* Constant Definitions */
> +#define IXR_FPGA_DONE_MASK	0X00000008U

You could use the BIT(x) macro here.
> +
> +/**
> + * struct zynqmp_fpga_priv - Private data structure
> + * @dev:	Device data structure
> + * @flags:	flags which is used to identify the bitfile type
> + */
> +struct zynqmp_fpga_priv {
> +	struct device *dev;
> +	u32 flags;
> +};
> +

[..]

Reviewed-by: Moritz Fischer <mdf@kernel.org>

Thanks,

Moritz

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Alan Tull]

On Tue, Apr 2, 2019 at 7:32 AM Nava kishore Manne <nava.manne@xilinx.com> wrote:

Hi Nava,

Looks good.

>
> This patch adds FPGA Manager support for the Xilinx
> ZynqMP chip.
>
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
Acked-by: Alan Tull <atull@kernel.org>

> ---
> Changes for v4:
>                 -Updated the Fpga Mgr registrations call's
>                  to 5.0
>                 -Removed dma_set_mask_and_coherent() As the FW
>                  supports only 32-bit address operations.
>
> Changes for v3:
>                 -Created patches on top of 5.0-rc5.
>                  No functional changes.
>
> Changes for v2:
>                 -Fixed some minor coding issues as suggested by
>                  Moritz
>
> Changes for v1:
>                 -None.
>
> Changes for RFC-V2:
>                 -Updated the Fpga Mgr registrations call's
>                  to 4.18

Thanks,
Alan

RE: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Nava kishore Manne]

Hi Alan,

Thanks for look into it and providing the ACK.  
I got one minor comments from Moritz Fischer do you want me fix that issue now or I can fix it later as it’s a minor comment?
In which kernel version i can expect this driver changes??


Regards,
Navakishore.

> -----Original Message-----
> From: Alan Tull [mailto:atull@kernel.org]
> Sent: Tuesday, April 2, 2019 11:57 PM
> To: Nava kishore Manne <navam@xilinx.com>
> Cc: Moritz Fischer <mdf@kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark
> Rutland <mark.rutland@arm.com>; Michal Simek <michals@xilinx.com>; Rajan
> Vaja <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>; linux-
> fpga@vger.kernel.org; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> TREE BINDINGS <devicetree@vger.kernel.org>; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; linux-
> kernel <linux-kernel@vger.kernel.org>; kishore m
> <chinnikishore369@gmail.com>
> Subject: Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for
> Xilinx zynqmp
> 
> On Tue, Apr 2, 2019 at 7:32 AM Nava kishore Manne <nava.manne@xilinx.com>
> wrote:
> 
> Hi Nava,
> 
> Looks good.
> 
> >
> > This patch adds FPGA Manager support for the Xilinx ZynqMP chip.
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> Acked-by: Alan Tull <atull@kernel.org>
> 
> > ---
> > Changes for v4:
> >                 -Updated the Fpga Mgr registrations call's
> >                  to 5.0
> >                 -Removed dma_set_mask_and_coherent() As the FW
> >                  supports only 32-bit address operations.
> >
> > Changes for v3:
> >                 -Created patches on top of 5.0-rc5.
> >                  No functional changes.
> >
> > Changes for v2:
> >                 -Fixed some minor coding issues as suggested by
> >                  Moritz
> >
> > Changes for v1:
> >                 -None.
> >
> > Changes for RFC-V2:
> >                 -Updated the Fpga Mgr registrations call's
> >                  to 4.18
> 
> Thanks,
> Alan

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Alan Tull]

On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <navam@xilinx.com> wrote:
>
> Hi Alan,
>
> Thanks for look into it and providing the ACK.
> I got one minor comments from Moritz Fischer do you want me fix that issue now or I can fix it later as it’s a minor comment?

Please fix for Moritz comment.

> In which kernel version i can expect this driver changes??

Patch 2/3 and 3/3 are dependent on 1/3 which isn't a drivers/fpga
thing, it's drivers/firmware.

Alan

>
>
> Regards,
> Navakishore.
>
> > -----Original Message-----
> > From: Alan Tull [mailto:atull@kernel.org]
> > Sent: Tuesday, April 2, 2019 11:57 PM
> > To: Nava kishore Manne <navam@xilinx.com>
> > Cc: Moritz Fischer <mdf@kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark
> > Rutland <mark.rutland@arm.com>; Michal Simek <michals@xilinx.com>; Rajan
> > Vaja <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>; linux-
> > fpga@vger.kernel.org; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> > TREE BINDINGS <devicetree@vger.kernel.org>; moderated list:ARM/FREESCALE
> > IMX / MXC ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; linux-
> > kernel <linux-kernel@vger.kernel.org>; kishore m
> > <chinnikishore369@gmail.com>
> > Subject: Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for
> > Xilinx zynqmp
> >
> > On Tue, Apr 2, 2019 at 7:32 AM Nava kishore Manne <nava.manne@xilinx.com>
> > wrote:
> >
> > Hi Nava,
> >
> > Looks good.
> >
> > >
> > > This patch adds FPGA Manager support for the Xilinx ZynqMP chip.
> > >
> > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > Acked-by: Alan Tull <atull@kernel.org>
> >
> > > ---
> > > Changes for v4:
> > >                 -Updated the Fpga Mgr registrations call's
> > >                  to 5.0
> > >                 -Removed dma_set_mask_and_coherent() As the FW
> > >                  supports only 32-bit address operations.
> > >
> > > Changes for v3:
> > >                 -Created patches on top of 5.0-rc5.
> > >                  No functional changes.
> > >
> > > Changes for v2:
> > >                 -Fixed some minor coding issues as suggested by
> > >                  Moritz
> > >
> > > Changes for v1:
> > >                 -None.
> > >
> > > Changes for RFC-V2:
> > >                 -Updated the Fpga Mgr registrations call's
> > >                  to 4.18
> >
> > Thanks,
> > Alan

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Michal Simek]

On 08. 04. 19 16:17, Alan Tull wrote:
> On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <navam@xilinx.com> wrote:
>>
>> Hi Alan,
>>
>> Thanks for look into it and providing the ACK.
>> I got one minor comments from Moritz Fischer do you want me fix that issue now or I can fix it later as it’s a minor comment?
> 
> Please fix for Moritz comment.
> 
>> In which kernel version i can expect this driver changes??
> 
> Patch 2/3 and 3/3 are dependent on 1/3 which isn't a drivers/fpga
> thing, it's drivers/firmware.

I can take it via arm-soc guys if you are ok with that.
Just need to have your ack in commits.
We have done it like this several times, IIRC reset, nvmem.

Thanks,
Michal

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Moritz Fischer]

Hi Michal,

On Mon, Apr 08, 2019 at 04:36:15PM +0200, Michal Simek wrote:
> On 08. 04. 19 16:17, Alan Tull wrote:
> > On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <navam@xilinx.com> wrote:
> >>
> >> Hi Alan,
> >>
> >> Thanks for look into it and providing the ACK.
> >> I got one minor comments from Moritz Fischer do you want me fix that issue now or I can fix it later as it’s a minor comment?
> > 
> > Please fix for Moritz comment.
> > 
> >> In which kernel version i can expect this driver changes??
> > 
> > Patch 2/3 and 3/3 are dependent on 1/3 which isn't a drivers/fpga
> > thing, it's drivers/firmware.
> 
> I can take it via arm-soc guys if you are ok with that.
> Just need to have your ack in commits.
> We have done it like this several times, IIRC reset, nvmem.

I'm fine with you taking it through arm-soc. Alan? I'll look at the other
patches.

Thanks,
Moritz

Re: [PATCH v4 1/3] firmware: xilinx: Add fpga API's

[Moritz Fischer]

Hi Nava,

On Tue, Apr 02, 2019 at 06:01:21PM +0530, Nava kishore Manne wrote:
> This Patch Adds fpga API's to support the Bitstream loading
> by using firmware interface.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
> Changes for v4:
> 		-None.
> 
> Chnages for v3:
> 		-Created patches on top of 5.0-rc5.
> 		 No functional changes.
> 
> Changes for v2:
> 		-Added Firmware FPGA Manager flags As suggested by
> 		 Moritz.
> 
> Changes for v1:
> 		-None.
> 
> Changes for RFC-V2:
> 		-New Patch
> 
>  drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h | 10 ++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 98f936125643..7159a90abc44 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -537,6 +537,50 @@ static int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset,
>  	return ret;
>  }
>  
> +/*
> + * zynqmp_pm_fpga_load - Perform the fpga load
> + * @address:	Address to write to
> + * @size:	pl bitstream size
> + * @flags:
> + *	BIT(0) - Bit-stream type.
> + *		 0 - Full Bitstream.
> + *		 1 - Partial Bitstream.
> + *
> + * This function provides access to pmufw. To transfer
> + * the required bitstream into PL.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int zynqmp_pm_fpga_load(const u64 address, const u32 size,
> +			       const u32 flags)
> +{
> +	return zynqmp_pm_invoke_fn(PM_FPGA_LOAD, lower_32_bits(address),
> +				   upper_32_bits(address), size, flags, NULL);
> +}
> +
> +/**
> + * zynqmp_pm_fpga_get_status - Read value from PCAP status register
> + * @value: Value to read
> + *
> + * This function provides access to the xilfpga library to get

xilfpga? Is that PMU firmware you're talking about?

> + * the PCAP status
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int zynqmp_pm_fpga_get_status(u32 *value)
> +{
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +	int ret;
> +
> +	if (!value)
> +		return -EINVAL;
> +
> +	ret = zynqmp_pm_invoke_fn(PM_FPGA_GET_STATUS, 0, 0, 0, 0, ret_payload);
> +	*value = ret_payload[1];
> +
> +	return ret;
> +}
> +
>  /**
>   * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
>   *			       master has initialized its own power management
> @@ -640,6 +684,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
>  	.request_node = zynqmp_pm_request_node,
>  	.release_node = zynqmp_pm_release_node,
>  	.set_requirement = zynqmp_pm_set_requirement,
> +	.fpga_load = zynqmp_pm_fpga_load,
> +	.fpga_get_status = zynqmp_pm_fpga_get_status,
>  };
>  
>  /**
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 642dab10f65d..4df226b6ab0f 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -48,6 +48,12 @@
>  #define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
>  #define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
>  
> +/*
> + * Firmware FPGA Manager flags
> + * XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration
> + */
> +#define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
> +
>  enum pm_api_id {
>  	PM_GET_API_VERSION = 1,
>  	PM_REQUEST_NODE = 13,
> @@ -56,6 +62,8 @@ enum pm_api_id {
>  	PM_RESET_ASSERT = 17,
>  	PM_RESET_GET_STATUS,
>  	PM_PM_INIT_FINALIZE = 21,
> +	PM_FPGA_LOAD = 22,
> +	PM_FPGA_GET_STATUS,

Any reason you can't do 'PM_FPGA_GET_STATUS = 23' here? Trying to
understand your reasoning. Are you planning to move them around?
>  	PM_GET_CHIPID = 24,
>  	PM_IOCTL = 34,
>  	PM_QUERY_DATA,
> @@ -258,6 +266,8 @@ struct zynqmp_pm_query_data {
>  struct zynqmp_eemi_ops {
>  	int (*get_api_version)(u32 *version);
>  	int (*get_chipid)(u32 *idcode, u32 *version);
> +	int (*fpga_load)(const u64 address, const u32 size, const u32 flags);
> +	int (*fpga_get_status)(u32 *value);
>  	int (*query_data)(struct zynqmp_pm_query_data qdata, u32 *out);
>  	int (*clock_enable)(u32 clock_id);
>  	int (*clock_disable)(u32 clock_id);
> -- 
> 2.18.0
> 

Thanks,
Moritz

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Alan Tull]

On Mon, Apr 8, 2019 at 11:51 AM Moritz Fischer <mdf@kernel.org> wrote:
>
> Hi Michal,
>
> On Mon, Apr 08, 2019 at 04:36:15PM +0200, Michal Simek wrote:
> > On 08. 04. 19 16:17, Alan Tull wrote:
> > > On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <navam@xilinx.com> wrote:
> > >>
> > >> Hi Alan,
> > >>
> > >> Thanks for look into it and providing the ACK.
> > >> I got one minor comments from Moritz Fischer do you want me fix that issue now or I can fix it later as it’s a minor comment?
> > >
> > > Please fix for Moritz comment.
> > >
> > >> In which kernel version i can expect this driver changes??
> > >
> > > Patch 2/3 and 3/3 are dependent on 1/3 which isn't a drivers/fpga
> > > thing, it's drivers/firmware.
> >
> > I can take it via arm-soc guys if you are ok with that.
> > Just need to have your ack in commits.
> > We have done it like this several times, IIRC reset, nvmem.
>
> I'm fine with you taking it through arm-soc. Alan?

Fine with me!

Alan

> I'll look at the other
> patches.
>
> Thanks,
> Moritz

Re: [PATCH v4 1/3] firmware: xilinx: Add fpga API's

[Michal Simek]

On 08. 04. 19 19:14, Moritz Fischer wrote:
> Hi Nava,
> 
> On Tue, Apr 02, 2019 at 06:01:21PM +0530, Nava kishore Manne wrote:
>> This Patch Adds fpga API's to support the Bitstream loading
>> by using firmware interface.
>>
>> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
>> ---
>> Changes for v4:
>> 		-None.
>>
>> Chnages for v3:
>> 		-Created patches on top of 5.0-rc5.
>> 		 No functional changes.
>>
>> Changes for v2:
>> 		-Added Firmware FPGA Manager flags As suggested by
>> 		 Moritz.
>>
>> Changes for v1:
>> 		-None.
>>
>> Changes for RFC-V2:
>> 		-New Patch
>>
>>  drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++
>>  include/linux/firmware/xlnx-zynqmp.h | 10 ++++++
>>  2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
>> index 98f936125643..7159a90abc44 100644
>> --- a/drivers/firmware/xilinx/zynqmp.c
>> +++ b/drivers/firmware/xilinx/zynqmp.c
>> @@ -537,6 +537,50 @@ static int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset,
>>  	return ret;
>>  }
>>  
>> +/*
>> + * zynqmp_pm_fpga_load - Perform the fpga load
>> + * @address:	Address to write to
>> + * @size:	pl bitstream size
>> + * @flags:
>> + *	BIT(0) - Bit-stream type.
>> + *		 0 - Full Bitstream.
>> + *		 1 - Partial Bitstream.
>> + *
>> + * This function provides access to pmufw. To transfer
>> + * the required bitstream into PL.
>> + *
>> + * Return: Returns status, either success or error+reason
>> + */
>> +static int zynqmp_pm_fpga_load(const u64 address, const u32 size,
>> +			       const u32 flags)
>> +{
>> +	return zynqmp_pm_invoke_fn(PM_FPGA_LOAD, lower_32_bits(address),
>> +				   upper_32_bits(address), size, flags, NULL);
>> +}
>> +
>> +/**
>> + * zynqmp_pm_fpga_get_status - Read value from PCAP status register
>> + * @value: Value to read
>> + *
>> + * This function provides access to the xilfpga library to get
> 
> xilfpga? Is that PMU firmware you're talking about?
> 
>> + * the PCAP status
>> + *
>> + * Return: Returns status, either success or error+reason
>> + */
>> +static int zynqmp_pm_fpga_get_status(u32 *value)
>> +{
>> +	u32 ret_payload[PAYLOAD_ARG_CNT];
>> +	int ret;
>> +
>> +	if (!value)
>> +		return -EINVAL;
>> +
>> +	ret = zynqmp_pm_invoke_fn(PM_FPGA_GET_STATUS, 0, 0, 0, 0, ret_payload);
>> +	*value = ret_payload[1];
>> +
>> +	return ret;
>> +}
>> +
>>  /**
>>   * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
>>   *			       master has initialized its own power management
>> @@ -640,6 +684,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
>>  	.request_node = zynqmp_pm_request_node,
>>  	.release_node = zynqmp_pm_release_node,
>>  	.set_requirement = zynqmp_pm_set_requirement,
>> +	.fpga_load = zynqmp_pm_fpga_load,
>> +	.fpga_get_status = zynqmp_pm_fpga_get_status,
>>  };
>>  
>>  /**
>> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
>> index 642dab10f65d..4df226b6ab0f 100644
>> --- a/include/linux/firmware/xlnx-zynqmp.h
>> +++ b/include/linux/firmware/xlnx-zynqmp.h
>> @@ -48,6 +48,12 @@
>>  #define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
>>  #define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
>>  
>> +/*
>> + * Firmware FPGA Manager flags
>> + * XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration
>> + */
>> +#define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
>> +
>>  enum pm_api_id {
>>  	PM_GET_API_VERSION = 1,
>>  	PM_REQUEST_NODE = 13,
>> @@ -56,6 +62,8 @@ enum pm_api_id {
>>  	PM_RESET_ASSERT = 17,
>>  	PM_RESET_GET_STATUS,
>>  	PM_PM_INIT_FINALIZE = 21,
>> +	PM_FPGA_LOAD = 22,
>> +	PM_FPGA_GET_STATUS,
> 
> Any reason you can't do 'PM_FPGA_GET_STATUS = 23' here? Trying to
> understand your reasoning. Are you planning to move them around?

It is 23 by design. In xilinx repo there was only the first value which
is recommended practice. But upstreaming is not done in the same order
that's why if there is a gap you need to assign values there.
Even that 22 can be removed in this case but it is just nit.

Thanks,
Michal

Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Michal Simek]

On 08. 04. 19 22:27, Alan Tull wrote:
> On Mon, Apr 8, 2019 at 11:51 AM Moritz Fischer <mdf@kernel.org> wrote:
>>
>> Hi Michal,
>>
>> On Mon, Apr 08, 2019 at 04:36:15PM +0200, Michal Simek wrote:
>>> On 08. 04. 19 16:17, Alan Tull wrote:
>>>> On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <navam@xilinx.com> wrote:
>>>>>
>>>>> Hi Alan,
>>>>>
>>>>> Thanks for look into it and providing the ACK.
>>>>> I got one minor comments from Moritz Fischer do you want me fix that issue now or I can fix it later as it’s a minor comment?
>>>>
>>>> Please fix for Moritz comment.
>>>>
>>>>> In which kernel version i can expect this driver changes??
>>>>
>>>> Patch 2/3 and 3/3 are dependent on 1/3 which isn't a drivers/fpga
>>>> thing, it's drivers/firmware.
>>>
>>> I can take it via arm-soc guys if you are ok with that.
>>> Just need to have your ack in commits.
>>> We have done it like this several times, IIRC reset, nvmem.
>>
>> I'm fine with you taking it through arm-soc. Alan?
> 
> Fine with me!

ok.
Nava: Please address all comments from reviewers and let me know when
all are happy and I will take it.

Thanks,
Michal

RE: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for Xilinx zynqmp

[Nava kishore Manne]

Hi Alan,

Thanks for the response.
Please find my response inline.

> -----Original Message-----
> From: Alan Tull [mailto:atull@kernel.org]
> Sent: Tuesday, April 9, 2019 1:57 AM
> To: Moritz Fischer <mdf@kernel.org>
> Cc: Michal Simek <michals@xilinx.com>; Nava kishore Manne
> <navam@xilinx.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah
> <JOLLYS@xilinx.com>; linux-fpga@vger.kernel.org; open list:OPEN FIRMWARE
> AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-
> kernel@lists.infradead.org>; linux-kernel <linux-kernel@vger.kernel.org>;
> kishore m <chinnikishore369@gmail.com>
> Subject: Re: [PATCH v4 3/3] fpga manager: Adding FPGA Manager support for
> Xilinx zynqmp
> 
> On Mon, Apr 8, 2019 at 11:51 AM Moritz Fischer <mdf@kernel.org> wrote:
> >
> > Hi Michal,
> >
> > On Mon, Apr 08, 2019 at 04:36:15PM +0200, Michal Simek wrote:
> > > On 08. 04. 19 16:17, Alan Tull wrote:
> > > > On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne
> <navam@xilinx.com> wrote:
> > > >>
> > > >> Hi Alan,
> > > >>
> > > >> Thanks for look into it and providing the ACK.
> > > >> I got one minor comments from Moritz Fischer do you want me fix that
> issue now or I can fix it later as it’s a minor comment?
> > > >
> > > > Please fix for Moritz comment.
Will fix in the next version.

Regards,
Navakishore.

RE: [PATCH v4 1/3] firmware: xilinx: Add fpga API's

[Nava kishore Manne]

Hi Moritz,

Thanks for the quick response.
Please find my response inline

> -----Original Message-----
> From: Michal Simek [mailto:michal.simek@xilinx.com]
> Sent: Tuesday, April 9, 2019 12:04 PM
> To: Moritz Fischer <mdf@kernel.org>; Nava kishore Manne
> <navam@xilinx.com>
> Cc: atull@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; Michal
> Simek <michals@xilinx.com>; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah
> <JOLLYS@xilinx.com>; linux-fpga@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> chinnikishore369@gmail.com
> Subject: Re: [PATCH v4 1/3] firmware: xilinx: Add fpga API's
> 
> On 08. 04. 19 19:14, Moritz Fischer wrote:
> > Hi Nava,
> >
> > On Tue, Apr 02, 2019 at 06:01:21PM +0530, Nava kishore Manne wrote:
> >> This Patch Adds fpga API's to support the Bitstream loading by using
> >> firmware interface.
> >>
> >> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> >> ---
> >> Changes for v4:
> >> 		-None.
> >>
> >> Chnages for v3:
> >> 		-Created patches on top of 5.0-rc5.
> >> 		 No functional changes.
> >>
> >> Changes for v2:
> >> 		-Added Firmware FPGA Manager flags As suggested by
> >> 		 Moritz.
> >>
> >> Changes for v1:
> >> 		-None.
> >>
> >> Changes for RFC-V2:
> >> 		-New Patch
> >>
> >>  drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++
> >>  include/linux/firmware/xlnx-zynqmp.h | 10 ++++++
> >>  2 files changed, 56 insertions(+)
> >>
> >> diff --git a/drivers/firmware/xilinx/zynqmp.c
> >> b/drivers/firmware/xilinx/zynqmp.c
> >> index 98f936125643..7159a90abc44 100644
> >> --- a/drivers/firmware/xilinx/zynqmp.c
> >> +++ b/drivers/firmware/xilinx/zynqmp.c
> >> @@ -537,6 +537,50 @@ static int zynqmp_pm_reset_get_status(const enum
> zynqmp_pm_reset reset,
> >>  	return ret;
> >>  }
> >>
> >> +/*
> >> + * zynqmp_pm_fpga_load - Perform the fpga load
> >> + * @address:	Address to write to
> >> + * @size:	pl bitstream size
> >> + * @flags:
> >> + *	BIT(0) - Bit-stream type.
> >> + *		 0 - Full Bitstream.
> >> + *		 1 - Partial Bitstream.
> >> + *
> >> + * This function provides access to pmufw. To transfer
> >> + * the required bitstream into PL.
> >> + *
> >> + * Return: Returns status, either success or error+reason  */ static
> >> +int zynqmp_pm_fpga_load(const u64 address, const u32 size,
> >> +			       const u32 flags)
> >> +{
> >> +	return zynqmp_pm_invoke_fn(PM_FPGA_LOAD, lower_32_bits(address),
> >> +				   upper_32_bits(address), size, flags, NULL); }
> >> +
> >> +/**
> >> + * zynqmp_pm_fpga_get_status - Read value from PCAP status register
> >> + * @value: Value to read
> >> + *
> >> + * This function provides access to the xilfpga library to get
> >
> > xilfpga? Is that PMU firmware you're talking about?

Xilfpga is a library and it’s a part of PMUFW BSP.
It will follow the below flow to configure the PL from Linux environment.
Linux -Fpga Manager framework <--> Linux-Firmware Driver <- -smc call-->ATF <--IPI call--> PMUFW<--> Xilfpga library.

> >
> >> + * the PCAP status
> >> + *
> >> + * Return: Returns status, either success or error+reason  */ static
> >> +int zynqmp_pm_fpga_get_status(u32 *value) {
> >> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> >> +	int ret;
> >> +
> >> +	if (!value)
> >> +		return -EINVAL;
> >> +
> >> +	ret = zynqmp_pm_invoke_fn(PM_FPGA_GET_STATUS, 0, 0, 0, 0,
> ret_payload);
> >> +	*value = ret_payload[1];
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  /**
> >>   * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
> >>   *			       master has initialized its own power management
> >> @@ -640,6 +684,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
> >>  	.request_node = zynqmp_pm_request_node,
> >>  	.release_node = zynqmp_pm_release_node,
> >>  	.set_requirement = zynqmp_pm_set_requirement,
> >> +	.fpga_load = zynqmp_pm_fpga_load,
> >> +	.fpga_get_status = zynqmp_pm_fpga_get_status,
> >>  };
> >>
> >>  /**
> >> diff --git a/include/linux/firmware/xlnx-zynqmp.h
> >> b/include/linux/firmware/xlnx-zynqmp.h
> >> index 642dab10f65d..4df226b6ab0f 100644
> >> --- a/include/linux/firmware/xlnx-zynqmp.h
> >> +++ b/include/linux/firmware/xlnx-zynqmp.h
> >> @@ -48,6 +48,12 @@
> >>  #define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
> >>  #define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
> >>
> >> +/*
> >> + * Firmware FPGA Manager flags
> >> + * XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration  */
> >> +#define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
> >> +
> >>  enum pm_api_id {
> >>  	PM_GET_API_VERSION = 1,
> >>  	PM_REQUEST_NODE = 13,
> >> @@ -56,6 +62,8 @@ enum pm_api_id {
> >>  	PM_RESET_ASSERT = 17,
> >>  	PM_RESET_GET_STATUS,
> >>  	PM_PM_INIT_FINALIZE = 21,
> >> +	PM_FPGA_LOAD = 22,
> >> +	PM_FPGA_GET_STATUS,
> >
> > Any reason you can't do 'PM_FPGA_GET_STATUS = 23' here? Trying to
> > understand your reasoning. Are you planning to move them around?
> 
> It is 23 by design. In xilinx repo there was only the first value which is
> recommended practice. But upstreaming is not done in the same order that's
> why if there is a gap you need to assign values there.
> Even that 22 can be removed in this case but it is just nit.
> 
 As Michal said here Assigning value of 22 is not needed. Will fix this issue in the next version.

Regards,
Navakishore.

Re: [PATCH v4 1/3] firmware: xilinx: Add fpga API's

[Moritz Fischer]

Hi Nava,

On Tue, Apr 09, 2019 at 08:54:34AM +0000, Nava kishore Manne wrote:
> Hi Moritz,
> 
> Thanks for the quick response.
> Please find my response inline
> 
> > -----Original Message-----
> > From: Michal Simek [mailto:michal.simek@xilinx.com]
> > Sent: Tuesday, April 9, 2019 12:04 PM
> > To: Moritz Fischer <mdf@kernel.org>; Nava kishore Manne
> > <navam@xilinx.com>
> > Cc: atull@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; Michal
> > Simek <michals@xilinx.com>; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah
> > <JOLLYS@xilinx.com>; linux-fpga@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > chinnikishore369@gmail.com
> > Subject: Re: [PATCH v4 1/3] firmware: xilinx: Add fpga API's
> > 
> > On 08. 04. 19 19:14, Moritz Fischer wrote:
> > > Hi Nava,
> > >
> > > On Tue, Apr 02, 2019 at 06:01:21PM +0530, Nava kishore Manne wrote:
> > >> This Patch Adds fpga API's to support the Bitstream loading by using
> > >> firmware interface.
> > >>
> > >> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > >> ---
> > >> Changes for v4:
> > >> 		-None.
> > >>
> > >> Chnages for v3:
> > >> 		-Created patches on top of 5.0-rc5.
> > >> 		 No functional changes.
> > >>
> > >> Changes for v2:
> > >> 		-Added Firmware FPGA Manager flags As suggested by
> > >> 		 Moritz.
> > >>
> > >> Changes for v1:
> > >> 		-None.
> > >>
> > >> Changes for RFC-V2:
> > >> 		-New Patch
> > >>
> > >>  drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++
> > >>  include/linux/firmware/xlnx-zynqmp.h | 10 ++++++
> > >>  2 files changed, 56 insertions(+)
> > >>
> > >> diff --git a/drivers/firmware/xilinx/zynqmp.c
> > >> b/drivers/firmware/xilinx/zynqmp.c
> > >> index 98f936125643..7159a90abc44 100644
> > >> --- a/drivers/firmware/xilinx/zynqmp.c
> > >> +++ b/drivers/firmware/xilinx/zynqmp.c
> > >> @@ -537,6 +537,50 @@ static int zynqmp_pm_reset_get_status(const enum
> > zynqmp_pm_reset reset,
> > >>  	return ret;
> > >>  }
> > >>
> > >> +/*
> > >> + * zynqmp_pm_fpga_load - Perform the fpga load
> > >> + * @address:	Address to write to
> > >> + * @size:	pl bitstream size
> > >> + * @flags:
> > >> + *	BIT(0) - Bit-stream type.
> > >> + *		 0 - Full Bitstream.
> > >> + *		 1 - Partial Bitstream.

Don't you wanna call out the defines instead here that you defined in
include/linux/firmware/xlnx-zynqmp.h?

> > >> + *
> > >> + * This function provides access to pmufw. To transfer
> > >> + * the required bitstream into PL.
> > >> + *
> > >> + * Return: Returns status, either success or error+reason  */ static
> > >> +int zynqmp_pm_fpga_load(const u64 address, const u32 size,
> > >> +			       const u32 flags)
> > >> +{
> > >> +	return zynqmp_pm_invoke_fn(PM_FPGA_LOAD, lower_32_bits(address),
> > >> +				   upper_32_bits(address), size, flags, NULL); }
> > >> +
> > >> +/**
> > >> + * zynqmp_pm_fpga_get_status - Read value from PCAP status register
> > >> + * @value: Value to read
> > >> + *
> > >> + * This function provides access to the xilfpga library to get
> > >
> > > xilfpga? Is that PMU firmware you're talking about?
> 
> Xilfpga is a library and it’s a part of PMUFW BSP.
> It will follow the below flow to configure the PL from Linux environment.
> Linux -Fpga Manager framework <--> Linux-Firmware Driver <- -smc call-->ATF <--IPI call--> PMUFW<--> Xilfpga library.

Fair enough. Was wondering if we could simplify/clarify the comment, but
also don't have a good suggestion :)
> 
> > >
> > >> + * the PCAP status
> > >> + *
> > >> + * Return: Returns status, either success or error+reason  */ static
> > >> +int zynqmp_pm_fpga_get_status(u32 *value) {
> > >> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> > >> +	int ret;
> > >> +
> > >> +	if (!value)
> > >> +		return -EINVAL;
> > >> +
> > >> +	ret = zynqmp_pm_invoke_fn(PM_FPGA_GET_STATUS, 0, 0, 0, 0,
> > ret_payload);
> > >> +	*value = ret_payload[1];
> > >> +
> > >> +	return ret;
> > >> +}
> > >> +
> > >>  /**
> > >>   * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
> > >>   *			       master has initialized its own power management
> > >> @@ -640,6 +684,8 @@ static const struct zynqmp_eemi_ops eemi_ops = {
> > >>  	.request_node = zynqmp_pm_request_node,
> > >>  	.release_node = zynqmp_pm_release_node,
> > >>  	.set_requirement = zynqmp_pm_set_requirement,
> > >> +	.fpga_load = zynqmp_pm_fpga_load,
> > >> +	.fpga_get_status = zynqmp_pm_fpga_get_status,
> > >>  };
> > >>
> > >>  /**
> > >> diff --git a/include/linux/firmware/xlnx-zynqmp.h
> > >> b/include/linux/firmware/xlnx-zynqmp.h
> > >> index 642dab10f65d..4df226b6ab0f 100644
> > >> --- a/include/linux/firmware/xlnx-zynqmp.h
> > >> +++ b/include/linux/firmware/xlnx-zynqmp.h
> > >> @@ -48,6 +48,12 @@
> > >>  #define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
> > >>  #define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
> > >>
> > >> +/*
> > >> + * Firmware FPGA Manager flags
> > >> + * XILINX_ZYNQMP_PM_FPGA_PARTIAL: FPGA partial reconfiguration  */
> > >> +#define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
> > >> +
> > >>  enum pm_api_id {
> > >>  	PM_GET_API_VERSION = 1,
> > >>  	PM_REQUEST_NODE = 13,
> > >> @@ -56,6 +62,8 @@ enum pm_api_id {
> > >>  	PM_RESET_ASSERT = 17,
> > >>  	PM_RESET_GET_STATUS,
> > >>  	PM_PM_INIT_FINALIZE = 21,
> > >> +	PM_FPGA_LOAD = 22,
> > >> +	PM_FPGA_GET_STATUS,
> > >
> > > Any reason you can't do 'PM_FPGA_GET_STATUS = 23' here? Trying to
> > > understand your reasoning. Are you planning to move them around?
> > 
> > It is 23 by design. In xilinx repo there was only the first value which is
> > recommended practice. But upstreaming is not done in the same order that's
> > why if there is a gap you need to assign values there.
> > Even that 22 can be removed in this case but it is just nit.
> > 
>  As Michal said here Assigning value of 22 is not needed. Will fix this issue in the next version.

Ok, I didn't check the Xilinx repos :) Makes sense, though.

With nits above addressed:
Reviewed-by: Moritz Fischer <mdf@kernel.org>

Thanks,
Moritz