[U-Boot] [PATCH 00/13] arm64: zynqmp: Clean communication with PMUFW

[U-Boot] [PATCH 00/13] arm64: zynqmp: Clean communication with PMUFW

[Michal Simek]

Hi,

This patch series using ZynqMP firmware driver to provide a interface to
communicate with the PMU Firmware. As part of the series a mailbox driver
is also implemented to handle communication through ipi interface.

There are two new wiring:
1. Reading PMUFW version via firmware driver
 - mailbox driver in case of SPL
 - SMC in case of full U-Boot
2. Using the same patch for loading PMUFW configuration object

The whole series is based on several patches I have sent already that's why
I am providing a branch which also contains this series.
https://gitlab.denx.de/u-boot/custodians/u-boot-microblaze/tree/next

Thanks,
Michal


Ibai Erkiaga (10):
  mailbox: check ops prior calling
  mailbox: allow subnode for mbox regs
  mailbox: zynqmp: ipi mailbox driver
  firmware: zynqmp: Add zynqmp-power support
  arm64: zynqmp: add firmware and mailbox node to DT
  firmware: zynqmp: get fw version with mailbox driver
  firmware: zynqmp: create firmware header
  arm64: zynqmp: use firmware driver to get version
  arm64: zynqmp: remove old fw version function
  arm64: zynqmp: probe firmware driver

Michal Simek (3):
  arm64: zynqmp: Cleanup PM SMC macro composition
  firmware: zynqmp: Separate function for sending message via mailbox
  arm64: zynqmp: Use mailbox driver for PMUFW config loading

 MAINTAINERS                                   |   2 +
 arch/arm/Kconfig                              |   3 +
 arch/arm/dts/zynqmp.dtsi                      |  44 +++++-
 arch/arm/mach-zynqmp/Makefile                 |   4 -
 arch/arm/mach-zynqmp/cpu.c                    |  24 +---
 arch/arm/mach-zynqmp/include/mach/sys_proto.h |  24 +---
 arch/arm/mach-zynqmp/pmu_ipc.c                | 112 ---------------
 board/xilinx/zynqmp/cmds.c                    |   1 +
 board/xilinx/zynqmp/zynqmp.c                  |  19 +--
 drivers/firmware/Kconfig                      |   2 +
 drivers/firmware/firmware-zynqmp.c            | 131 +++++++++++++++++
 drivers/fpga/zynqmppl.c                       |   5 +-
 drivers/mailbox/Kconfig                       |   6 +
 drivers/mailbox/Makefile                      |   1 +
 drivers/mailbox/mailbox-uclass.c              |  19 ++-
 drivers/mailbox/zynqmp-ipi.c                  | 134 ++++++++++++++++++
 include/zynqmp_firmware.h                     |  37 +++++
 17 files changed, 388 insertions(+), 180 deletions(-)
 delete mode 100644 arch/arm/mach-zynqmp/pmu_ipc.c
 create mode 100644 drivers/mailbox/zynqmp-ipi.c
 create mode 100644 include/zynqmp_firmware.h

-- 
2.17.1

[U-Boot] [PATCH 01/13] mailbox: check ops prior calling

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

Check if request and free operations are present prior calling to the
functions.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/mailbox/mailbox-uclass.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/mailbox-uclass.c b/drivers/mailbox/mailbox-uclass.c
index 1b4a5863c9e5..809f26b20258 100644
--- a/drivers/mailbox/mailbox-uclass.c
+++ b/drivers/mailbox/mailbox-uclass.c
@@ -63,7 +63,8 @@ int mbox_get_by_index(struct udevice *dev, int index, struct mbox_chan *chan)
 		return ret;
 	}
 
-	ret = ops->request(chan);
+	if (ops->request)
+		ret = ops->request(chan);
 	if (ret) {
 		debug("ops->request() failed: %d\n", ret);
 		return ret;
@@ -94,7 +95,10 @@ int mbox_free(struct mbox_chan *chan)
 
 	debug("%s(chan=%p)\n", __func__, chan);
 
-	return ops->free(chan);
+	if (ops->free)
+		return ops->free(chan);
+
+	return 0;
 }
 
 int mbox_send(struct mbox_chan *chan, const void *data)
-- 
2.17.1

[U-Boot] [PATCH 02/13] mailbox: allow subnode for mbox regs

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

The following patch allows the mailbox node in DT to host subnodes with
mailbox definitions. If the client phandle to the mailbox is not the
mailbox driver node, just checks parents as well.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/mailbox/mailbox-uclass.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/mailbox-uclass.c b/drivers/mailbox/mailbox-uclass.c
index 809f26b20258..9fdb6279e4f3 100644
--- a/drivers/mailbox/mailbox-uclass.c
+++ b/drivers/mailbox/mailbox-uclass.c
@@ -49,7 +49,16 @@ int mbox_get_by_index(struct udevice *dev, int index, struct mbox_chan *chan)
 	if (ret) {
 		debug("%s: uclass_get_device_by_of_offset failed: %d\n",
 		      __func__, ret);
-		return ret;
+
+		/* Test with parent node */
+		ret = uclass_get_device_by_ofnode(UCLASS_MAILBOX,
+						  ofnode_get_parent(args.node),
+						  &dev_mbox);
+		if (ret) {
+			debug("%s: mbox node from parent failed: %d\n",
+			      __func__, ret);
+			return ret;
+		};
 	}
 	ops = mbox_dev_ops(dev_mbox);
 
-- 
2.17.1

[U-Boot] [PATCH 03/13] mailbox: zynqmp: ipi mailbox driver

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

ZynqMP mailbox driver implementing IPI communication with PMU. This would
allow U-Boot SPL to communicate with PMUFW to request privileged
operations.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 MAINTAINERS                                   |   1 +
 arch/arm/mach-zynqmp/include/mach/sys_proto.h |   5 +
 drivers/mailbox/Kconfig                       |   6 +
 drivers/mailbox/Makefile                      |   1 +
 drivers/mailbox/zynqmp-ipi.c                  | 134 ++++++++++++++++++
 5 files changed, 147 insertions(+)
 create mode 100644 drivers/mailbox/zynqmp-ipi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f448c5f19e00..f5feb89ac3e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -448,6 +448,7 @@ F:	drivers/gpio/zynq_gpio.c
 F:	drivers/i2c/i2c-cdns.c
 F:	drivers/i2c/muxes/pca954x.c
 F:	drivers/i2c/zynq_i2c.c
+F:	drivers/mailbox/zynqmp-ipi.c
 F:	drivers/mmc/zynq_sdhci.c
 F:	drivers/mtd/nand/raw/zynq_nand.c
 F:	drivers/net/phy/xilinx_phy.c
diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index 915badc6fbee..f25d414dcb1e 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -54,6 +54,11 @@ enum {
 	TCM_SPLIT,
 };
 
+struct zynqmp_ipi_msg {
+	size_t len;
+	u32 *buf;
+};
+
 int zynq_board_read_rom_ethaddr(unsigned char *ethaddr);
 unsigned int zynqmp_get_silicon_version(void);
 
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 11bf5522db53..85c2a829aed8 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -41,4 +41,10 @@ config K3_SEC_PROXY
 	  Select this driver if your platform has support for this hardware
 	  block.
 
+config ZYNQMP_IPI
+	bool "Xilinx ZynqMP IPI controller support"
+	depends on DM_MAILBOX && ARCH_ZYNQMP
+	help
+	  This enables support for the Xilinx ZynqMP Inter Processor Interrupt
+	  communication controller.
 endmenu
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index a753cc4e6806..d2ace8cd212e 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_SANDBOX_MBOX) += sandbox-mbox-test.o
 obj-$(CONFIG_STM32_IPCC) += stm32-ipcc.o
 obj-$(CONFIG_TEGRA_HSP) += tegra-hsp.o
 obj-$(CONFIG_K3_SEC_PROXY) += k3-sec-proxy.o
