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

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

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)

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

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, +};

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" },

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.

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

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

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,

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.

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

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.

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

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[] = {

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

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");

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,

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)

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)

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.

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

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

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

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:
- Reading PMUFW version via firmware driver
- mailbox driver in case of SPL
- SMC in case of full U-Boot
- 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.
participants (2)
-
Luca Ceresoli
-
Michal Simek