2019-04-02 12:38:47

by Nava kishore Manne

[permalink] [raw]
Subject: [PATCH v4 0/3] Add Bitstream configuration support for ZynqMP

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


2019-04-02 12:38:57

by Nava kishore Manne

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

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

Signed-off-by: Nava kishore Manne <[email protected]>
---
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 <[email protected]>");
+MODULE_DESCRIPTION("Xilinx ZynqMp FPGA Manager");
+MODULE_LICENSE("GPL");
--
2.18.0

2019-04-02 12:39:00

by Nava kishore Manne

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

Add documentation to describe Xilinx ZynqMP fpga driver
bindings.

Signed-off-by: Nava kishore Manne <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Alan Tull <[email protected]>
Acked-by: Moritz Fischer <[email protected]>
---
.../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

2019-04-02 13:22:30

by Nava kishore Manne

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

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

Signed-off-by: Nava kishore Manne <[email protected]>
---
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

2019-04-02 14:59:45

by Moritz Fischer

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

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 <[email protected]>

Thanks,

Moritz

2019-04-02 18:34:32

by Alan Tull

[permalink] [raw]
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 <[email protected]> wrote:

Hi Nava,

Looks good.

>
> This patch adds FPGA Manager support for the Xilinx
> ZynqMP chip.
>
> Signed-off-by: Nava kishore Manne <[email protected]>
Acked-by: Alan Tull <[email protected]>

> ---
> 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

2019-04-08 12:40:49

by Nava kishore Manne

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

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:[email protected]]
> Sent: Tuesday, April 2, 2019 11:57 PM
> To: Nava kishore Manne <[email protected]>
> Cc: Moritz Fischer <[email protected]>; Rob Herring <[email protected]>; Mark
> Rutland <[email protected]>; Michal Simek <[email protected]>; Rajan
> Vaja <[email protected]>; Jolly Shah <[email protected]>; linux-
> [email protected]; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> TREE BINDINGS <[email protected]>; moderated list:ARM/FREESCALE
> IMX / MXC ARM ARCHITECTURE <[email protected]>; linux-
> kernel <[email protected]>; kishore m
> <[email protected]>
> 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 <[email protected]>
> wrote:
>
> Hi Nava,
>
> Looks good.
>
> >
> > This patch adds FPGA Manager support for the Xilinx ZynqMP chip.
> >
> > Signed-off-by: Nava kishore Manne <[email protected]>
> Acked-by: Alan Tull <[email protected]>
>
> > ---
> > 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

2019-04-08 14:19:46

by Alan Tull

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

On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <[email protected]> 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:[email protected]]
> > Sent: Tuesday, April 2, 2019 11:57 PM
> > To: Nava kishore Manne <[email protected]>
> > Cc: Moritz Fischer <[email protected]>; Rob Herring <[email protected]>; Mark
> > Rutland <[email protected]>; Michal Simek <[email protected]>; Rajan
> > Vaja <[email protected]>; Jolly Shah <[email protected]>; linux-
> > [email protected]; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> > TREE BINDINGS <[email protected]>; moderated list:ARM/FREESCALE
> > IMX / MXC ARM ARCHITECTURE <[email protected]>; linux-
> > kernel <[email protected]>; kishore m
> > <[email protected]>
> > 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 <[email protected]>
> > wrote:
> >
> > Hi Nava,
> >
> > Looks good.
> >
> > >
> > > This patch adds FPGA Manager support for the Xilinx ZynqMP chip.
> > >
> > > Signed-off-by: Nava kishore Manne <[email protected]>
> > Acked-by: Alan Tull <[email protected]>
> >
> > > ---
> > > 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

2019-04-08 15:42:02

by Michal Simek

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

On 08. 04. 19 16:17, Alan Tull wrote:
> On Mon, Apr 8, 2019 at 7:39 AM Nava kishore Manne <[email protected]> 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

2019-04-08 16:52:15

by Moritz Fischer

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

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 <[email protected]> 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

2019-04-08 18:30:05

by Moritz Fischer

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

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 <[email protected]>
> ---
> 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

2019-04-08 21:44:19

by Alan Tull

[permalink] [raw]
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 <[email protected]> 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 <[email protected]> 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

2019-04-09 06:35:33

by Michal Simek

[permalink] [raw]
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 <[email protected]>
>> ---
>> 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

2019-04-09 06:36:59

by Michal Simek

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

On 08. 04. 19 22:27, Alan Tull wrote:
> On Mon, Apr 8, 2019 at 11:51 AM Moritz Fischer <[email protected]> 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 <[email protected]> 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

2019-04-09 06:51:02

by Nava kishore Manne

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

Hi Alan,

Thanks for the response.
Please find my response inline.

> -----Original Message-----
> From: Alan Tull [mailto:[email protected]]
> Sent: Tuesday, April 9, 2019 1:57 AM
> To: Moritz Fischer <[email protected]>
> Cc: Michal Simek <[email protected]>; Nava kishore Manne
> <[email protected]>; Rob Herring <[email protected]>; Mark Rutland
> <[email protected]>; Rajan Vaja <[email protected]>; Jolly Shah
> <[email protected]>; [email protected]; open list:OPEN FIRMWARE
> AND FLATTENED DEVICE TREE BINDINGS <[email protected]>;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-
> [email protected]>; linux-kernel <[email protected]>;
> kishore m <[email protected]>
> 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 <[email protected]> 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
> <[email protected]> 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.

2019-04-09 08:58:32

by Nava kishore Manne

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

Hi Moritz,

Thanks for the quick response.
Please find my response inline

> -----Original Message-----
> From: Michal Simek [mailto:[email protected]]
> Sent: Tuesday, April 9, 2019 12:04 PM
> To: Moritz Fischer <[email protected]>; Nava kishore Manne
> <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; Michal
> Simek <[email protected]>; Rajan Vaja <[email protected]>; Jolly Shah
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> 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 <[email protected]>
> >> ---
> >> 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.

2019-04-09 15:56:00

by Moritz Fischer

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

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:[email protected]]
> > Sent: Tuesday, April 9, 2019 12:04 PM
> > To: Moritz Fischer <[email protected]>; Nava kishore Manne
> > <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; Michal
> > Simek <[email protected]>; Rajan Vaja <[email protected]>; Jolly Shah
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]
> > 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 <[email protected]>
> > >> ---
> > >> 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 <[email protected]>

Thanks,
Moritz