+obj-$(CONFIG_ZYNQMP_IPI) += zynqmp-ipi.o
diff --git a/drivers/mailbox/zynqmp-ipi.c b/drivers/mailbox/zynqmp-ipi.c
new file mode 100644
index 000000000000..c181a7b81768
--- /dev/null
+++ b/drivers/mailbox/zynqmp-ipi.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Xilinx Zynq MPSoC Mailbox driver
+ *
+ * Copyright (C) 2018-2019 Xilinx, Inc.
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <dm.h>
+#include <mailbox-uclass.h>
+#include <mach/sys_proto.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <wait_bit.h>
+
+/* IPI bitmasks, register base */
+/* TODO: move reg base to DT */
+#define IPI_BIT_MASK_PMU0     0x10000
+#define IPI_INT_REG_BASE_APU  0xFF300000
+
+struct ipi_int_regs {
+	u32 trig; /* 0x0  */
+	u32 obs;  /* 0x4  */
+	u32 ist;  /* 0x8  */
+	u32 imr;  /* 0xC  */
+	u32 ier;  /* 0x10 */
+	u32 idr;  /* 0x14 */
+};
+
+#define ipi_int_apu ((struct ipi_int_regs *)IPI_INT_REG_BASE_APU)
+
+struct zynqmp_ipi {
+	void __iomem *local_req_regs;
+	void __iomem *local_res_regs;
+	void __iomem *remote_req_regs;
+	void __iomem *remote_res_regs;
+};
+
+static int zynqmp_ipi_send(struct mbox_chan *chan, const void *data)
+{
+	const struct zynqmp_ipi_msg *msg = (struct zynqmp_ipi_msg *)data;
+	struct zynqmp_ipi *zynqmp = dev_get_priv(chan->dev);
+	u32 ret;
+	u32 *mbx = (u32 *)zynqmp->local_req_regs;
+
+	for (size_t i = 0; i < msg->len; i++)
+		writel(msg->buf[i], &mbx[i]);
+
+	/* Write trigger interrupt */
+	writel(IPI_BIT_MASK_PMU0, &ipi_int_apu->trig);
+
+	/* Wait until observation bit is cleared */
+	ret = wait_for_bit_le32(&ipi_int_apu->obs, IPI_BIT_MASK_PMU0, false,
+				100, false);
+
+	debug("%s, send %ld bytes\n", __func__, msg->len);
+	return ret;
+};
+
+static int zynqmp_ipi_recv(struct mbox_chan *chan, void *data)
+{
+	struct zynqmp_ipi_msg *msg = (struct zynqmp_ipi_msg *)data;
+	struct zynqmp_ipi *zynqmp = dev_get_priv(chan->dev);
+	u32 *mbx = (u32 *)zynqmp->local_res_regs;
+
+	for (size_t i = 0; i < msg->len; i++)
+		msg->buf[i] = readl(&mbx[i]);
+
+	debug("%s, recv %ld bytes\n", __func__, msg->len);
+	return 0;
+};
+
+static int zynqmp_ipi_probe(struct udevice *dev)
+{
+	struct zynqmp_ipi *zynqmp = dev_get_priv(dev);
+	struct resource res;
+	ofnode node;
+
+	debug("%s(dev=%p)\n", __func__, dev);
+
+	/* Get subnode where the regs are defined */
+	/* Note IPI mailbox node needs to be the first one in DT */
+	node = ofnode_first_subnode(dev_ofnode(dev));
+
+	if (ofnode_read_resource_byname(node, "local_request_region", &res)) {
+		dev_err(dev, "No reg property for local_request_region\n");
+		return -EINVAL;
+	};
+	zynqmp->local_req_regs = devm_ioremap(dev, res.start,
+					      (res.start - res.end));
+
+	if (ofnode_read_resource_byname(node, "local_response_region", &res)) {
+		dev_err(dev, "No reg property for local_response_region\n");
+		return -EINVAL;
+	};
+	zynqmp->local_res_regs = devm_ioremap(dev, res.start,
+					      (res.start - res.end));
+
+	if (ofnode_read_resource_byname(node, "remote_request_region", &res)) {
+		dev_err(dev, "No reg property for remote_request_region\n");
+		return -EINVAL;
+	};
+	zynqmp->remote_req_regs = devm_ioremap(dev, res.start,
+					       (res.start - res.end));
+
+	if (ofnode_read_resource_byname(node, "remote_response_region", &res)) {
+		dev_err(dev, "No reg property for remote_response_region\n");
+		return -EINVAL;
+	};
+	zynqmp->remote_res_regs = devm_ioremap(dev, res.start,
+					       (res.start - res.end));
+
+	return 0;
+};
+
+static const struct udevice_id zynqmp_ipi_ids[] = {
+	{ .compatible = "xlnx,zynqmp-ipi-mailbox" },
+	{ }
+};
+
+struct mbox_ops zynqmp_ipi_mbox_ops = {
+	.send = zynqmp_ipi_send,
+	.recv = zynqmp_ipi_recv,
+};
+
+U_BOOT_DRIVER(zynqmp_ipi) = {
+	.name = "zynqmp-ipi",
+	.id = UCLASS_MAILBOX,
+	.of_match = zynqmp_ipi_ids,
+	.probe = zynqmp_ipi_probe,
+	.priv_auto_alloc_size = sizeof(struct zynqmp_ipi),
+	.ops = &zynqmp_ipi_mbox_ops,
+};
-- 
2.17.1

[U-Boot] [PATCH 04/13] firmware: zynqmp: Add zynqmp-power support

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

zynqmp-power driver for ZynqMP to handle the communication with the PMU
firmware. Firmware driver just probes subnodes and power driver handles
communication with PMU using the IPI mailbox driver.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/firmware/Kconfig           |  2 ++
 drivers/firmware/firmware-zynqmp.c | 40 ++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index b70a2063551c..9596ec16c7f7 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL
 config ZYNQMP_FIRMWARE
 	bool "ZynqMP Firmware interface"
 	select FIRMWARE
+	select ZYNQMP_IPI
+	select DM_MAILBOX
 	help
 	  Firmware interface driver is used by different
 	  drivers to communicate with the firmware for
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index 6644a7166ca0..b0930447b988 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -1,6 +1,46 @@
 // SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Zynq MPSoC Firmware driver
+ *
+ * Copyright (C) 2018-2019 Xilinx, Inc.
+ */
 
+#include <common.h>
 #include <dm.h>
+#include <mailbox.h>
+#include <asm/arch/sys_proto.h>
+
+struct zynqmp_power {
+	struct mbox_chan tx_chan;
+	struct mbox_chan rx_chan;
+} zynqmp_power;
+
+static int zynqmp_power_probe(struct udevice *dev)
+{
+	int ret = 0;
+
+	debug("%s, (dev=%p)\n", __func__, dev);
+
+	ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
+	ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);
+
+	if (ret)
+		debug("%s, cannot get mailboxes\n", __func__);
+
+	return ret;
+};
+
+static const struct udevice_id zynqmp_power_ids[] = {
+	{ .compatible = "xlnx,zynqmp-power" },
+	{ }
+};
+
+U_BOOT_DRIVER(zynqmp_power) = {
+	.name = "zynqmp_power",
+	.id = UCLASS_FIRMWARE,
+	.of_match = zynqmp_power_ids,
+	.probe = zynqmp_power_probe,
+};
 
 static const struct udevice_id zynqmp_firmware_ids[] = {
 	{ .compatible = "xlnx,zynqmp-firmware" },
-- 
2.17.1

[U-Boot] [PATCH 05/13] arm64: zynqmp: add firmware and mailbox node to DT

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

The following patch adds a mailbox node and firmware node to following the
mainline DT.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 arch/arm/dts/zynqmp.dtsi | 44 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/arch/arm/dts/zynqmp.dtsi b/arch/arm/dts/zynqmp.dtsi
index dfb6ebc64ce5..8e35171dd01d 100644
--- a/arch/arm/dts/zynqmp.dtsi
+++ b/arch/arm/dts/zynqmp.dtsi
@@ -96,6 +96,29 @@
 		};
 	};
 
+	zynqmp_ipi {
+		u-boot,dm-pre-reloc;
+		compatible = "xlnx,zynqmp-ipi-mailbox";
+		interrupt-parent = <&gic>;
+		interrupts = <0 35 4>;
+		xlnx,ipi-id = <0>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		ipi_mailbox_pmu1: mailbox at ff990400 {
+			u-boot,dm-pre-reloc;
+			reg = <0x0 0xff9905c0 0x0 0x20>,
+			      <0x0 0xff9905e0 0x0 0x20>,
+			      <0x0 0xff990e80 0x0 0x20>,
+			      <0x0 0xff990ea0 0x0 0x20>;
+			reg-names = "local_request_region" , "local_response_region",
+				    "remote_request_region", "remote_response_region";
+			#mbox-cells = <1>;
+			xlnx,ipi-id = <4>;
+		};
+	};
+
 	dcc: dcc {
 		compatible = "arm,dcc";
 		status = "disabled";
@@ -116,11 +139,22 @@
 		method = "smc";
 	};
 
-	pmufw: firmware {
-		compatible = "xlnx,zynqmp-pm";
-		method = "smc";
-		interrupt-parent = <&gic>;
-		interrupts = <0 35 4>;
+	firmware {
+		zynqmp-firmware {
+			compatible = "xlnx,zynqmp-firmware";
+			method = "smc";
+			#power-domain-cells = <0x1>;
+			u-boot,dm-pre-reloc;
+
+			zynqmp_power: zynqmp-power {
+				u-boot,dm-pre-reloc;
+				compatible = "xlnx,zynqmp-power";
+				interrupt-parent = <&gic>;
+				interrupts = <0 35 4>;
+				mboxes = <&ipi_mailbox_pmu1 0>, <&ipi_mailbox_pmu1 1>;
+				mbox-names = "tx", "rx";
+			};
+		};
 	};
 
 	timer {
-- 
2.17.1

[U-Boot] [PATCH 06/13] arm64: zynqmp: Cleanup PM SMC macro composition

[Michal Simek]

Cleanup PM ID handling by using enum values.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
---

 arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index f25d414dcb1e..573c4ffceed9 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -10,7 +10,8 @@
 #define PAYLOAD_ARG_CNT		5
 
 #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
-#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
+#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
+	(PM_SIP_SVC + PM_SECURE_IMAGE)
 #define KEY_PTR_LEN	32
 
 #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
@@ -21,7 +22,8 @@
 
 #define ZYNQMP_FPGA_AUTH_DDR	1
 
-#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
+#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
+	(PM_SIP_SVC + PM_GET_API_VERSION)
 
 #define ZYNQMP_PM_VERSION_MAJOR		1
 #define ZYNQMP_PM_VERSION_MINOR		0
@@ -36,6 +38,13 @@
 
 #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
 
+#define PM_SIP_SVC	0xc2000000
+
+enum pm_api_id {
+	PM_GET_API_VERSION = 1,
+	PM_SECURE_IMAGE = 45,
+};
+
 enum {
 	IDCODE,
 	VERSION,
-- 
2.17.1

[U-Boot] [PATCH 07/13] firmware: zynqmp: get fw version with mailbox driver

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

Implements the function to get PMU Firmware version using the mailbox
driver or smc call based on if running SPL or not. Additionally gets
version as part of the ZynqMP Firmware driver probing

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/firmware/firmware-zynqmp.c | 69 +++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index b0930447b988..f4d9fd9569e2 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -10,11 +10,69 @@
 #include <mailbox.h>
 #include <asm/arch/sys_proto.h>
 
+#define PMUFW_PAYLOAD_ARG_CNT	8
+
 struct zynqmp_power {
 	struct mbox_chan tx_chan;
 	struct mbox_chan rx_chan;
 } zynqmp_power;
 
+static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
+{
+	struct zynqmp_ipi_msg msg;
+
+	if (req_len > PMUFW_PAYLOAD_ARG_CNT ||
+	    res_maxlen > PMUFW_PAYLOAD_ARG_CNT)
+		return -EINVAL;
+
+	if (!(zynqmp_power.tx_chan.dev) || !(&zynqmp_power.rx_chan.dev))
+		return -EINVAL;
+
+	msg.buf = (u32 *)req;
+	msg.len = req_len;
+	mbox_send(&zynqmp_power.tx_chan, &msg);
+
+	msg.buf = res;
+	msg.len = res_maxlen;
+	mbox_recv(&zynqmp_power.rx_chan, &msg, 100);
+
+	return 0;
+}
+
+unsigned int zynqmp_firmware_version(void)
+{
+	int ret;
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	static u32 pm_api_version = ZYNQMP_PM_VERSION_INVALID;
+
+	/*
+	 * Get PMU version only once and later
+	 * just return stored values instead of
+	 * asking PMUFW again.
+	 **/
+	if (pm_api_version == ZYNQMP_PM_VERSION_INVALID) {
+		if (IS_ENABLED(CONFIG_SPL_BUILD)) {
+			const u32 request[] = { PM_GET_API_VERSION };
+
+			ret = ipi_req(request, ARRAY_SIZE(request),
+				      ret_payload, 2);
+		} else {
+			ret = invoke_smc(ZYNQMP_SIP_SVC_GET_API_VERSION, 0, 0,
+					 0, 0, ret_payload);
+		};
+
+		if (ret)
+			panic("PMUFW is not found - Please load it!\n");
+
+		pm_api_version = ret_payload[1];
+		if (pm_api_version < ZYNQMP_PM_VERSION)
+			panic("PMUFW version error. Expected: v%d.%d\n",
+			      ZYNQMP_PM_VERSION_MAJOR, ZYNQMP_PM_VERSION_MINOR);
+	}
+
+	return pm_api_version;
+};
+
 static int zynqmp_power_probe(struct udevice *dev)
 {
 	int ret = 0;
@@ -24,10 +82,17 @@ static int zynqmp_power_probe(struct udevice *dev)
 	ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
 	ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);
 
-	if (ret)
+	if (ret) {
 		debug("%s, cannot get mailboxes\n", __func__);
+		return ret;
+	}
+
+	ret = zynqmp_firmware_version();
+	printf("PMUFW:\tv%d.%d\n",
+	       ret >> ZYNQMP_PM_VERSION_MAJOR_SHIFT,
+	       ret & ZYNQMP_PM_VERSION_MINOR_MASK);
 
-	return ret;
+	return 0;
 };
 
 static const struct udevice_id zynqmp_power_ids[] = {
-- 
2.17.1

[U-Boot] [PATCH 08/13] firmware: zynqmp: create firmware header

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

New firmware header to place firmware specific macro and function
declarations. The patch also moves the macros defining PM operations as
well as some helper macros.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 MAINTAINERS                                   |  1 +
 arch/arm/mach-zynqmp/cpu.c                    |  1 +
 arch/arm/mach-zynqmp/include/mach/sys_proto.h | 25 ------------
 board/xilinx/zynqmp/cmds.c                    |  1 +
 board/xilinx/zynqmp/zynqmp.c                  |  1 +
 drivers/firmware/firmware-zynqmp.c            |  1 +
 drivers/fpga/zynqmppl.c                       |  1 +
 include/zynqmp_firmware.h                     | 38 +++++++++++++++++++
 8 files changed, 44 insertions(+), 25 deletions(-)
 create mode 100644 include/zynqmp_firmware.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f5feb89ac3e9..5d5c0fc61f03 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -460,6 +460,7 @@ F:	drivers/timer/cadence-ttc.c
 F:	drivers/usb/host/ehci-zynq.c
 F:	drivers/watchdog/cdns_wdt.c
 F:	include/zynqmppl.h
+F:	include/zynqmp_firmware.h
 F:	tools/zynqmp*
 N:	ultra96
 N:	zynqmp
diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c
index 5ef1a52862c0..f28b964a1560 100644
--- a/arch/arm/mach-zynqmp/cpu.c
+++ b/arch/arm/mach-zynqmp/cpu.c
@@ -9,6 +9,7 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/armv8/mmu.h>
 #include <asm/io.h>
+#include <zynqmp_firmware.h>
 
 #define ZYNQ_SILICON_VER_MASK	0xF000
 #define ZYNQ_SILICON_VER_SHIFT	12
diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index 573c4ffceed9..658974445417 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -10,8 +10,6 @@
 #define PAYLOAD_ARG_CNT		5
 
 #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
-#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
-	(PM_SIP_SVC + PM_SECURE_IMAGE)
 #define KEY_PTR_LEN	32
 
 #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
@@ -22,29 +20,6 @@
 
 #define ZYNQMP_FPGA_AUTH_DDR	1
 
-#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
-	(PM_SIP_SVC + PM_GET_API_VERSION)
-
-#define ZYNQMP_PM_VERSION_MAJOR		1
-#define ZYNQMP_PM_VERSION_MINOR		0
-#define ZYNQMP_PM_VERSION_MAJOR_SHIFT	16
-#define ZYNQMP_PM_VERSION_MINOR_MASK	0xFFFF
-
-#define ZYNQMP_PM_VERSION	\
-	((ZYNQMP_PM_VERSION_MAJOR << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | \
-				 ZYNQMP_PM_VERSION_MINOR)
-
-#define ZYNQMP_PM_VERSION_INVALID	~0
-
-#define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
-
-#define PM_SIP_SVC	0xc2000000
-
-enum pm_api_id {
-	PM_GET_API_VERSION = 1,
-	PM_SECURE_IMAGE = 45,
-};
-
 enum {
 	IDCODE,
 	VERSION,
diff --git a/board/xilinx/zynqmp/cmds.c b/board/xilinx/zynqmp/cmds.c
index ed7ba58c6475..f53a1b63bea6 100644
--- a/board/xilinx/zynqmp/cmds.c
+++ b/board/xilinx/zynqmp/cmds.c
@@ -7,6 +7,7 @@
 #include <common.h>
 #include <env.h>
 #include <malloc.h>
+#include <zynqmp_firmware.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/io.h>
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 6524badf299c..d9186f463f20 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -21,6 +21,7 @@
 #include <usb.h>
 #include <dwc3-uboot.h>
 #include <zynqmppl.h>
+#include <zynqmp_firmware.h>
 #include <g_dnl.h>
 #include <linux/sizes.h>
 
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index f4d9fd9569e2..d70f34f24388 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <dm.h>
 #include <mailbox.h>
+#include <zynqmp_firmware.h>
 #include <asm/arch/sys_proto.h>
 
 #define PMUFW_PAYLOAD_ARG_CNT	8
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index f6d9d50ef15f..6168626aeef3 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -8,6 +8,7 @@
 #include <console.h>
 #include <common.h>
 #include <zynqmppl.h>
+#include <zynqmp_firmware.h>
 #include <linux/sizes.h>
 #include <asm/arch/sys_proto.h>
 #include <memalign.h>
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
new file mode 100644
index 000000000000..c522cae8399b
--- /dev/null
+++ b/include/zynqmp_firmware.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Xilinx Zynq MPSoC Firmware driver
+ *
+ * Copyright (C) 2018-2019 Xilinx, Inc.
+ */
+
+#ifndef _ZYNQM_FIRMWARE_H_
+#define _ZYNQM_FIRMWARE_H_
+
+#define PM_SIP_SVC      0xc2000000
+#define ZYNQMP_SIP_SVC_GET_API_VERSION          \
+	(PM_SIP_SVC + PM_GET_API_VERSION)
+#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD       \
+	(PM_SIP_SVC + PM_SECURE_IMAGE)
+
+#define ZYNQMP_PM_VERSION_MAJOR         1
+#define ZYNQMP_PM_VERSION_MINOR         0
+#define ZYNQMP_PM_VERSION_MAJOR_SHIFT   16
+#define ZYNQMP_PM_VERSION_MINOR_MASK    0xFFFF
+
+#define ZYNQMP_PM_VERSION       \
+	((ZYNQMP_PM_VERSION_MAJOR << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | \
+	 ZYNQMP_PM_VERSION_MINOR)
+
+#define ZYNQMP_PM_VERSION_INVALID       ~0
+
+#define PMUFW_V1_0      ((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
+
+enum pm_api_id {
+	PM_GET_API_VERSION = 1,
+	PM_SET_CONFIGURATION,
+	PM_SECURE_IMAGE = 45,
+};
+
+unsigned int zynqmp_firmware_version(void);
+
+#endif /* _ZYNQMPPL_H_ */
-- 
2.17.1

[U-Boot] [PATCH 09/13] arm64: zynqmp: use firmware driver to get version

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

Use the new function from firmware version to get the firmware version.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 board/xilinx/zynqmp/zynqmp.c | 2 +-
 drivers/fpga/zynqmppl.c      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index d9186f463f20..8a66d3e50aad 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -321,7 +321,7 @@ int board_early_init_f(void)
 #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_CLK_ZYNQMP)
 	u32 pm_api_version;
 
-	pm_api_version = zynqmp_pmufw_version();
+	pm_api_version = zynqmp_firmware_version();
 	printf("PMUFW:\tv%d.%d\n",
 	       pm_api_version >> ZYNQMP_PM_VERSION_MAJOR_SHIFT,
 	       pm_api_version & ZYNQMP_PM_VERSION_MINOR_MASK);
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 6168626aeef3..c2670271c8ea 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -152,7 +152,7 @@ static ulong zynqmp_align_dma_buffer(u32 *buf, u32 len, u32 swap)
 
 		buf = new_buf;
 	} else if ((swap != SWAP_DONE) &&
-		   (zynqmp_pmufw_version() <= PMUFW_V1_0)) {
+		   (zynqmp_firmware_version() <= PMUFW_V1_0)) {
 		/* For bitstream which are aligned */
 		new_buf = buf;
 
@@ -205,7 +205,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize,
 	u32 ret_payload[PAYLOAD_ARG_CNT];
 	bool xilfpga_old = false;
 
-	if (zynqmp_pmufw_version() <= PMUFW_V1_0) {
+	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
 		puts("WARN: PMUFW v1.0 or less is detected\n");
 		puts("WARN: Not all bitstream formats are supported\n");
 		puts("WARN: Please upgrade PMUFW\n");
-- 
2.17.1

[U-Boot] [PATCH 10/13] arm64: zynqmp: remove old fw version function

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

Removes the old function to get the firmware version.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 arch/arm/mach-zynqmp/cpu.c                    | 23 -------------------
 arch/arm/mach-zynqmp/include/mach/sys_proto.h |  1 -
 2 files changed, 24 deletions(-)

diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c
index f28b964a1560..bb21cbcadf69 100644
--- a/arch/arm/mach-zynqmp/cpu.c
+++ b/arch/arm/mach-zynqmp/cpu.c
@@ -180,29 +180,6 @@ int __maybe_unused invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2,
 	return regs.regs[0];
 }
 
-unsigned int  __maybe_unused zynqmp_pmufw_version(void)
-{
-	int ret;
-	u32 ret_payload[PAYLOAD_ARG_CNT];
-	static u32 pm_api_version = ZYNQMP_PM_VERSION_INVALID;
-
-	/*
-	 * Get PMU version only once and later
-	 * just return stored values instead of
-	 * asking PMUFW again.
-	 */
-	if (pm_api_version == ZYNQMP_PM_VERSION_INVALID) {
-		ret = invoke_smc(ZYNQMP_SIP_SVC_GET_API_VERSION, 0, 0, 0, 0,
-				 ret_payload);
-		pm_api_version = ret_payload[1];
-
-		if (ret)
-			panic("PMUFW is not found - Please load it!\n");
-	}
-
-	return pm_api_version;
-}
-
 static int zynqmp_mmio_rawwrite(const u32 address,
 		      const u32 mask,
 		      const u32 value)
diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index 658974445417..27603a60ff8f 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -48,7 +48,6 @@ unsigned int zynqmp_get_silicon_version(void);
 
 void handoff_setup(void);
 
-unsigned int zynqmp_pmufw_version(void);
 int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value);
 int zynqmp_mmio_read(const u32 address, u32 *value);
 int invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3,
-- 
2.17.1

[U-Boot] [PATCH 11/13] arm64: zynqmp: probe firmware driver

[Michal Simek]

From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>

Probe ZynqMP firmware driver on the board initialization phase and
ensure that firmware is in place to continue execution. The probing is
done on board_init so it can be used for both SPL and U-Boot proper.

Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 arch/arm/Kconfig             |  3 +++
 board/xilinx/zynqmp/zynqmp.c | 18 ++++++------------
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 257a98d795e5..e4ec703139de 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1056,9 +1056,12 @@ config ARCH_ZYNQMP
 	select DM_SPI if SPI
 	select DM_SPI_FLASH if DM_SPI
 	select DM_USB if USB
+	select FIRMWARE
 	select OF_CONTROL
 	select SPL_BOARD_INIT if SPL
 	select SPL_CLK if SPL
+	select SPL_DM_MAILBOX if SPL
+	select SPL_FIRMWARE if SPL
 	select SPL_SEPARATE_BSS if SPL
 	select SUPPORT_SPL
 	imply BOARD_LATE_INIT
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 8a66d3e50aad..b94936474d7e 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -318,18 +318,6 @@ static char *zynqmp_get_silicon_idcode_name(void)
 int board_early_init_f(void)
 {
 	int ret = 0;
-#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_CLK_ZYNQMP)
-	u32 pm_api_version;
-
-	pm_api_version = zynqmp_firmware_version();
-	printf("PMUFW:\tv%d.%d\n",
-	       pm_api_version >> ZYNQMP_PM_VERSION_MAJOR_SHIFT,
-	       pm_api_version & ZYNQMP_PM_VERSION_MINOR_MASK);
-
-	if (pm_api_version < ZYNQMP_PM_VERSION)
-		panic("PMUFW version error. Expected: v%d.%d\n",
-		      ZYNQMP_PM_VERSION_MAJOR, ZYNQMP_PM_VERSION_MINOR);
-#endif
 
 #if defined(CONFIG_ZYNQMP_PSU_INIT_ENABLED)
 	ret = psu_init();
@@ -340,6 +328,12 @@ int board_early_init_f(void)
 
 int board_init(void)
 {
+	struct udevice *dev;
+
+	uclass_get_device_by_name(UCLASS_FIRMWARE, "zynqmp-power", &dev);
+	if (!dev)
+		panic("PMU Firmware device not found - Enable it");
+
 #if defined(CONFIG_SPL_BUILD)
 	/* Check *at build time* if the filename is an non-empty string */
 	if (sizeof(CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE) > 1)
-- 
2.17.1

[U-Boot] [PATCH 12/13] firmware: zynqmp: Separate function for sending message via mailbox

[Michal Simek]

U-Boot running in EL3 can't use SMC that's why there is a need to talk to
PMUFW directly via mailbox. The same logic is applied to all functions
which need to talk to PMUFW that's why move this logic to separate function
to avoid code duplication.

Also SMC request ID can be composed from PM_SIP_SVC offset that's why
ZYNQMP_SIP_SVC_GET_API_VERSION macro can be removed completely.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/firmware/firmware-zynqmp.c | 19 ++++++++++---------
 include/zynqmp_firmware.h          |  2 --
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index d70f34f24388..b7e3039c8337 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -40,6 +40,14 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
 	return 0;
 }
 
+static int send_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
+{
+	if (IS_ENABLED(CONFIG_SPL_BUILD))
+		return ipi_req(req, req_len, res, 2);
+
+	return invoke_smc(req[0] + PM_SIP_SVC, 0, 0, 0, 0, res);
+}
+
 unsigned int zynqmp_firmware_version(void)
 {
 	int ret;
@@ -52,16 +60,9 @@ unsigned int zynqmp_firmware_version(void)
 	 * asking PMUFW again.
 	 **/
 	if (pm_api_version == ZYNQMP_PM_VERSION_INVALID) {
-		if (IS_ENABLED(CONFIG_SPL_BUILD)) {
-			const u32 request[] = { PM_GET_API_VERSION };
-
-			ret = ipi_req(request, ARRAY_SIZE(request),
-				      ret_payload, 2);
-		} else {
-			ret = invoke_smc(ZYNQMP_SIP_SVC_GET_API_VERSION, 0, 0,
-					 0, 0, ret_payload);
-		};
+		const u32 request[] = { PM_GET_API_VERSION };
 
+		ret = send_req(request, ARRAY_SIZE(request), ret_payload, 2);
 		if (ret)
 			panic("PMUFW is not found - Please load it!\n");
 
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
index c522cae8399b..1fbc82414ab5 100644
--- a/include/zynqmp_firmware.h
+++ b/include/zynqmp_firmware.h
@@ -9,8 +9,6 @@
 #define _ZYNQM_FIRMWARE_H_
 
 #define PM_SIP_SVC      0xc2000000
-#define ZYNQMP_SIP_SVC_GET_API_VERSION          \
-	(PM_SIP_SVC + PM_GET_API_VERSION)
 #define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD       \
 	(PM_SIP_SVC + PM_SECURE_IMAGE)
 
-- 
2.17.1

[U-Boot] [PATCH 13/13] arm64: zynqmp: Use mailbox driver for PMUFW config loading

[Michal Simek]

With new mailbox driver PMUFW configuration object can be loaded via the
same interface and there is no need to have pmu_ipc.c completely.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 arch/arm/mach-zynqmp/Makefile                 |   4 -
 arch/arm/mach-zynqmp/include/mach/sys_proto.h |   2 -
 arch/arm/mach-zynqmp/pmu_ipc.c                | 112 ------------------
 drivers/firmware/firmware-zynqmp.c            |  24 ++++
 include/zynqmp_firmware.h                     |   1 +
 5 files changed, 25 insertions(+), 118 deletions(-)
 delete mode 100644 arch/arm/mach-zynqmp/pmu_ipc.c

diff --git a/arch/arm/mach-zynqmp/Makefile b/arch/arm/mach-zynqmp/Makefile
index f3765e45b1b9..8a3b0747244a 100644
--- a/arch/arm/mach-zynqmp/Makefile
+++ b/arch/arm/mach-zynqmp/Makefile
@@ -8,7 +8,3 @@ obj-y	+= cpu.o
 obj-$(CONFIG_MP)	+= mp.o
 obj-$(CONFIG_SPL_BUILD) += spl.o handoff.o
 obj-$(CONFIG_ZYNQMP_PSU_INIT_ENABLED)	+= psu_spl_init.o
-
-ifneq ($(CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE),"")
-obj-$(CONFIG_SPL_BUILD) += pmu_ipc.o
-endif
diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index 27603a60ff8f..69e729fb7625 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -60,6 +60,4 @@ int chip_id(unsigned char id);
 void tcm_init(u8 mode);
 #endif
 
-void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
-
 #endif /* _ASM_ARCH_SYS_PROTO_H */
diff --git a/arch/arm/mach-zynqmp/pmu_ipc.c b/arch/arm/mach-zynqmp/pmu_ipc.c
deleted file mode 100644
index d8858ea3ff99..000000000000
--- a/arch/arm/mach-zynqmp/pmu_ipc.c
+++ /dev/null
@@ -1,112 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Inter-Processor Communication with the Platform Management Unit (PMU)
- * firmware.
- *
- * (C) Copyright 2019 Luca Ceresoli
- * Luca Ceresoli <luca@lucaceresoli.net>
- */
-
-#include <common.h>
-#include <asm/io.h>
-#include <asm/arch/sys_proto.h>
-
-/* IPI bitmasks, register base and register offsets */
-#define IPI_BIT_MASK_APU      0x00001
-#define IPI_BIT_MASK_PMU0     0x10000
-#define IPI_REG_BASE_APU      0xFF300000
-#define IPI_REG_BASE_PMU0     0xFF330000
-#define IPI_REG_OFFSET_TRIG   0x00
-#define IPI_REG_OFFSET_OBR    0x04
-
-/* IPI mailbox buffer offsets */
-#define IPI_BUF_BASE_APU               0xFF990400
-#define IPI_BUF_OFFSET_TARGET_PMU      0x1C0
-#define IPI_BUF_OFFSET_REQ             0x00
-#define IPI_BUF_OFFSET_RESP            0x20
-
-#define PMUFW_PAYLOAD_ARG_CNT          8
-
-/* PMUFW commands */
-#define PMUFW_CMD_SET_CONFIGURATION    2
-
-static void pmu_ipc_send_request(const u32 *req, size_t req_len)
-{
-	u32 *mbx = (u32 *)(IPI_BUF_BASE_APU +
-			   IPI_BUF_OFFSET_TARGET_PMU +
-			   IPI_BUF_OFFSET_REQ);
-	size_t i;
-
-	for (i = 0; i < req_len; i++)
-		writel(req[i], &mbx[i]);
-}
-
-static void pmu_ipc_read_response(unsigned int *value, size_t count)
-{
-	u32 *mbx = (u32 *)(IPI_BUF_BASE_APU +
-			   IPI_BUF_OFFSET_TARGET_PMU +
-			   IPI_BUF_OFFSET_RESP);
-	size_t i;
-
-	for (i = 0; i < count; i++)
-		value[i] = readl(&mbx[i]);
-}
-
-/**
- * Send request to PMU and get the response.
- *
- * @req:        Request buffer. Byte 0 is the API ID, other bytes are optional
- *              parameters.
- * @req_len:    Request length in number of 32-bit words.
- * @res:        Response buffer. Byte 0 is the error code, other bytes are
- *              optional parameters. Optional, if @res_maxlen==0 the parameters
- *              will not be read.
- * @res_maxlen: Space allocated for the response in number of 32-bit words.
- *
- * @return Error code returned by the PMU (i.e. the first word of the response)
- */
-static int pmu_ipc_request(const u32 *req, size_t req_len,
-			   u32 *res, size_t res_maxlen)
-{
-	u32 status;
-
-	if (req_len > PMUFW_PAYLOAD_ARG_CNT ||
-	    res_maxlen > PMUFW_PAYLOAD_ARG_CNT)
-		return -EINVAL;
-
-	pmu_ipc_send_request(req, req_len);
-
-	/* Raise Inter-Processor Interrupt to PMU and wait for response */
-	writel(IPI_BIT_MASK_PMU0, IPI_REG_BASE_APU + IPI_REG_OFFSET_TRIG);
-	do {
-		status = readl(IPI_REG_BASE_APU + IPI_REG_OFFSET_OBR);
-	} while (status & IPI_BIT_MASK_PMU0);
-
-	pmu_ipc_read_response(res, res_maxlen);
-
-	return 0;
-}
-
-/**
- * Send a configuration object to the PMU firmware.
- *
- * @cfg_obj: Pointer to the configuration object
- * @size:    Size of @cfg_obj in bytes
- */
-void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
-{
-	const u32 request[] = {
-		PMUFW_CMD_SET_CONFIGURATION,
-		(u32)((u64)cfg_obj)
-	};
-	u32 response;
-	int err;
-
-	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
-
-	err = pmu_ipc_request(request,  ARRAY_SIZE(request), &response, 1);
-	if (err)
-		panic("Cannot load PMUFW configuration object (%d)\n", err);
-	if (response != 0)
-		panic("PMUFW returned 0x%08x status!\n", response);
-}
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index b7e3039c8337..7712019ec69c 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -75,6 +75,30 @@ unsigned int zynqmp_firmware_version(void)
 	return pm_api_version;
 };
 
+/**
+ * Send a configuration object to the PMU firmware.
+ *
+ * @cfg_obj: Pointer to the configuration object
+ * @size:    Size of @cfg_obj in bytes
+ */
+void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
+{
+	const u32 request[] = {
+		PM_SET_CONFIGURATION,
+		(u32)((u64)cfg_obj)
+	};
+	u32 response;
+	int err;
+
+	printf("Loading new PMUFW cfg obj (%ld bytes)\n", size);
+
+	err = send_req(request, ARRAY_SIZE(request), &response, 1);
+	if (err)
+		panic("Cannot load PMUFW configuration object (%d)\n", err);
+	if (response != 0)
+		panic("PMUFW returned 0x%08x status!\n", response);
+}
+
 static int zynqmp_power_probe(struct udevice *dev)
 {
 	int ret = 0;
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
index 1fbc82414ab5..2622a57e681e 100644
--- a/include/zynqmp_firmware.h
+++ b/include/zynqmp_firmware.h
@@ -32,5 +32,6 @@ enum pm_api_id {
 };
 
 unsigned int zynqmp_firmware_version(void);
+void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
 
 #endif /* _ZYNQMPPL_H_ */
-- 
2.17.1

[U-Boot] [PATCH 00/13] arm64: zynqmp: Clean communication with PMUFW

[Luca Ceresoli]

Hi Michal, Ibai,

On 27/09/19 15:34, Michal Simek wrote:
> Hi,
> 
> This patch series using ZynqMP firmware driver to provide a interface to
> communicate with the PMU Firmware. As part of the series a mailbox driver
> is also implemented to handle communication through ipi interface.
> 
> There are two new wiring:
> 1. Reading PMUFW version via firmware driver
>  - mailbox driver in case of SPL
>  - SMC in case of full U-Boot
> 2. Using the same patch for loading PMUFW configuration object
> 
> The whole series is based on several patches I have sent already that's why
> I am providing a branch which also contains this series.
> https://gitlab.denx.de/u-boot/custodians/u-boot-microblaze/tree/next
> 
> Thanks,
> Michal

Good to see this work going on, thanks!

I can't review in detail most of the patches, but I'm still replying
with some (mostly minor) comments.

-- 
Luca

[U-Boot] [PATCH 04/13] firmware: zynqmp: Add zynqmp-power support

[Luca Ceresoli]

Hi Ibai, Michal,

On 27/09/19 15:34, Michal Simek wrote:
> From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> 
> zynqmp-power driver for ZynqMP to handle the communication with the PMU
> firmware. Firmware driver just probes subnodes and power driver handles
> communication with PMU using the IPI mailbox driver.
> 
> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  drivers/firmware/Kconfig           |  2 ++
>  drivers/firmware/firmware-zynqmp.c | 40 ++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index b70a2063551c..9596ec16c7f7 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL
>  config ZYNQMP_FIRMWARE
>  	bool "ZynqMP Firmware interface"
>  	select FIRMWARE
> +	select ZYNQMP_IPI
> +	select DM_MAILBOX
>  	help
>  	  Firmware interface driver is used by different
>  	  drivers to communicate with the firmware for
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index 6644a7166ca0..b0930447b988 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -1,6 +1,46 @@
>  // SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx Zynq MPSoC Firmware driver
> + *
> + * Copyright (C) 2018-2019 Xilinx, Inc.
> + */
>  
> +#include <common.h>
>  #include <dm.h>
> +#include <mailbox.h>
> +#include <asm/arch/sys_proto.h>
> +
> +struct zynqmp_power {
> +	struct mbox_chan tx_chan;
> +	struct mbox_chan rx_chan;
> +} zynqmp_power;
> +
> +static int zynqmp_power_probe(struct udevice *dev)
> +{
> +	int ret = 0;
> +
> +	debug("%s, (dev=%p)\n", __func__, dev);
> +
> +	ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
> +	ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);

If these two calls return different error values, the binary or will
produce a nonsense 'ret' value. E.g. (-EINVAL | -ENODATA) equals -ENOTDIR.

Otherwise looks good.

-- 
Luca

[U-Boot] [PATCH 06/13] arm64: zynqmp: Cleanup PM SMC macro composition

[Luca Ceresoli]

Hi Michal,

On 27/09/19 15:34, Michal Simek wrote:
> Cleanup PM ID handling by using enum values.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> ---
> 
>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> index f25d414dcb1e..573c4ffceed9 100644
> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> @@ -10,7 +10,8 @@
>  #define PAYLOAD_ARG_CNT		5
>  
>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>  #define KEY_PTR_LEN	32
>  
>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
> @@ -21,7 +22,8 @@
>  
>  #define ZYNQMP_FPGA_AUTH_DDR	1
>  
> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>  
>  #define ZYNQMP_PM_VERSION_MAJOR		1
>  #define ZYNQMP_PM_VERSION_MINOR		0
> @@ -36,6 +38,13 @@
>  
>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>  
> +#define PM_SIP_SVC	0xc2000000
> +
> +enum pm_api_id {
> +	PM_GET_API_VERSION = 1,
> +	PM_SECURE_IMAGE = 45,
> +};
> +

This is a matter of personal taste, but I prefer to define values before
using them. So unless there is a good reason to define these values here
I'd rather move them before.

-- 
Luca

[U-Boot] [PATCH 12/13] firmware: zynqmp: Separate function for sending message via mailbox

[Luca Ceresoli]

Hi Michal,

On 27/09/19 15:35, Michal Simek wrote:
> U-Boot running in EL3 can't use SMC that's why there is a need to talk to
> PMUFW directly via mailbox. The same logic is applied to all functions
> which need to talk to PMUFW that's why move this logic to separate function
> to avoid code duplication.
> 
> Also SMC request ID can be composed from PM_SIP_SVC offset that's why
> ZYNQMP_SIP_SVC_GET_API_VERSION macro can be removed completely.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  drivers/firmware/firmware-zynqmp.c | 19 ++++++++++---------
>  include/zynqmp_firmware.h          |  2 --
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index d70f34f24388..b7e3039c8337 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -40,6 +40,14 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
>  	return 0;
>  }
>  
> +static int send_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
> +{
> +	if (IS_ENABLED(CONFIG_SPL_BUILD))
> +		return ipi_req(req, req_len, res, 2);

I guess the last parameter should be res_maxlen, not 2.

Other than than, looks good.

-- 
Luca

[U-Boot] [PATCH 13/13] arm64: zynqmp: Use mailbox driver for PMUFW config loading

[Luca Ceresoli]

Hi Michal,

On 27/09/19 15:35, Michal Simek wrote:
> With new mailbox driver PMUFW configuration object can be loaded via the
> same interface and there is no need to have pmu_ipc.c completely.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  arch/arm/mach-zynqmp/Makefile                 |   4 -
>  arch/arm/mach-zynqmp/include/mach/sys_proto.h |   2 -
>  arch/arm/mach-zynqmp/pmu_ipc.c                | 112 ------------------

Goodbye, beloved pmu_ipc.c... :)

Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[U-Boot] [PATCH 06/13] arm64: zynqmp: Cleanup PM SMC macro composition

[Michal Simek]

On 02. 10. 19 11:34, Luca Ceresoli wrote:
> Hi Michal,
> 
> On 27/09/19 15:34, Michal Simek wrote:
>> Cleanup PM ID handling by using enum values.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>> ---
>>
>>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>> index f25d414dcb1e..573c4ffceed9 100644
>> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>> @@ -10,7 +10,8 @@
>>  #define PAYLOAD_ARG_CNT		5
>>  
>>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
>> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
>> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
>> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>>  #define KEY_PTR_LEN	32
>>  
>>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
>> @@ -21,7 +22,8 @@
>>  
>>  #define ZYNQMP_FPGA_AUTH_DDR	1
>>  
>> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
>> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
>> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>>  
>>  #define ZYNQMP_PM_VERSION_MAJOR		1
>>  #define ZYNQMP_PM_VERSION_MINOR		0
>> @@ -36,6 +38,13 @@
>>  
>>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>>  
>> +#define PM_SIP_SVC	0xc2000000
>> +
>> +enum pm_api_id {
>> +	PM_GET_API_VERSION = 1,
>> +	PM_SECURE_IMAGE = 45,
>> +};
>> +
> 
> This is a matter of personal taste, but I prefer to define values before
> using them. So unless there is a good reason to define these values here
> I'd rather move them before.

ZYNQMP_SIP_SVC.. macros are still used. It is just changing a way how
they are defined because PM_SIP_SVC is really just SMC identification.

M

[U-Boot] [PATCH 06/13] arm64: zynqmp: Cleanup PM SMC macro composition

[Luca Ceresoli]

Hi Michal,

On 02/10/19 11:36, Michal Simek wrote:
> On 02. 10. 19 11:34, Luca Ceresoli wrote:
>> Hi Michal,
>>
>> On 27/09/19 15:34, Michal Simek wrote:
>>> Cleanup PM ID handling by using enum values.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>>> ---
>>>
>>>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>> index f25d414dcb1e..573c4ffceed9 100644
>>> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>> @@ -10,7 +10,8 @@
>>>  #define PAYLOAD_ARG_CNT		5
>>>  
>>>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
>>> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
>>> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
>>> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>>>  #define KEY_PTR_LEN	32
>>>  
>>>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
>>> @@ -21,7 +22,8 @@
>>>  
>>>  #define ZYNQMP_FPGA_AUTH_DDR	1
>>>  
>>> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
>>> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
>>> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>>>  
>>>  #define ZYNQMP_PM_VERSION_MAJOR		1
>>>  #define ZYNQMP_PM_VERSION_MINOR		0
>>> @@ -36,6 +38,13 @@
>>>  
>>>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>>>  
>>> +#define PM_SIP_SVC	0xc2000000
>>> +
>>> +enum pm_api_id {
>>> +	PM_GET_API_VERSION = 1,
>>> +	PM_SECURE_IMAGE = 45,
>>> +};
>>> +
>>
>> This is a matter of personal taste, but I prefer to define values before
>> using them. So unless there is a good reason to define these values here
>> I'd rather move them before.
> 
> ZYNQMP_SIP_SVC.. macros are still used. It is just changing a way how
> they are defined because PM_SIP_SVC is really just SMC identification.

My point is about order of lines in the file, not about patch order in
the series.

Let me clarify. The lines where PM_SECURE_IMAGE is used is above the
line there it is declared. Same for PM_GET_API_VERSION. I would (in this
same patch) declare enum pm_api_id above in the file.

I hope it is clearer now, sorry for the confusion.

-- 
Luca

[U-Boot] [PATCH 06/13] arm64: zynqmp: Cleanup PM SMC macro composition

[Michal Simek]

On 02. 10. 19 11:44, Luca Ceresoli wrote:
> Hi Michal,
> 
> On 02/10/19 11:36, Michal Simek wrote:
>> On 02. 10. 19 11:34, Luca Ceresoli wrote:
>>> Hi Michal,
>>>
>>> On 27/09/19 15:34, Michal Simek wrote:
>>>> Cleanup PM ID handling by using enum values.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>>>> ---
>>>>
>>>>  arch/arm/mach-zynqmp/include/mach/sys_proto.h | 13 +++++++++++--
>>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>>> index f25d414dcb1e..573c4ffceed9 100644
>>>> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>>> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
>>>> @@ -10,7 +10,8 @@
>>>>  #define PAYLOAD_ARG_CNT		5
>>>>  
>>>>  #define ZYNQMP_CSU_SILICON_VER_MASK	0xF
>>>> -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	0xC200002D
>>>> +#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD	\
>>>> +	(PM_SIP_SVC + PM_SECURE_IMAGE)
>>>>  #define KEY_PTR_LEN	32
>>>>  
>>>>  #define ZYNQMP_FPGA_BIT_AUTH_DDR	1
>>>> @@ -21,7 +22,8 @@
>>>>  
>>>>  #define ZYNQMP_FPGA_AUTH_DDR	1
>>>>  
>>>> -#define ZYNQMP_SIP_SVC_GET_API_VERSION		0xC2000001
>>>> +#define ZYNQMP_SIP_SVC_GET_API_VERSION		\
>>>> +	(PM_SIP_SVC + PM_GET_API_VERSION)
>>>>  
>>>>  #define ZYNQMP_PM_VERSION_MAJOR		1
>>>>  #define ZYNQMP_PM_VERSION_MINOR		0
>>>> @@ -36,6 +38,13 @@
>>>>  
>>>>  #define PMUFW_V1_0	((1 << ZYNQMP_PM_VERSION_MAJOR_SHIFT) | 0)
>>>>  
>>>> +#define PM_SIP_SVC	0xc2000000
>>>> +
>>>> +enum pm_api_id {
>>>> +	PM_GET_API_VERSION = 1,
>>>> +	PM_SECURE_IMAGE = 45,
>>>> +};
>>>> +
>>>
>>> This is a matter of personal taste, but I prefer to define values before
>>> using them. So unless there is a good reason to define these values here
>>> I'd rather move them before.
>>
>> ZYNQMP_SIP_SVC.. macros are still used. It is just changing a way how
>> they are defined because PM_SIP_SVC is really just SMC identification.
> 
> My point is about order of lines in the file, not about patch order in
> the series.
> 
> Let me clarify. The lines where PM_SECURE_IMAGE is used is above the
> line there it is declared. Same for PM_GET_API_VERSION. I would (in this
> same patch) declare enum pm_api_id above in the file.
> 
> I hope it is clearer now, sorry for the confusion.
> 

Ok. Got you. Let me look at it. It shouldn't be a problem in this patch
because that values are going to move anyway. But we can resort these
stuff in zynqmp_firmware.h.

M

[U-Boot] [PATCH 12/13] firmware: zynqmp: Separate function for sending message via mailbox

[Michal Simek]

On 02. 10. 19 11:34, Luca Ceresoli wrote:
> Hi Michal,
> 
> On 27/09/19 15:35, Michal Simek wrote:
>> U-Boot running in EL3 can't use SMC that's why there is a need to talk to
>> PMUFW directly via mailbox. The same logic is applied to all functions
>> which need to talk to PMUFW that's why move this logic to separate function
>> to avoid code duplication.
>>
>> Also SMC request ID can be composed from PM_SIP_SVC offset that's why
>> ZYNQMP_SIP_SVC_GET_API_VERSION macro can be removed completely.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  drivers/firmware/firmware-zynqmp.c | 19 ++++++++++---------
>>  include/zynqmp_firmware.h          |  2 --
>>  2 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
>> index d70f34f24388..b7e3039c8337 100644
>> --- a/drivers/firmware/firmware-zynqmp.c
>> +++ b/drivers/firmware/firmware-zynqmp.c
>> @@ -40,6 +40,14 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
>>  	return 0;
>>  }
>>  
>> +static int send_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
>> +{
>> +	if (IS_ENABLED(CONFIG_SPL_BUILD))
>> +		return ipi_req(req, req_len, res, 2);
> 
> I guess the last parameter should be res_maxlen, not 2.
> 
> Other than than, looks good.

good catch. Will fix.

M

[U-Boot] [PATCH 04/13] firmware: zynqmp: Add zynqmp-power support

[Michal Simek]

On 02. 10. 19 11:34, Luca Ceresoli wrote:
> Hi Ibai, Michal,
> 
> On 27/09/19 15:34, Michal Simek wrote:
>> From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>>
>> zynqmp-power driver for ZynqMP to handle the communication with the PMU
>> firmware. Firmware driver just probes subnodes and power driver handles
>> communication with PMU using the IPI mailbox driver.
>>
>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  drivers/firmware/Kconfig           |  2 ++
>>  drivers/firmware/firmware-zynqmp.c | 40 ++++++++++++++++++++++++++++++
>>  2 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> index b70a2063551c..9596ec16c7f7 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL
>>  config ZYNQMP_FIRMWARE
>>  	bool "ZynqMP Firmware interface"
>>  	select FIRMWARE
>> +	select ZYNQMP_IPI
>> +	select DM_MAILBOX
>>  	help
>>  	  Firmware interface driver is used by different
>>  	  drivers to communicate with the firmware for
>> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
>> index 6644a7166ca0..b0930447b988 100644
>> --- a/drivers/firmware/firmware-zynqmp.c
>> +++ b/drivers/firmware/firmware-zynqmp.c
>> @@ -1,6 +1,46 @@
>>  // SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Xilinx Zynq MPSoC Firmware driver
>> + *
>> + * Copyright (C) 2018-2019 Xilinx, Inc.
>> + */
>>  
>> +#include <common.h>
>>  #include <dm.h>
>> +#include <mailbox.h>
>> +#include <asm/arch/sys_proto.h>
>> +
>> +struct zynqmp_power {
>> +	struct mbox_chan tx_chan;
>> +	struct mbox_chan rx_chan;
>> +} zynqmp_power;
>> +
>> +static int zynqmp_power_probe(struct udevice *dev)
>> +{
>> +	int ret = 0;
>> +
>> +	debug("%s, (dev=%p)\n", __func__, dev);
>> +
>> +	ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan);
>> +	ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan);
> 
> If these two calls return different error values, the binary or will
> produce a nonsense 'ret' value. E.g. (-EINVAL | -ENODATA) equals -ENOTDIR.
> 
> Otherwise looks good.
> 

Let's fix this. Also mbox_send/mbox_recv from firmware-zynqmp.c should
be fixed too.

Thanks,
Michal