[U-Boot] [RFC v2 0/2] arm64: zynqmp: pass a PMUFW config object

[U-Boot] [RFC v2 0/2] arm64: zynqmp: pass a PMUFW config object

[Luca Ceresoli]

Hi,

a long-standing issue in the ZynqMP users community is the management on
the PMU firmware configuration object when U-Boot SPL is used.

The Platform Management Unit (PMU) needs a configuration object (cfg obj)
to know how to operate the SoC. When using the "Xilinx workflow", the
Xilinx FSBL (First Stage Bootloader) has the SPL role. FSBL has a built-in
cfg obj and passes it to the PMUFW at runtime before jumping to U-Boot
proper.

This is just not implemented in the U-Boot code. The best workaround for
U-Boot SPL users is to patch [0] the PMUFW itself to have the cfg obj
built-in and self-load it. This approach has some drawbacks: among others,
it forces to use a different PMUFW binary for each hardware and hardware
configuation. It also makes it impossible to change the configuration after
boot.

This patchset is a first attempt at filling the gap by allowing U-Boot SPL
to load the cfg obj firmware at runtime. It adds a Kconfig string option to
point to the cfg obj in the form of a C file as produced by Xilinx tools
(usually called pm_cfg_obj.c). If the option is non-empty, code is enabled
to compile that file in U-Boot SPL and send the configuration to PMUFW.

The next development target will be to load the cfg obj from a binary blob
instead of compiling it from a .c file. With that in place, patch 1 will be
removed and patch 2 cleaned up.

These patches are tested on the UltraZed EV board.

Since RFC v1 [1] the loading has been reimplemented in SPL (it was in
U-Boot proper). Now it does not rely on ARM Trusted Firmware but instead
talks directly with the PMU via IPI mailboxes.

[0] https://github.com/topic-embedded-products/meta-topic/blob/master/recipes-bsp/pmu-firmware/pmu-firmware/0001-Load-XPm_ConfigObject-at-boot.patch
[1] https://lists.denx.de/pipermail/u-boot/2019-February/360450.html

Luca


Luca Ceresoli (2):
  zynqmp: add minimal include files to build a pm_cfg_obj.c
  arm64: zynqmp: spl: install a PMU firmware config object at runtime

 board/xilinx/zynqmp/Kconfig      |  26 ++++
 board/xilinx/zynqmp/Makefile     |   5 +
 board/xilinx/zynqmp/pm_defs.h    | 254 +++++++++++++++++++++++++++++++
 board/xilinx/zynqmp/pmu_global.h |   0
 board/xilinx/zynqmp/pmu_ipc.c    | 116 ++++++++++++++
 board/xilinx/zynqmp/pmu_ipc.h    |  14 ++
 board/xilinx/zynqmp/xil_types.h  |   1 +
 board/xilinx/zynqmp/zynqmp.c     |  11 ++
 8 files changed, 427 insertions(+)
 create mode 100644 board/xilinx/zynqmp/pm_defs.h
 create mode 100644 board/xilinx/zynqmp/pmu_global.h
 create mode 100644 board/xilinx/zynqmp/pmu_ipc.c
 create mode 100644 board/xilinx/zynqmp/pmu_ipc.h
 create mode 100644 board/xilinx/zynqmp/xil_types.h

-- 
2.17.1

[U-Boot] [RFC v2 1/2] zynqmp: add minimal include files to build a pm_cfg_obj.c

[Luca Ceresoli]

A following commit will allow U-Boot to pass a configuration object to
the ZynqMP PMU firmware. This configuration object is generated by
Xilinx tools in the form of a C file (pm_cfg_obj.c), which #includes a
few headers with constants definitions.

In order to allow pm_cfg_obj.c to build, include in U-Boot a minimal
version of those headers files:

 - pm_defs: a copy of [0], reduced to remove unneeded values
 - pmu_global.h: empty file, it is included but not really needed
 - xil_types.h: just includes common.h which has all the needed types

[0] https://github.com/Xilinx/embeddedsw/blob/xilinx-v2018.3/lib/sw_services/xilpm/src/common/pm_defs.h

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 board/xilinx/zynqmp/pm_defs.h    | 254 +++++++++++++++++++++++++++++++
 board/xilinx/zynqmp/pmu_global.h |   0
 board/xilinx/zynqmp/xil_types.h  |   1 +
 3 files changed, 255 insertions(+)
 create mode 100644 board/xilinx/zynqmp/pm_defs.h
 create mode 100644 board/xilinx/zynqmp/pmu_global.h
 create mode 100644 board/xilinx/zynqmp/xil_types.h

diff --git a/board/xilinx/zynqmp/pm_defs.h b/board/xilinx/zynqmp/pm_defs.h
new file mode 100644
index 000000000000..a339d5ef4fd1
--- /dev/null
+++ b/board/xilinx/zynqmp/pm_defs.h
@@ -0,0 +1,254 @@
+/******************************************************************************
+*
+* Copyright (C) 2015-2018 Xilinx, Inc.  All rights reserved.
+*
+* Permission is hereby granted, free of charge, to any person obtaining a copy
+* of this software and associated documentation files (the "Software"), to deal
+* in the Software without restriction, including without limitation the rights
+* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+* copies of the Software, and to permit persons to whom the Software is
+* furnished to do so, subject to the following conditions:
+*
+* The above copyright notice and this permission notice shall be included in
+* all copies or substantial portions of the Software.
+*
+* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+* XILINX  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
+* OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+* SOFTWARE.
+*
+* Except as contained in this notice, the name of the Xilinx shall not be used
+* in advertising or otherwise to promote the sale, use or other dealings in
+* this Software without prior written authorization from Xilinx.
+*
+******************************************************************************/
+
+/*****************************************************************************/
+/**
+ * @file pm_defs.h
+ *
+ * PM Definitions implementation
+ * @addtogroup xpm_apis XilPM APIs
+ * @{
+ *****************************************************************************/
+
+#ifndef PM_DEFS_H_
+#define PM_DEFS_H_
+
+/** @name Capabilities for RAM
+ *
+ * @{
+ */
+#define PM_CAP_ACCESS	0x1U
+#define PM_CAP_CONTEXT	0x2U
+#define PM_CAP_WAKEUP	0x4U
+/**@}*/
+
+/**
+ * PM Node ID Enum
+ */
+enum XPmNodeId {
+	NODE_UNKNOWN,
+	NODE_APU,
+	NODE_APU_0,
+	NODE_APU_1,
+	NODE_APU_2,
+	NODE_APU_3,
+	NODE_RPU,
+	NODE_RPU_0,
+	NODE_RPU_1,
+	NODE_PLD,
+	NODE_FPD,
+	NODE_OCM_BANK_0,
+	NODE_OCM_BANK_1,
+	NODE_OCM_BANK_2,
+	NODE_OCM_BANK_3,
+	NODE_TCM_0_A,
+	NODE_TCM_0_B,
+	NODE_TCM_1_A,
+	NODE_TCM_1_B,
+	NODE_L2,
+	NODE_GPU_PP_0,
+	NODE_GPU_PP_1,
+	NODE_USB_0,
+	NODE_USB_1,
+	NODE_TTC_0,
+	NODE_TTC_1,
+	NODE_TTC_2,
+	NODE_TTC_3,
+	NODE_SATA,
+	NODE_ETH_0,
+	NODE_ETH_1,
+	NODE_ETH_2,
+	NODE_ETH_3,
+	NODE_UART_0,
+	NODE_UART_1,
+	NODE_SPI_0,
+	NODE_SPI_1,
+	NODE_I2C_0,
+	NODE_I2C_1,
+	NODE_SD_0,
+	NODE_SD_1,
+	NODE_DP,
+	NODE_GDMA,
+	NODE_ADMA,
+	NODE_NAND,
+	NODE_QSPI,
+	NODE_GPIO,
+	NODE_CAN_0,
+	NODE_CAN_1,
+	NODE_EXTERN,
+	NODE_APLL,
+	NODE_VPLL,
+	NODE_DPLL,
+	NODE_RPLL,
+	NODE_IOPLL,
+	NODE_DDR,
+	NODE_IPI_APU,
+	NODE_IPI_RPU_0,
+	NODE_GPU,
+	NODE_PCIE,
+	NODE_PCAP,
+	NODE_RTC,
+	NODE_LPD,
+	NODE_VCU,
+	NODE_IPI_RPU_1,
+	NODE_IPI_PL_0,
+	NODE_IPI_PL_1,
+	NODE_IPI_PL_2,
+	NODE_IPI_PL_3,
+	NODE_PL,
+	NODE_ID_MAX
+};
+
+/**
+ *  PM Reset Line IDs
+ */
+enum XPmReset {
+	XILPM_RESET_PCIE_CFG = 1000,
+	XILPM_RESET_PCIE_BRIDGE,
+	XILPM_RESET_PCIE_CTRL,
+	XILPM_RESET_DP,
+	XILPM_RESET_SWDT_CRF,
+	XILPM_RESET_AFI_FM5,
+	XILPM_RESET_AFI_FM4,
+	XILPM_RESET_AFI_FM3,
+	XILPM_RESET_AFI_FM2,
+	XILPM_RESET_AFI_FM1,
+	XILPM_RESET_AFI_FM0,
+	XILPM_RESET_GDMA,
+	XILPM_RESET_GPU_PP1,
+	XILPM_RESET_GPU_PP0,
+	XILPM_RESET_GPU,
+	XILPM_RESET_GT,
+	XILPM_RESET_SATA,
+	XILPM_RESET_ACPU3_PWRON,
+	XILPM_RESET_ACPU2_PWRON,
+	XILPM_RESET_ACPU1_PWRON,
+	XILPM_RESET_ACPU0_PWRON,
+	XILPM_RESET_APU_L2,
+	XILPM_RESET_ACPU3,
+	XILPM_RESET_ACPU2,
+	XILPM_RESET_ACPU1,
+	XILPM_RESET_ACPU0,
+	XILPM_RESET_DDR,
+	XILPM_RESET_APM_FPD,
+	XILPM_RESET_SOFT,
+	XILPM_RESET_GEM0,
+	XILPM_RESET_GEM1,
+	XILPM_RESET_GEM2,
+	XILPM_RESET_GEM3,
+	XILPM_RESET_QSPI,
+	XILPM_RESET_UART0,
+	XILPM_RESET_UART1,
+	XILPM_RESET_SPI0,
+	XILPM_RESET_SPI1,
+	XILPM_RESET_SDIO0,
+	XILPM_RESET_SDIO1,
+	XILPM_RESET_CAN0,
+	XILPM_RESET_CAN1,
+	XILPM_RESET_I2C0,
+	XILPM_RESET_I2C1,
+	XILPM_RESET_TTC0,
+	XILPM_RESET_TTC1,
+	XILPM_RESET_TTC2,
+	XILPM_RESET_TTC3,
+	XILPM_RESET_SWDT_CRL,
+	XILPM_RESET_NAND,
+	XILPM_RESET_ADMA,
+	XILPM_RESET_GPIO,
+	XILPM_RESET_IOU_CC,
+	XILPM_RESET_TIMESTAMP,
+	XILPM_RESET_RPU_R50,
+	XILPM_RESET_RPU_R51,
+	XILPM_RESET_RPU_AMBA,
+	XILPM_RESET_OCM,
+	XILPM_RESET_RPU_PGE,
+	XILPM_RESET_USB0_CORERESET,
+	XILPM_RESET_USB1_CORERESET,
+	XILPM_RESET_USB0_HIBERRESET,
+	XILPM_RESET_USB1_HIBERRESET,
+	XILPM_RESET_USB0_APB,
+	XILPM_RESET_USB1_APB,
+	XILPM_RESET_IPI,
+	XILPM_RESET_APM_LPD,
+	XILPM_RESET_RTC,
+	XILPM_RESET_SYSMON,
+	XILPM_RESET_AFI_FM6,
+	XILPM_RESET_LPD_SWDT,
+	XILPM_RESET_FPD,
+	XILPM_RESET_RPU_DBG1,
+	XILPM_RESET_RPU_DBG0,
+	XILPM_RESET_DBG_LPD,
+	XILPM_RESET_DBG_FPD,
+	XILPM_RESET_APLL,
+	XILPM_RESET_DPLL,
+	XILPM_RESET_VPLL,
+	XILPM_RESET_IOPLL,
+	XILPM_RESET_RPLL,
+	XILPM_RESET_GPO3_PL_0,
+	XILPM_RESET_GPO3_PL_1,
+	XILPM_RESET_GPO3_PL_2,
+	XILPM_RESET_GPO3_PL_3,
+	XILPM_RESET_GPO3_PL_4,
+	XILPM_RESET_GPO3_PL_5,
+	XILPM_RESET_GPO3_PL_6,
+	XILPM_RESET_GPO3_PL_7,
+	XILPM_RESET_GPO3_PL_8,
+	XILPM_RESET_GPO3_PL_9,
+	XILPM_RESET_GPO3_PL_10,
+	XILPM_RESET_GPO3_PL_11,
+	XILPM_RESET_GPO3_PL_12,
+	XILPM_RESET_GPO3_PL_13,
+	XILPM_RESET_GPO3_PL_14,
+	XILPM_RESET_GPO3_PL_15,
+	XILPM_RESET_GPO3_PL_16,
+	XILPM_RESET_GPO3_PL_17,
+	XILPM_RESET_GPO3_PL_18,
+	XILPM_RESET_GPO3_PL_19,
+	XILPM_RESET_GPO3_PL_20,
+	XILPM_RESET_GPO3_PL_21,
+	XILPM_RESET_GPO3_PL_22,
+	XILPM_RESET_GPO3_PL_23,
+	XILPM_RESET_GPO3_PL_24,
+	XILPM_RESET_GPO3_PL_25,
+	XILPM_RESET_GPO3_PL_26,
+	XILPM_RESET_GPO3_PL_27,
+	XILPM_RESET_GPO3_PL_28,
+	XILPM_RESET_GPO3_PL_29,
+	XILPM_RESET_GPO3_PL_30,
+	XILPM_RESET_GPO3_PL_31,
+	XILPM_RESET_RPU_LS,
+	XILPM_RESET_PS_ONLY,
+	XILPM_RESET_PL,
+	XILPM_RESET_GPIO5_EMIO_92,
+	XILPM_RESET_GPIO5_EMIO_93,
+	XILPM_RESET_GPIO5_EMIO_94,
+	XILPM_RESET_GPIO5_EMIO_95,
+};
+
+ /** @} */
+#endif /* PM_DEFS_H_ */
diff --git a/board/xilinx/zynqmp/pmu_global.h b/board/xilinx/zynqmp/pmu_global.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/board/xilinx/zynqmp/xil_types.h b/board/xilinx/zynqmp/xil_types.h
new file mode 100644
index 000000000000..65e11030d85e
--- /dev/null
+++ b/board/xilinx/zynqmp/xil_types.h
@@ -0,0 +1 @@
+#include <common.h>
-- 
2.17.1

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Luca Ceresoli]

Optionally allow U-Boot to load at the PMU firmware configuration object
into the Power Management Unit (PMU) on Xilinx ZynqMP.

The configuration object is required by the PMU FW to enable most SoC
peripherals. So far the only way to boot using U-Boot SPL was to hard-code
the configuration object in the PMU firmware. Allow a different boot
process, where the PMU FW is equal for any ZynqMP chip and its
configuration is passed at runtime by U-Boot SPL.

All the code for Inter-processor communication with the PMU is isolated in
a new file (pmu_ipc.c). The code is inspired by the same feature as
implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
Firmware:

 * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
 * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357

The load is logged on the console during boot:

  U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
  Loading PMUFW cfg obj (2008 bytes)
  EL Level:	EL3
  ...

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

---

Changes RFC v1 -> RFC v2:
 - Load the cfg_obj in SPL, not U-Boot proper: this required a complete
   reimplementation since we cannot rely on ATF now
 - Update and refine the Kconfig option help

Todo:
 - don't compile pm_cfg_obj.c from source, load it from a binary file
---
 board/xilinx/zynqmp/Kconfig   |  26 ++++++++
 board/xilinx/zynqmp/Makefile  |   5 ++
 board/xilinx/zynqmp/pmu_ipc.c | 116 ++++++++++++++++++++++++++++++++++
 board/xilinx/zynqmp/pmu_ipc.h |  14 ++++
 board/xilinx/zynqmp/zynqmp.c  |  11 ++++
 5 files changed, 172 insertions(+)
 create mode 100644 board/xilinx/zynqmp/pmu_ipc.c
 create mode 100644 board/xilinx/zynqmp/pmu_ipc.h

diff --git a/board/xilinx/zynqmp/Kconfig b/board/xilinx/zynqmp/Kconfig
index 7d1f7398c3e9..4201e38571e7 100644
--- a/board/xilinx/zynqmp/Kconfig
+++ b/board/xilinx/zynqmp/Kconfig
@@ -15,4 +15,30 @@ config CMD_ZYNQMP
 	  and authentication feature enabled while generating
 	  BOOT.BIN using Xilinx bootgen tool.
 
+config ZYNQMP_LOAD_PM_CFG_OBJ_FILE
+	string "PMU firmware configuration object to load at runtime"
+	help
+	  Path to a PMU firmware configuration object to be built into
+	  U-Boot and loaded at runtime by SPL into the PMU firmware.
+
+	  The ZynqMP Power Management Unit (PMU) needs a configuration
+	  object for most SoC peripherals to work. It can be either
+	  hard-coded in the PMUFW or passed at runtime.
+
+	  If the configuration object is not hard-coded in the PMU
+	  firmware, U-Boot SPL can load it at runtime. To enable this
+	  feature set here the file name (absolute path or relative to
+	  board/xilinx/zynqmp/). It will be loaded into the PMU firmware by
+	  U-Boot SPL during board initialization.
+
+	  Note: if your pm_cfg_obj.c is generated by the Xilinx tools,
+	  you must remove any linking directives from the
+	  XPm_ConfigObject declaration! E.g.:
+
+	    -const u32 XPm_ConfigObject[] __attribute__((used, section(".sys_cfg_data"))) = {
+	    +const u32 XPm_ConfigObject[] = {
+
+	  Leave this option empty if your PMU firmware has a hard-coded
+	  configuration object or you are loading it by any other means.
+
 endif
diff --git a/board/xilinx/zynqmp/Makefile b/board/xilinx/zynqmp/Makefile
index 80f8ca7e1e4b..0d8f52ecd631 100644
--- a/board/xilinx/zynqmp/Makefile
+++ b/board/xilinx/zynqmp/Makefile
@@ -33,6 +33,11 @@ ifneq ($(call ifdef_any_of, CONFIG_ZYNQMP_PSU_INIT_ENABLED CONFIG_SPL_BUILD),)
 obj-y += $(init-objs)
 endif
 
+ifneq ($(CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE),"")
+CFLAGS_zynqmp.o += -DZYNQMP_LOAD_PM_CFG_OBJ -I$(srctree)/$(src)
+obj-$(CONFIG_SPL_BUILD) += pmu_ipc.o
+endif
+
 obj-$(CONFIG_MMC_SDHCI_ZYNQ) += tap_delays.o
 
 ifndef CONFIG_SPL_BUILD
diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
new file mode 100644
index 000000000000..6306d33d6f17
--- /dev/null
+++ b/board/xilinx/zynqmp/pmu_ipc.c
@@ -0,0 +1,116 @@
+// 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 "pmu_ipc.h"
+#include <common.h>
+#include <asm/io.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
+
+/* Xilinx FSBL sets 8, ATF sets 6. Which one is correct? */
+#define PMUFW_PAYLOAD_ARG_CNT          6
+
+/* PMUFW commands */
+#define PMUFW_CMD_SET_CONFIGURATION    2
+
+static void pmu_ipc_send_request(const u32 *req, size_t req_len)
+{
+	u32 *mbx = 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 = 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
+	const u32 request[] = {
+		PMUFW_CMD_SET_CONFIGURATION,
+		CONFIG_SPL_TEXT_BASE
+	};
+	u32 response;
+	int err;
+
+	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
+
+	memcpy(ocm, cfg_obj, 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/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
new file mode 100644
index 000000000000..37bb72c1b20a
--- /dev/null
+++ b/board/xilinx/zynqmp/pmu_ipc.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * (C) Copyright 2019 Luca Ceresoli
+ * Luca Ceresoli <luca@lucaceresoli.net>
+ */
+
+#ifndef __ZYNQMP_PMU_IPC_H__
+#define __ZYNQMP_PMU_IPC_H__
+
+#include <linux/types.h>
+
+void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
+
+#endif /* __ZYNQMP_PMU_IPC_H__ */
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 5e1d2116bc32..1d5e25961863 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -4,6 +4,8 @@
  * Michal Simek <michal.simek@xilinx.com>
  */
 
+#include "pmu_ipc.h"
+
 #include <common.h>
 #include <sata.h>
 #include <ahci.h>
@@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
 }
 #endif
 
+#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
+#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
+#endif
+
 int board_early_init_f(void)
 {
 	int ret = 0;
@@ -332,6 +338,11 @@ int board_early_init_f(void)
 
 int board_init(void)
 {
+#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
+	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
+					sizeof(XPm_ConfigObject));
+#endif
+
 	printf("EL Level:\tEL%d\n", current_el());
 
 #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \
-- 
2.17.1

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Michal Simek]

On 21. 03. 19 16:48, Luca Ceresoli wrote:
> Optionally allow U-Boot to load at the PMU firmware configuration object
> into the Power Management Unit (PMU) on Xilinx ZynqMP.
> 
> The configuration object is required by the PMU FW to enable most SoC
> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
> the configuration object in the PMU firmware. Allow a different boot
> process, where the PMU FW is equal for any ZynqMP chip and its
> configuration is passed at runtime by U-Boot SPL.
> 
> All the code for Inter-processor communication with the PMU is isolated in
> a new file (pmu_ipc.c). The code is inspired by the same feature as
> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
> Firmware:
> 
>  * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>  * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
> 
> The load is logged on the console during boot:
> 
>   U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>   Loading PMUFW cfg obj (2008 bytes)
>   EL Level:	EL3
>   ...
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
> ---
> 
> Changes RFC v1 -> RFC v2:
>  - Load the cfg_obj in SPL, not U-Boot proper: this required a complete
>    reimplementation since we cannot rely on ATF now
>  - Update and refine the Kconfig option help
> 
> Todo:
>  - don't compile pm_cfg_obj.c from source, load it from a binary file
> ---
>  board/xilinx/zynqmp/Kconfig   |  26 ++++++++
>  board/xilinx/zynqmp/Makefile  |   5 ++
>  board/xilinx/zynqmp/pmu_ipc.c | 116 ++++++++++++++++++++++++++++++++++
>  board/xilinx/zynqmp/pmu_ipc.h |  14 ++++
>  board/xilinx/zynqmp/zynqmp.c  |  11 ++++
>  5 files changed, 172 insertions(+)
>  create mode 100644 board/xilinx/zynqmp/pmu_ipc.c
>  create mode 100644 board/xilinx/zynqmp/pmu_ipc.h
> 
> diff --git a/board/xilinx/zynqmp/Kconfig b/board/xilinx/zynqmp/Kconfig
> index 7d1f7398c3e9..4201e38571e7 100644
> --- a/board/xilinx/zynqmp/Kconfig
> +++ b/board/xilinx/zynqmp/Kconfig
> @@ -15,4 +15,30 @@ config CMD_ZYNQMP
>  	  and authentication feature enabled while generating
>  	  BOOT.BIN using Xilinx bootgen tool.
>  
> +config ZYNQMP_LOAD_PM_CFG_OBJ_FILE
> +	string "PMU firmware configuration object to load at runtime"
> +	help
> +	  Path to a PMU firmware configuration object to be built into
> +	  U-Boot and loaded at runtime by SPL into the PMU firmware.
> +
> +	  The ZynqMP Power Management Unit (PMU) needs a configuration
> +	  object for most SoC peripherals to work. It can be either
> +	  hard-coded in the PMUFW or passed at runtime.
> +
> +	  If the configuration object is not hard-coded in the PMU
> +	  firmware, U-Boot SPL can load it at runtime. To enable this
> +	  feature set here the file name (absolute path or relative to
> +	  board/xilinx/zynqmp/). It will be loaded into the PMU firmware by
> +	  U-Boot SPL during board initialization.
> +
> +	  Note: if your pm_cfg_obj.c is generated by the Xilinx tools,
> +	  you must remove any linking directives from the
> +	  XPm_ConfigObject declaration! E.g.:
> +
> +	    -const u32 XPm_ConfigObject[] __attribute__((used, section(".sys_cfg_data"))) = {
> +	    +const u32 XPm_ConfigObject[] = {
> +
> +	  Leave this option empty if your PMU firmware has a hard-coded
> +	  configuration object or you are loading it by any other means.
> +
>  endif
> diff --git a/board/xilinx/zynqmp/Makefile b/board/xilinx/zynqmp/Makefile
> index 80f8ca7e1e4b..0d8f52ecd631 100644
> --- a/board/xilinx/zynqmp/Makefile
> +++ b/board/xilinx/zynqmp/Makefile
> @@ -33,6 +33,11 @@ ifneq ($(call ifdef_any_of, CONFIG_ZYNQMP_PSU_INIT_ENABLED CONFIG_SPL_BUILD),)
>  obj-y += $(init-objs)
>  endif
>  
> +ifneq ($(CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE),"")
> +CFLAGS_zynqmp.o += -DZYNQMP_LOAD_PM_CFG_OBJ -I$(srctree)/$(src)
> +obj-$(CONFIG_SPL_BUILD) += pmu_ipc.o
> +endif
> +
>  obj-$(CONFIG_MMC_SDHCI_ZYNQ) += tap_delays.o
>  
>  ifndef CONFIG_SPL_BUILD
> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
> new file mode 100644
> index 000000000000..6306d33d6f17
> --- /dev/null
> +++ b/board/xilinx/zynqmp/pmu_ipc.c
> @@ -0,0 +1,116 @@
> +// 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 "pmu_ipc.h"
> +#include <common.h>
> +#include <asm/io.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
> +
> +/* Xilinx FSBL sets 8, ATF sets 6. Which one is correct? */
> +#define PMUFW_PAYLOAD_ARG_CNT          6
> +
> +/* PMUFW commands */
> +#define PMUFW_CMD_SET_CONFIGURATION    2
> +
> +static void pmu_ipc_send_request(const u32 *req, size_t req_len)
> +{
> +	u32 *mbx = 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 = 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;
> +}

All above should be mailbox driver. It means this should go to
drivers/mailbox and be split to mbox send/recv functions.

But I have no problem to use this configuration in the first patch and
move to mbox driver in separate patch.

> +
> +/**
> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
> +	const u32 request[] = {
> +		PMUFW_CMD_SET_CONFIGURATION,
> +		CONFIG_SPL_TEXT_BASE
> +	};
> +	u32 response;
> +	int err;
> +
> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
> +
> +	memcpy(ocm, cfg_obj, 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);
> +}

And this can stay here or go to arch/arm/mach-zynq/

> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
> new file mode 100644
> index 000000000000..37bb72c1b20a
> --- /dev/null
> +++ b/board/xilinx/zynqmp/pmu_ipc.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * (C) Copyright 2019 Luca Ceresoli
> + * Luca Ceresoli <luca@lucaceresoli.net>
> + */
> +
> +#ifndef __ZYNQMP_PMU_IPC_H__
> +#define __ZYNQMP_PMU_IPC_H__
> +
> +#include <linux/types.h>
> +
> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
> +


arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.



> +#endif /* __ZYNQMP_PMU_IPC_H__ */
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 5e1d2116bc32..1d5e25961863 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -4,6 +4,8 @@
>   * Michal Simek <michal.simek@xilinx.com>
>   */
>  
> +#include "pmu_ipc.h"
> +
>  #include <common.h>
>  #include <sata.h>
>  #include <ahci.h>
> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>  }
>  #endif
>  
> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
> +#endif
> +
>  int board_early_init_f(void)
>  {
>  	int ret = 0;
> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>  
>  int board_init(void)
>  {
> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
> +					sizeof(XPm_ConfigObject));
> +#endif

As we discussed over IRC. I think that this should be simply bin
firmware file compare to C built by u-boot.

Thanks,
Michal

[U-Boot] [RFC v2 1/2] zynqmp: add minimal include files to build a pm_cfg_obj.c

[Michal Simek]

On 21. 03. 19 16:48, Luca Ceresoli wrote:
> A following commit will allow U-Boot to pass a configuration object to
> the ZynqMP PMU firmware. This configuration object is generated by
> Xilinx tools in the form of a C file (pm_cfg_obj.c), which #includes a
> few headers with constants definitions.
> 
> In order to allow pm_cfg_obj.c to build, include in U-Boot a minimal
> version of those headers files:
> 
>  - pm_defs: a copy of [0], reduced to remove unneeded values
>  - pmu_global.h: empty file, it is included but not really needed
>  - xil_types.h: just includes common.h which has all the needed types
> 
> [0] https://github.com/Xilinx/embeddedsw/blob/xilinx-v2018.3/lib/sw_services/xilpm/src/common/pm_defs.h
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  board/xilinx/zynqmp/pm_defs.h    | 254 +++++++++++++++++++++++++++++++
>  board/xilinx/zynqmp/pmu_global.h |   0
>  board/xilinx/zynqmp/xil_types.h  |   1 +
>  3 files changed, 255 insertions(+)
>  create mode 100644 board/xilinx/zynqmp/pm_defs.h
>  create mode 100644 board/xilinx/zynqmp/pmu_global.h
>  create mode 100644 board/xilinx/zynqmp/xil_types.h
> 
> diff --git a/board/xilinx/zynqmp/pm_defs.h b/board/xilinx/zynqmp/pm_defs.h
> new file mode 100644
> index 000000000000..a339d5ef4fd1
> --- /dev/null
> +++ b/board/xilinx/zynqmp/pm_defs.h
> @@ -0,0 +1,254 @@
> +/******************************************************************************
> +*
> +* Copyright (C) 2015-2018 Xilinx, Inc.  All rights reserved.
> +*
> +* Permission is hereby granted, free of charge, to any person obtaining a copy
> +* of this software and associated documentation files (the "Software"), to deal
> +* in the Software without restriction, including without limitation the rights
> +* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> +* copies of the Software, and to permit persons to whom the Software is
> +* furnished to do so, subject to the following conditions:
> +*
> +* The above copyright notice and this permission notice shall be included in
> +* all copies or substantial portions of the Software.
> +*
> +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> +* XILINX  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> +* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
> +* OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +* SOFTWARE.
> +*
> +* Except as contained in this notice, the name of the Xilinx shall not be used
> +* in advertising or otherwise to promote the sale, use or other dealings in
> +* this Software without prior written authorization from Xilinx.
> +*
> +******************************************************************************/
> +
> +/*****************************************************************************/
> +/**
> + * @file pm_defs.h
> + *
> + * PM Definitions implementation
> + * @addtogroup xpm_apis XilPM APIs
> + * @{
> + *****************************************************************************/
> +
> +#ifndef PM_DEFS_H_
> +#define PM_DEFS_H_
> +
> +/** @name Capabilities for RAM
> + *
> + * @{
> + */
> +#define PM_CAP_ACCESS	0x1U
> +#define PM_CAP_CONTEXT	0x2U
> +#define PM_CAP_WAKEUP	0x4U
> +/**@}*/
> +
> +/**
> + * PM Node ID Enum
> + */
> +enum XPmNodeId {
> +	NODE_UNKNOWN,
> +	NODE_APU,
> +	NODE_APU_0,
> +	NODE_APU_1,
> +	NODE_APU_2,
> +	NODE_APU_3,
> +	NODE_RPU,
> +	NODE_RPU_0,
> +	NODE_RPU_1,
> +	NODE_PLD,
> +	NODE_FPD,
> +	NODE_OCM_BANK_0,
> +	NODE_OCM_BANK_1,
> +	NODE_OCM_BANK_2,
> +	NODE_OCM_BANK_3,
> +	NODE_TCM_0_A,
> +	NODE_TCM_0_B,
> +	NODE_TCM_1_A,
> +	NODE_TCM_1_B,
> +	NODE_L2,
> +	NODE_GPU_PP_0,
> +	NODE_GPU_PP_1,
> +	NODE_USB_0,
> +	NODE_USB_1,
> +	NODE_TTC_0,
> +	NODE_TTC_1,
> +	NODE_TTC_2,
> +	NODE_TTC_3,
> +	NODE_SATA,
> +	NODE_ETH_0,
> +	NODE_ETH_1,
> +	NODE_ETH_2,
> +	NODE_ETH_3,
> +	NODE_UART_0,
> +	NODE_UART_1,
> +	NODE_SPI_0,
> +	NODE_SPI_1,
> +	NODE_I2C_0,
> +	NODE_I2C_1,
> +	NODE_SD_0,
> +	NODE_SD_1,
> +	NODE_DP,
> +	NODE_GDMA,
> +	NODE_ADMA,
> +	NODE_NAND,
> +	NODE_QSPI,
> +	NODE_GPIO,
> +	NODE_CAN_0,
> +	NODE_CAN_1,
> +	NODE_EXTERN,
> +	NODE_APLL,
> +	NODE_VPLL,
> +	NODE_DPLL,
> +	NODE_RPLL,
> +	NODE_IOPLL,
> +	NODE_DDR,
> +	NODE_IPI_APU,
> +	NODE_IPI_RPU_0,
> +	NODE_GPU,
> +	NODE_PCIE,
> +	NODE_PCAP,
> +	NODE_RTC,
> +	NODE_LPD,
> +	NODE_VCU,
> +	NODE_IPI_RPU_1,
> +	NODE_IPI_PL_0,
> +	NODE_IPI_PL_1,
> +	NODE_IPI_PL_2,
> +	NODE_IPI_PL_3,
> +	NODE_PL,
> +	NODE_ID_MAX
> +};
> +
> +/**
> + *  PM Reset Line IDs
> + */
> +enum XPmReset {
> +	XILPM_RESET_PCIE_CFG = 1000,
> +	XILPM_RESET_PCIE_BRIDGE,
> +	XILPM_RESET_PCIE_CTRL,
> +	XILPM_RESET_DP,
> +	XILPM_RESET_SWDT_CRF,
> +	XILPM_RESET_AFI_FM5,
> +	XILPM_RESET_AFI_FM4,
> +	XILPM_RESET_AFI_FM3,
> +	XILPM_RESET_AFI_FM2,
> +	XILPM_RESET_AFI_FM1,
> +	XILPM_RESET_AFI_FM0,
> +	XILPM_RESET_GDMA,
> +	XILPM_RESET_GPU_PP1,
> +	XILPM_RESET_GPU_PP0,
> +	XILPM_RESET_GPU,
> +	XILPM_RESET_GT,
> +	XILPM_RESET_SATA,
> +	XILPM_RESET_ACPU3_PWRON,
> +	XILPM_RESET_ACPU2_PWRON,
> +	XILPM_RESET_ACPU1_PWRON,
> +	XILPM_RESET_ACPU0_PWRON,
> +	XILPM_RESET_APU_L2,
> +	XILPM_RESET_ACPU3,
> +	XILPM_RESET_ACPU2,
> +	XILPM_RESET_ACPU1,
> +	XILPM_RESET_ACPU0,
> +	XILPM_RESET_DDR,
> +	XILPM_RESET_APM_FPD,
> +	XILPM_RESET_SOFT,
> +	XILPM_RESET_GEM0,
> +	XILPM_RESET_GEM1,
> +	XILPM_RESET_GEM2,
> +	XILPM_RESET_GEM3,
> +	XILPM_RESET_QSPI,
> +	XILPM_RESET_UART0,
> +	XILPM_RESET_UART1,
> +	XILPM_RESET_SPI0,
> +	XILPM_RESET_SPI1,
> +	XILPM_RESET_SDIO0,
> +	XILPM_RESET_SDIO1,
> +	XILPM_RESET_CAN0,
> +	XILPM_RESET_CAN1,
> +	XILPM_RESET_I2C0,
> +	XILPM_RESET_I2C1,
> +	XILPM_RESET_TTC0,
> +	XILPM_RESET_TTC1,
> +	XILPM_RESET_TTC2,
> +	XILPM_RESET_TTC3,
> +	XILPM_RESET_SWDT_CRL,
> +	XILPM_RESET_NAND,
> +	XILPM_RESET_ADMA,
> +	XILPM_RESET_GPIO,
> +	XILPM_RESET_IOU_CC,
> +	XILPM_RESET_TIMESTAMP,
> +	XILPM_RESET_RPU_R50,
> +	XILPM_RESET_RPU_R51,
> +	XILPM_RESET_RPU_AMBA,
> +	XILPM_RESET_OCM,
> +	XILPM_RESET_RPU_PGE,
> +	XILPM_RESET_USB0_CORERESET,
> +	XILPM_RESET_USB1_CORERESET,
> +	XILPM_RESET_USB0_HIBERRESET,
> +	XILPM_RESET_USB1_HIBERRESET,
> +	XILPM_RESET_USB0_APB,
> +	XILPM_RESET_USB1_APB,
> +	XILPM_RESET_IPI,
> +	XILPM_RESET_APM_LPD,
> +	XILPM_RESET_RTC,
> +	XILPM_RESET_SYSMON,
> +	XILPM_RESET_AFI_FM6,
> +	XILPM_RESET_LPD_SWDT,
> +	XILPM_RESET_FPD,
> +	XILPM_RESET_RPU_DBG1,
> +	XILPM_RESET_RPU_DBG0,
> +	XILPM_RESET_DBG_LPD,
> +	XILPM_RESET_DBG_FPD,
> +	XILPM_RESET_APLL,
> +	XILPM_RESET_DPLL,
> +	XILPM_RESET_VPLL,
> +	XILPM_RESET_IOPLL,
> +	XILPM_RESET_RPLL,
> +	XILPM_RESET_GPO3_PL_0,
> +	XILPM_RESET_GPO3_PL_1,
> +	XILPM_RESET_GPO3_PL_2,
> +	XILPM_RESET_GPO3_PL_3,
> +	XILPM_RESET_GPO3_PL_4,
> +	XILPM_RESET_GPO3_PL_5,
> +	XILPM_RESET_GPO3_PL_6,
> +	XILPM_RESET_GPO3_PL_7,
> +	XILPM_RESET_GPO3_PL_8,
> +	XILPM_RESET_GPO3_PL_9,
> +	XILPM_RESET_GPO3_PL_10,
> +	XILPM_RESET_GPO3_PL_11,
> +	XILPM_RESET_GPO3_PL_12,
> +	XILPM_RESET_GPO3_PL_13,
> +	XILPM_RESET_GPO3_PL_14,
> +	XILPM_RESET_GPO3_PL_15,
> +	XILPM_RESET_GPO3_PL_16,
> +	XILPM_RESET_GPO3_PL_17,
> +	XILPM_RESET_GPO3_PL_18,
> +	XILPM_RESET_GPO3_PL_19,
> +	XILPM_RESET_GPO3_PL_20,
> +	XILPM_RESET_GPO3_PL_21,
> +	XILPM_RESET_GPO3_PL_22,
> +	XILPM_RESET_GPO3_PL_23,
> +	XILPM_RESET_GPO3_PL_24,
> +	XILPM_RESET_GPO3_PL_25,
> +	XILPM_RESET_GPO3_PL_26,
> +	XILPM_RESET_GPO3_PL_27,
> +	XILPM_RESET_GPO3_PL_28,
> +	XILPM_RESET_GPO3_PL_29,
> +	XILPM_RESET_GPO3_PL_30,
> +	XILPM_RESET_GPO3_PL_31,
> +	XILPM_RESET_RPU_LS,
> +	XILPM_RESET_PS_ONLY,
> +	XILPM_RESET_PL,
> +	XILPM_RESET_GPIO5_EMIO_92,
> +	XILPM_RESET_GPIO5_EMIO_93,
> +	XILPM_RESET_GPIO5_EMIO_94,
> +	XILPM_RESET_GPIO5_EMIO_95,
> +};
> +
> + /** @} */
> +#endif /* PM_DEFS_H_ */
> diff --git a/board/xilinx/zynqmp/pmu_global.h b/board/xilinx/zynqmp/pmu_global.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/board/xilinx/zynqmp/xil_types.h b/board/xilinx/zynqmp/xil_types.h
> new file mode 100644
> index 000000000000..65e11030d85e
> --- /dev/null
> +++ b/board/xilinx/zynqmp/xil_types.h
> @@ -0,0 +1 @@
> +#include <common.h>
> 

I found one version of this file with GPL license. But even that
I think this should be taken as blob and compiled/generated out of
u-boot source code. Still pm_cfg_obj.c has no GPL license in it.

Thanks,
Michal

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Luca Ceresoli]

Hi Michal,

thanks for the feedback.

On 27/03/19 16:03, Michal Simek wrote:
> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>> Optionally allow U-Boot to load at the PMU firmware configuration object
>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>
>> The configuration object is required by the PMU FW to enable most SoC
>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>> the configuration object in the PMU firmware. Allow a different boot
>> process, where the PMU FW is equal for any ZynqMP chip and its
>> configuration is passed at runtime by U-Boot SPL.
>>
>> All the code for Inter-processor communication with the PMU is isolated in
>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>> Firmware:
>>
>>  * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>  * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>
>> The load is logged on the console during boot:
>>
>>   U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>   Loading PMUFW cfg obj (2008 bytes)
>>   EL Level:	EL3
>>   ...
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
[...]
>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>> new file mode 100644
>> index 000000000000..6306d33d6f17
>> --- /dev/null
>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>> + * firmware.
[...]
>> +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;
>> +}
> 
> All above should be mailbox driver. It means this should go to
> drivers/mailbox and be split to mbox send/recv functions.

Oh, wow, there's a mailbox uclass! I'll have a look into it.

> But I have no problem to use this configuration in the first patch and
> move to mbox driver in separate patch.

Good to know, I'll use this option in case it takes too long to make it
a proper mailbox driver.

>> +/**
>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>> +	const u32 request[] = {
>> +		PMUFW_CMD_SET_CONFIGURATION,
>> +		CONFIG_SPL_TEXT_BASE
>> +	};
>> +	u32 response;
>> +	int err;
>> +
>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>> +
>> +	memcpy(ocm, cfg_obj, 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);
>> +}
> 
> And this can stay here or go to arch/arm/mach-zynq/

Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.

I assume "zynq" here means the whole zynq family, including zynqmp.

>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>> new file mode 100644
>> index 000000000000..37bb72c1b20a
>> --- /dev/null
>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * (C) Copyright 2019 Luca Ceresoli
>> + * Luca Ceresoli <luca@lucaceresoli.net>
>> + */
>> +
>> +#ifndef __ZYNQMP_PMU_IPC_H__
>> +#define __ZYNQMP_PMU_IPC_H__
>> +
>> +#include <linux/types.h>
>> +
>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>> +
> 
> 
> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.

Ok, but I guess you mean s/zynqmp/zynq/, as above.

>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>> index 5e1d2116bc32..1d5e25961863 100644
>> --- a/board/xilinx/zynqmp/zynqmp.c
>> +++ b/board/xilinx/zynqmp/zynqmp.c
>> @@ -4,6 +4,8 @@
>>   * Michal Simek <michal.simek@xilinx.com>
>>   */
>>  
>> +#include "pmu_ipc.h"
>> +
>>  #include <common.h>
>>  #include <sata.h>
>>  #include <ahci.h>
>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>  }
>>  #endif
>>  
>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>> +#endif
>> +
>>  int board_early_init_f(void)
>>  {
>>  	int ret = 0;
>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>  
>>  int board_init(void)
>>  {
>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>> +					sizeof(XPm_ConfigObject));
>> +#endif
> 
> As we discussed over IRC. I think that this should be simply bin
> firmware file compare to C built by u-boot.

Sure. I have a working prototype that uses a binary blob. It still needs
a decent way to produce a blob and to be updated according to your review.

-- 
Luca

[U-Boot] [RFC v2 1/2] zynqmp: add minimal include files to build a pm_cfg_obj.c

[Luca Ceresoli]

Hi Michal,

On 27/03/19 16:10, Michal Simek wrote:
> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>> A following commit will allow U-Boot to pass a configuration object to
>> the ZynqMP PMU firmware. This configuration object is generated by
>> Xilinx tools in the form of a C file (pm_cfg_obj.c), which #includes a
>> few headers with constants definitions.
>>
>> In order to allow pm_cfg_obj.c to build, include in U-Boot a minimal
>> version of those headers files:
>>
>>  - pm_defs: a copy of [0], reduced to remove unneeded values
>>  - pmu_global.h: empty file, it is included but not really needed
>>  - xil_types.h: just includes common.h which has all the needed types
>>
>> [0] https://github.com/Xilinx/embeddedsw/blob/xilinx-v2018.3/lib/sw_services/xilpm/src/common/pm_defs.h
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>>  board/xilinx/zynqmp/pm_defs.h    | 254 +++++++++++++++++++++++++++++++
>>  board/xilinx/zynqmp/pmu_global.h |   0
>>  board/xilinx/zynqmp/xil_types.h  |   1 +
>>  3 files changed, 255 insertions(+)
>>  create mode 100644 board/xilinx/zynqmp/pm_defs.h
>>  create mode 100644 board/xilinx/zynqmp/pmu_global.h
>>  create mode 100644 board/xilinx/zynqmp/xil_types.h
[...]
> 
> I found one version of this file with GPL license.

Of pm_defs.h? Interesting, where can I find it?

> But even that
> I think this should be taken as blob and compiled/generated out of
> u-boot source code. Still pm_cfg_obj.c has no GPL license in it.

Yes, sure. But still we need a pm_defs.h if we want to obtain a blob out
of a Xilinx-generated pm_cfg_obj.c, so the GPL'ed file you mentioned
would be useful.

-- 
Luca

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Michal Simek]

On 29. 03. 19 13:22, Luca Ceresoli wrote:
> Hi Michal,
> 
> thanks for the feedback.
> 
> On 27/03/19 16:03, Michal Simek wrote:
>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>
>>> The configuration object is required by the PMU FW to enable most SoC
>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>> the configuration object in the PMU firmware. Allow a different boot
>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>> configuration is passed at runtime by U-Boot SPL.
>>>
>>> All the code for Inter-processor communication with the PMU is isolated in
>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>> Firmware:
>>>
>>>  * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>  * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>
>>> The load is logged on the console during boot:
>>>
>>>   U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>   Loading PMUFW cfg obj (2008 bytes)
>>>   EL Level:	EL3
>>>   ...
>>>
>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> [...]
>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>> new file mode 100644
>>> index 000000000000..6306d33d6f17
>>> --- /dev/null
>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>> @@ -0,0 +1,116 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>> + * firmware.
> [...]
>>> +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;
>>> +}
>>
>> All above should be mailbox driver. It means this should go to
>> drivers/mailbox and be split to mbox send/recv functions.
> 
> Oh, wow, there's a mailbox uclass! I'll have a look into it.
> 
>> But I have no problem to use this configuration in the first patch and
>> move to mbox driver in separate patch.
> 
> Good to know, I'll use this option in case it takes too long to make it
> a proper mailbox driver.


Even if you convert this to mailbox uclass I see a value to do it in 2
steps. The first this implementation and then moving to mailbox driver
to educate and show how cleaner code can be.

> 
>>> +/**
>>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>> +	const u32 request[] = {
>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>> +		CONFIG_SPL_TEXT_BASE
>>> +	};
>>> +	u32 response;
>>> +	int err;
>>> +
>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>> +
>>> +	memcpy(ocm, cfg_obj, 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);
>>> +}
>>
>> And this can stay here or go to arch/arm/mach-zynq/
> 
> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
> 
> I assume "zynq" here means the whole zynq family, including zynqmp.

No sorry - that was my typo - it should be all zynqmp.

> 
>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>> new file mode 100644
>>> index 000000000000..37bb72c1b20a
>>> --- /dev/null
>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +/*
>>> + * (C) Copyright 2019 Luca Ceresoli
>>> + * Luca Ceresoli <luca@lucaceresoli.net>
>>> + */
>>> +
>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>> +#define __ZYNQMP_PMU_IPC_H__
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>> +
>>
>>
>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
> 
> Ok, but I guess you mean s/zynqmp/zynq/, as above.

As above zynqmp. Sorry my typo.

> 
>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index 5e1d2116bc32..1d5e25961863 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -4,6 +4,8 @@
>>>   * Michal Simek <michal.simek@xilinx.com>
>>>   */
>>>  
>>> +#include "pmu_ipc.h"
>>> +
>>>  #include <common.h>
>>>  #include <sata.h>
>>>  #include <ahci.h>
>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>  }
>>>  #endif
>>>  
>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>> +#endif
>>> +
>>>  int board_early_init_f(void)
>>>  {
>>>  	int ret = 0;
>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>  
>>>  int board_init(void)
>>>  {
>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>> +					sizeof(XPm_ConfigObject));
>>> +#endif
>>
>> As we discussed over IRC. I think that this should be simply bin
>> firmware file compare to C built by u-boot.
> 
> Sure. I have a working prototype that uses a binary blob. It still needs
> a decent way to produce a blob and to be updated according to your review.

Great.

Thanks,
Michal

[U-Boot] [RFC v2 1/2] zynqmp: add minimal include files to build a pm_cfg_obj.c

[Michal Simek]

On 29. 03. 19 13:23, Luca Ceresoli wrote:
> Hi Michal,
> 
> On 27/03/19 16:10, Michal Simek wrote:
>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>> A following commit will allow U-Boot to pass a configuration object to
>>> the ZynqMP PMU firmware. This configuration object is generated by
>>> Xilinx tools in the form of a C file (pm_cfg_obj.c), which #includes a
>>> few headers with constants definitions.
>>>
>>> In order to allow pm_cfg_obj.c to build, include in U-Boot a minimal
>>> version of those headers files:
>>>
>>>  - pm_defs: a copy of [0], reduced to remove unneeded values
>>>  - pmu_global.h: empty file, it is included but not really needed
>>>  - xil_types.h: just includes common.h which has all the needed types
>>>
>>> [0] https://github.com/Xilinx/embeddedsw/blob/xilinx-v2018.3/lib/sw_services/xilpm/src/common/pm_defs.h
>>>
>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>>> ---
>>>  board/xilinx/zynqmp/pm_defs.h    | 254 +++++++++++++++++++++++++++++++
>>>  board/xilinx/zynqmp/pmu_global.h |   0
>>>  board/xilinx/zynqmp/xil_types.h  |   1 +
>>>  3 files changed, 255 insertions(+)
>>>  create mode 100644 board/xilinx/zynqmp/pm_defs.h
>>>  create mode 100644 board/xilinx/zynqmp/pmu_global.h
>>>  create mode 100644 board/xilinx/zynqmp/xil_types.h
> [...]
>>
>> I found one version of this file with GPL license.
> 
> Of pm_defs.h? Interesting, where can I find it?

it is not publicly available but I have seen that.

> 
>> But even that
>> I think this should be taken as blob and compiled/generated out of
>> u-boot source code. Still pm_cfg_obj.c has no GPL license in it.
> 
> Yes, sure. But still we need a pm_defs.h if we want to obtain a blob out
> of a Xilinx-generated pm_cfg_obj.c, so the GPL'ed file you mentioned
> would be useful.

But pm_cfg_obj.c has xilinx license that's why pm_defs.h shouldn't be
gpl if you want to compile it together.
Anyway firmware blob will get rid of this and code which does this can
use Xilinx license as other standalone code which customers use.

Thanks,
Michal

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Mike Looijmans]

On 29-03-19 13:22, Luca Ceresoli wrote:
> Hi Michal,
> 
> thanks for the feedback.
> 
> On 27/03/19 16:03, Michal Simek wrote:
>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>
>>> The configuration object is required by the PMU FW to enable most SoC
>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>> the configuration object in the PMU firmware. Allow a different boot
>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>> configuration is passed at runtime by U-Boot SPL.
>>>
>>> All the code for Inter-processor communication with the PMU is isolated in
>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>> Firmware:
>>>
>>>   * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>   * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>
>>> The load is logged on the console during boot:
>>>
>>>    U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>    Loading PMUFW cfg obj (2008 bytes)
>>>    EL Level:	EL3
>>>    ...
>>>
>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> [...]
>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>> new file mode 100644
>>> index 000000000000..6306d33d6f17
>>> --- /dev/null
>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>> @@ -0,0 +1,116 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>> + * firmware.
> [...]
>>> +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;
>>> +}
>>
>> All above should be mailbox driver. It means this should go to
>> drivers/mailbox and be split to mbox send/recv functions.
> 
> Oh, wow, there's a mailbox uclass! I'll have a look into it.
> 
>> But I have no problem to use this configuration in the first patch and
>> move to mbox driver in separate patch.
> 
> Good to know, I'll use this option in case it takes too long to make it
> a proper mailbox driver.
> 
>>> +/**
>>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>> +	const u32 request[] = {
>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>> +		CONFIG_SPL_TEXT_BASE
>>> +	};
>>> +	u32 response;
>>> +	int err;
>>> +
>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>> +
>>> +	memcpy(ocm, cfg_obj, 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);
>>> +}
>>
>> And this can stay here or go to arch/arm/mach-zynq/
> 
> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
> 
> I assume "zynq" here means the whole zynq family, including zynqmp.
> 
>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>> new file mode 100644
>>> index 000000000000..37bb72c1b20a
>>> --- /dev/null
>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +/*
>>> + * (C) Copyright 2019 Luca Ceresoli
>>> + * Luca Ceresoli <luca@lucaceresoli.net>
>>> + */
>>> +
>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>> +#define __ZYNQMP_PMU_IPC_H__
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>> +
>>
>>
>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
> 
> Ok, but I guess you mean s/zynqmp/zynq/, as above.
> 
>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index 5e1d2116bc32..1d5e25961863 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -4,6 +4,8 @@
>>>    * Michal Simek <michal.simek@xilinx.com>
>>>    */
>>>   
>>> +#include "pmu_ipc.h"
>>> +
>>>   #include <common.h>
>>>   #include <sata.h>
>>>   #include <ahci.h>
>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>   }
>>>   #endif
>>>   
>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>> +#endif
>>> +
>>>   int board_early_init_f(void)
>>>   {
>>>   	int ret = 0;
>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>   
>>>   int board_init(void)
>>>   {
>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>> +					sizeof(XPm_ConfigObject));
>>> +#endif
>>
>> As we discussed over IRC. I think that this should be simply bin
>> firmware file compare to C built by u-boot.
> 
> Sure. I have a working prototype that uses a binary blob. It still needs
> a decent way to produce a blob and to be updated according to your review.

It should be doable to write a Python script to parse the C file and create an 
equivalent binary (using "struct" module) which is just an array of integers 
in the end. That avoids the need for a microblaze C compiler...

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Michal Simek]

On 03. 04. 19 13:24, Mike Looijmans wrote:
> On 29-03-19 13:22, Luca Ceresoli wrote:
>> Hi Michal,
>>
>> thanks for the feedback.
>>
>> On 27/03/19 16:03, Michal Simek wrote:
>>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>>
>>>> The configuration object is required by the PMU FW to enable most SoC
>>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>>> the configuration object in the PMU firmware. Allow a different boot
>>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>>> configuration is passed at runtime by U-Boot SPL.
>>>>
>>>> All the code for Inter-processor communication with the PMU is isolated in
>>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>>> Firmware:
>>>>
>>>>   * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>>   * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>>
>>>> The load is logged on the console during boot:
>>>>
>>>>    U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>>    Loading PMUFW cfg obj (2008 bytes)
>>>>    EL Level:	EL3
>>>>    ...
>>>>
>>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> [...]
>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>>> new file mode 100644
>>>> index 000000000000..6306d33d6f17
>>>> --- /dev/null
>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>>> @@ -0,0 +1,116 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>>> + * firmware.
>> [...]
>>>> +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;
>>>> +}
>>>
>>> All above should be mailbox driver. It means this should go to
>>> drivers/mailbox and be split to mbox send/recv functions.
>>
>> Oh, wow, there's a mailbox uclass! I'll have a look into it.
>>
>>> But I have no problem to use this configuration in the first patch and
>>> move to mbox driver in separate patch.
>>
>> Good to know, I'll use this option in case it takes too long to make it
>> a proper mailbox driver.
>>
>>>> +/**
>>>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>>> +	const u32 request[] = {
>>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>>> +		CONFIG_SPL_TEXT_BASE
>>>> +	};
>>>> +	u32 response;
>>>> +	int err;
>>>> +
>>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>>> +
>>>> +	memcpy(ocm, cfg_obj, 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);
>>>> +}
>>>
>>> And this can stay here or go to arch/arm/mach-zynq/
>>
>> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
>>
>> I assume "zynq" here means the whole zynq family, including zynqmp.
>>
>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>>> new file mode 100644
>>>> index 000000000000..37bb72c1b20a
>>>> --- /dev/null
>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>>> @@ -0,0 +1,14 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>> +/*
>>>> + * (C) Copyright 2019 Luca Ceresoli
>>>> + * Luca Ceresoli <luca@lucaceresoli.net>
>>>> + */
>>>> +
>>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>>> +#define __ZYNQMP_PMU_IPC_H__
>>>> +
>>>> +#include <linux/types.h>
>>>> +
>>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>>> +
>>>
>>>
>>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
>>
>> Ok, but I guess you mean s/zynqmp/zynq/, as above.
>>
>>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>>> index 5e1d2116bc32..1d5e25961863 100644
>>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>>> @@ -4,6 +4,8 @@
>>>>    * Michal Simek <michal.simek@xilinx.com>
>>>>    */
>>>>   
>>>> +#include "pmu_ipc.h"
>>>> +
>>>>   #include <common.h>
>>>>   #include <sata.h>
>>>>   #include <ahci.h>
>>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>>   }
>>>>   #endif
>>>>   
>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>> +#endif
>>>> +
>>>>   int board_early_init_f(void)
>>>>   {
>>>>   	int ret = 0;
>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>   
>>>>   int board_init(void)
>>>>   {
>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>> +					sizeof(XPm_ConfigObject));
>>>> +#endif
>>>
>>> As we discussed over IRC. I think that this should be simply bin
>>> firmware file compare to C built by u-boot.
>>
>> Sure. I have a working prototype that uses a binary blob. It still needs
>> a decent way to produce a blob and to be updated according to your review.
> 
> It should be doable to write a Python script to parse the C file and create an 
> equivalent binary (using "struct" module) which is just an array of integers 
> in the end. That avoids the need for a microblaze C compiler...

It can be done in many different ways. Even having "tool" which exactly
describes all that values there would be useful.

Thanks,
Michal

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Luca Ceresoli]

Hi Mike,

On 03/04/19 13:24, Mike Looijmans wrote:
> On 29-03-19 13:22, Luca Ceresoli wrote:
>> Hi Michal,
>>
>> thanks for the feedback.
>>
>> On 27/03/19 16:03, Michal Simek wrote:
>>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>>
>>>> The configuration object is required by the PMU FW to enable most SoC
>>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>>> the configuration object in the PMU firmware. Allow a different boot
>>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>>> configuration is passed at runtime by U-Boot SPL.
>>>>
>>>> All the code for Inter-processor communication with the PMU is isolated in
>>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>>> Firmware:
>>>>
>>>>   * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>>   * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>>
>>>> The load is logged on the console during boot:
>>>>
>>>>    U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>>    Loading PMUFW cfg obj (2008 bytes)
>>>>    EL Level:	EL3
>>>>    ...
>>>>
>>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> [...]
>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>>> new file mode 100644
>>>> index 000000000000..6306d33d6f17
>>>> --- /dev/null
>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>>> @@ -0,0 +1,116 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>>> + * firmware.
>> [...]
>>>> +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;
>>>> +}
>>>
>>> All above should be mailbox driver. It means this should go to
>>> drivers/mailbox and be split to mbox send/recv functions.
>>
>> Oh, wow, there's a mailbox uclass! I'll have a look into it.
>>
>>> But I have no problem to use this configuration in the first patch and
>>> move to mbox driver in separate patch.
>>
>> Good to know, I'll use this option in case it takes too long to make it
>> a proper mailbox driver.
>>
>>>> +/**
>>>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>>> +	const u32 request[] = {
>>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>>> +		CONFIG_SPL_TEXT_BASE
>>>> +	};
>>>> +	u32 response;
>>>> +	int err;
>>>> +
>>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>>> +
>>>> +	memcpy(ocm, cfg_obj, 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);
>>>> +}
>>>
>>> And this can stay here or go to arch/arm/mach-zynq/
>>
>> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
>>
>> I assume "zynq" here means the whole zynq family, including zynqmp.
>>
>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>>> new file mode 100644
>>>> index 000000000000..37bb72c1b20a
>>>> --- /dev/null
>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>>> @@ -0,0 +1,14 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>> +/*
>>>> + * (C) Copyright 2019 Luca Ceresoli
>>>> + * Luca Ceresoli <luca@lucaceresoli.net>
>>>> + */
>>>> +
>>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>>> +#define __ZYNQMP_PMU_IPC_H__
>>>> +
>>>> +#include <linux/types.h>
>>>> +
>>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>>> +
>>>
>>>
>>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
>>
>> Ok, but I guess you mean s/zynqmp/zynq/, as above.
>>
>>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>>> index 5e1d2116bc32..1d5e25961863 100644
>>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>>> @@ -4,6 +4,8 @@
>>>>    * Michal Simek <michal.simek@xilinx.com>
>>>>    */
>>>>   
>>>> +#include "pmu_ipc.h"
>>>> +
>>>>   #include <common.h>
>>>>   #include <sata.h>
>>>>   #include <ahci.h>
>>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>>   }
>>>>   #endif
>>>>   
>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>> +#endif
>>>> +
>>>>   int board_early_init_f(void)
>>>>   {
>>>>   	int ret = 0;
>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>   
>>>>   int board_init(void)
>>>>   {
>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>> +					sizeof(XPm_ConfigObject));
>>>> +#endif
>>>
>>> As we discussed over IRC. I think that this should be simply bin
>>> firmware file compare to C built by u-boot.
>>
>> Sure. I have a working prototype that uses a binary blob. It still needs
>> a decent way to produce a blob and to be updated according to your review.
> 
> It should be doable to write a Python script to parse the C file and create an 
> equivalent binary (using "struct" module) which is just an array of integers 
> in the end. That avoids the need for a microblaze C compiler...

There's no need for a microblaze compiler. pm_cfg_obj.c is simply
declaring a u32 array and some #defines, any C compiler is enough.

That said, my current solution (a trivial main.c that compiles the u32
array and outputs it to a binary file) is not nice at all, and it
requires a pm_defs.h file.

The python script you mention looks way better from a user perspective,
although the parsing might be a bit fragile. I'll consider it, thanks
for the suggestion.

In the past I even prototyped a python script that parses the Vivado
.xpr project file and produces a pm_cfg_obj.c. It avoided the need to
run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
extended to produce a .bin directly, or a self-standing .c file that
doesn't need pm_defs.h, thus removing any licensing issue. But it never
grew complete to handle all cases. Obvious, since *I* don't even know
all of the cases. :)

-- 
Luca

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Mike Looijmans]

On 03-04-19 23:18, Luca Ceresoli wrote:
> Hi Mike,
> 
> On 03/04/19 13:24, Mike Looijmans wrote:
>> On 29-03-19 13:22, Luca Ceresoli wrote:
>>> Hi Michal,
>>>
>>> thanks for the feedback.
>>>
>>> On 27/03/19 16:03, Michal Simek wrote:
>>>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>>>
>>>>> The configuration object is required by the PMU FW to enable most SoC
>>>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>>>> the configuration object in the PMU firmware. Allow a different boot
>>>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>>>> configuration is passed at runtime by U-Boot SPL.
>>>>>
>>>>> All the code for Inter-processor communication with the PMU is isolated in
>>>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>>>> Firmware:
>>>>>
>>>>>    * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>>>    * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>>>
>>>>> The load is logged on the console during boot:
>>>>>
>>>>>     U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>>>     Loading PMUFW cfg obj (2008 bytes)
>>>>>     EL Level:	EL3
>>>>>     ...
>>>>>
>>>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>>> [...]
>>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>>>> new file mode 100644
>>>>> index 000000000000..6306d33d6f17
>>>>> --- /dev/null
>>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>>>> @@ -0,0 +1,116 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>> +/*
>>>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>>>> + * firmware.
>>> [...]
>>>>> +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;
>>>>> +}
>>>>
>>>> All above should be mailbox driver. It means this should go to
>>>> drivers/mailbox and be split to mbox send/recv functions.
>>>
>>> Oh, wow, there's a mailbox uclass! I'll have a look into it.
>>>
>>>> But I have no problem to use this configuration in the first patch and
>>>> move to mbox driver in separate patch.
>>>
>>> Good to know, I'll use this option in case it takes too long to make it
>>> a proper mailbox driver.
>>>
>>>>> +/**
>>>>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>>>> +	const u32 request[] = {
>>>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>>>> +		CONFIG_SPL_TEXT_BASE
>>>>> +	};
>>>>> +	u32 response;
>>>>> +	int err;
>>>>> +
>>>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>>>> +
>>>>> +	memcpy(ocm, cfg_obj, 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);
>>>>> +}
>>>>
>>>> And this can stay here or go to arch/arm/mach-zynq/
>>>
>>> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
>>>
>>> I assume "zynq" here means the whole zynq family, including zynqmp.
>>>
>>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>>>> new file mode 100644
>>>>> index 000000000000..37bb72c1b20a
>>>>> --- /dev/null
>>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>>>> @@ -0,0 +1,14 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>>> +/*
>>>>> + * (C) Copyright 2019 Luca Ceresoli
>>>>> + * Luca Ceresoli <luca@lucaceresoli.net>
>>>>> + */
>>>>> +
>>>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>>>> +#define __ZYNQMP_PMU_IPC_H__
>>>>> +
>>>>> +#include <linux/types.h>
>>>>> +
>>>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>>>> +
>>>>
>>>>
>>>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
>>>
>>> Ok, but I guess you mean s/zynqmp/zynq/, as above.
>>>
>>>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>>>> index 5e1d2116bc32..1d5e25961863 100644
>>>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>>>> @@ -4,6 +4,8 @@
>>>>>     * Michal Simek <michal.simek@xilinx.com>
>>>>>     */
>>>>>    
>>>>> +#include "pmu_ipc.h"
>>>>> +
>>>>>    #include <common.h>
>>>>>    #include <sata.h>
>>>>>    #include <ahci.h>
>>>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>>>    }
>>>>>    #endif
>>>>>    
>>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>>> +#endif
>>>>> +
>>>>>    int board_early_init_f(void)
>>>>>    {
>>>>>    	int ret = 0;
>>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>>    
>>>>>    int board_init(void)
>>>>>    {
>>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>>> +					sizeof(XPm_ConfigObject));
>>>>> +#endif
>>>>
>>>> As we discussed over IRC. I think that this should be simply bin
>>>> firmware file compare to C built by u-boot.
>>>
>>> Sure. I have a working prototype that uses a binary blob. It still needs
>>> a decent way to produce a blob and to be updated according to your review.
>>
>> It should be doable to write a Python script to parse the C file and create an
>> equivalent binary (using "struct" module) which is just an array of integers
>> in the end. That avoids the need for a microblaze C compiler...
> 
> There's no need for a microblaze compiler. pm_cfg_obj.c is simply
> declaring a u32 array and some #defines, any C compiler is enough.
> 
> That said, my current solution (a trivial main.c that compiles the u32
> array and outputs it to a binary file) is not nice at all, and it
> requires a pm_defs.h file.
> 
> The python script you mention looks way better from a user perspective,
> although the parsing might be a bit fragile. I'll consider it, thanks
> for the suggestion.
> 
> In the past I even prototyped a python script that parses the Vivado
> .xpr project file and produces a pm_cfg_obj.c. It avoided the need to
> run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
> extended to produce a .bin directly, or a self-standing .c file that
> doesn't need pm_defs.h, thus removing any licensing issue. But it never
> grew complete to handle all cases. Obvious, since *I* don't even know
> all of the cases. :)


Another approach would be to simply create and include a "god mode" config 
object that just allows access to all periferals. As far as I can see, such a 
config object would just work on all boards. There's nothing really board 
specific in the config object, and it's rather lame anyway to have to go and 
compile a new bootloader just because you want to use a SPI controller...

The config object I compile into the pmu firmware is of that type anyway.

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Michal Simek]

On 04. 04. 19 7:38, Mike Looijmans wrote:
> On 03-04-19 23:18, Luca Ceresoli wrote:
>> Hi Mike,
>>
>> On 03/04/19 13:24, Mike Looijmans wrote:
>>> On 29-03-19 13:22, Luca Ceresoli wrote:
>>>> Hi Michal,
>>>>
>>>> thanks for the feedback.
>>>>
>>>> On 27/03/19 16:03, Michal Simek wrote:
>>>>> On 21. 03. 19 16:48, Luca Ceresoli wrote:
>>>>>> Optionally allow U-Boot to load at the PMU firmware configuration object
>>>>>> into the Power Management Unit (PMU) on Xilinx ZynqMP.
>>>>>>
>>>>>> The configuration object is required by the PMU FW to enable most SoC
>>>>>> peripherals. So far the only way to boot using U-Boot SPL was to hard-code
>>>>>> the configuration object in the PMU firmware. Allow a different boot
>>>>>> process, where the PMU FW is equal for any ZynqMP chip and its
>>>>>> configuration is passed at runtime by U-Boot SPL.
>>>>>>
>>>>>> All the code for Inter-processor communication with the PMU is isolated in
>>>>>> a new file (pmu_ipc.c). The code is inspired by the same feature as
>>>>>> implemented in the Xilinx First Stage Bootloader (FSBL) and Arm Trusted
>>>>>> Firmware:
>>>>>>
>>>>>>    * https://github.com/Xilinx/embeddedsw/blob/fb647e6b4c00f5154eba52a88b948195b6f1dc2b/lib/sw_apps/zynqmp_fsbl/src/xfsbl_misc_drivers.c#L295
>>>>>>    * https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/plat/xilinx/zynqmp/pm_service/pm_api_sys.c#L357
>>>>>>
>>>>>> The load is logged on the console during boot:
>>>>>>
>>>>>>     U-Boot SPL 2018.01 (Mar 20 2019 - 08:12:21)
>>>>>>     Loading PMUFW cfg obj (2008 bytes)
>>>>>>     EL Level:	EL3
>>>>>>     ...
>>>>>>
>>>>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>>>> [...]
>>>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.c b/board/xilinx/zynqmp/pmu_ipc.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..6306d33d6f17
>>>>>> --- /dev/null
>>>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.c
>>>>>> @@ -0,0 +1,116 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>>> +/*
>>>>>> + * Inter-Processor Communication with the Platform Management Unit (PMU)
>>>>>> + * firmware.
>>>> [...]
>>>>>> +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;
>>>>>> +}
>>>>>
>>>>> All above should be mailbox driver. It means this should go to
>>>>> drivers/mailbox and be split to mbox send/recv functions.
>>>>
>>>> Oh, wow, there's a mailbox uclass! I'll have a look into it.
>>>>
>>>>> But I have no problem to use this configuration in the first patch and
>>>>> move to mbox driver in separate patch.
>>>>
>>>> Good to know, I'll use this option in case it takes too long to make it
>>>> a proper mailbox driver.
>>>>
>>>>>> +/**
>>>>>> + * 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 *ocm = (u32 *)CONFIG_SPL_TEXT_BASE;
>>>>>> +	const u32 request[] = {
>>>>>> +		PMUFW_CMD_SET_CONFIGURATION,
>>>>>> +		CONFIG_SPL_TEXT_BASE
>>>>>> +	};
>>>>>> +	u32 response;
>>>>>> +	int err;
>>>>>> +
>>>>>> +	printf("Loading PMUFW cfg obj (%ld bytes)\n", size);
>>>>>> +
>>>>>> +	memcpy(ocm, cfg_obj, 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);
>>>>>> +}
>>>>>
>>>>> And this can stay here or go to arch/arm/mach-zynq/
>>>>
>>>> Ok, I'll move it to arch/arm/mach-zynq/pmu.c or so.
>>>>
>>>> I assume "zynq" here means the whole zynq family, including zynqmp.
>>>>
>>>>>> diff --git a/board/xilinx/zynqmp/pmu_ipc.h b/board/xilinx/zynqmp/pmu_ipc.h
>>>>>> new file mode 100644
>>>>>> index 000000000000..37bb72c1b20a
>>>>>> --- /dev/null
>>>>>> +++ b/board/xilinx/zynqmp/pmu_ipc.h
>>>>>> @@ -0,0 +1,14 @@
>>>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>>>> +/*
>>>>>> + * (C) Copyright 2019 Luca Ceresoli
>>>>>> + * Luca Ceresoli <luca@lucaceresoli.net>
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef __ZYNQMP_PMU_IPC_H__
>>>>>> +#define __ZYNQMP_PMU_IPC_H__
>>>>>> +
>>>>>> +#include <linux/types.h>
>>>>>> +
>>>>>> +void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
>>>>>> +
>>>>>
>>>>>
>>>>> arch/arm/mach-zynqmp/include/mach/sys_proto.h should be fine.
>>>>
>>>> Ok, but I guess you mean s/zynqmp/zynq/, as above.
>>>>
>>>>>> +#endif /* __ZYNQMP_PMU_IPC_H__ */
>>>>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>>>>> index 5e1d2116bc32..1d5e25961863 100644
>>>>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>>>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>>>>> @@ -4,6 +4,8 @@
>>>>>>     * Michal Simek <michal.simek@xilinx.com>
>>>>>>     */
>>>>>>    
>>>>>> +#include "pmu_ipc.h"
>>>>>> +
>>>>>>    #include <common.h>
>>>>>>    #include <sata.h>
>>>>>>    #include <ahci.h>
>>>>>> @@ -302,6 +304,10 @@ static char *zynqmp_get_silicon_idcode_name(void)
>>>>>>    }
>>>>>>    #endif
>>>>>>    
>>>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>>>> +#endif
>>>>>> +
>>>>>>    int board_early_init_f(void)
>>>>>>    {
>>>>>>    	int ret = 0;
>>>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>>>    
>>>>>>    int board_init(void)
>>>>>>    {
>>>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>>>> +					sizeof(XPm_ConfigObject));
>>>>>> +#endif
>>>>>
>>>>> As we discussed over IRC. I think that this should be simply bin
>>>>> firmware file compare to C built by u-boot.
>>>>
>>>> Sure. I have a working prototype that uses a binary blob. It still needs
>>>> a decent way to produce a blob and to be updated according to your review.
>>>
>>> It should be doable to write a Python script to parse the C file and create an
>>> equivalent binary (using "struct" module) which is just an array of integers
>>> in the end. That avoids the need for a microblaze C compiler...
>>
>> There's no need for a microblaze compiler. pm_cfg_obj.c is simply
>> declaring a u32 array and some #defines, any C compiler is enough.
>>
>> That said, my current solution (a trivial main.c that compiles the u32
>> array and outputs it to a binary file) is not nice at all, and it
>> requires a pm_defs.h file.
>>
>> The python script you mention looks way better from a user perspective,
>> although the parsing might be a bit fragile. I'll consider it, thanks
>> for the suggestion.
>>
>> In the past I even prototyped a python script that parses the Vivado
>> .xpr project file and produces a pm_cfg_obj.c. It avoided the need to
>> run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
>> extended to produce a .bin directly, or a self-standing .c file that
>> doesn't need pm_defs.h, thus removing any licensing issue. But it never
>> grew complete to handle all cases. Obvious, since *I* don't even know
>> all of the cases. :)
> 
> 
> Another approach would be to simply create and include a "god mode" config 
> object that just allows access to all periferals. As far as I can see, such a 
> config object would just work on all boards. There's nothing really board 
> specific in the config object, and it's rather lame anyway to have to go and 
> compile a new bootloader just because you want to use a SPI controller...
> 
> The config object I compile into the pmu firmware is of that type anyway.

Most of that stuff should be the same for all boards. But there are some
stuff which can be just board specific.

Thanks,
Michal

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Luca Ceresoli]

Hi Mike, Michal,

On 04/04/19 08:49, Michal Simek wrote:
[...]
>>>>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>>>>> +#endif
>>>>>>> +
>>>>>>>    int board_early_init_f(void)
>>>>>>>    {
>>>>>>>    	int ret = 0;
>>>>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>>>>    
>>>>>>>    int board_init(void)
>>>>>>>    {
>>>>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>>>>> +					sizeof(XPm_ConfigObject));
>>>>>>> +#endif
>>>>>>
>>>>>> As we discussed over IRC. I think that this should be simply bin
>>>>>> firmware file compare to C built by u-boot.
>>>>>
>>>>> Sure. I have a working prototype that uses a binary blob. It still needs
>>>>> a decent way to produce a blob and to be updated according to your review.
>>>>
>>>> It should be doable to write a Python script to parse the C file and create an
>>>> equivalent binary (using "struct" module) which is just an array of integers
>>>> in the end. That avoids the need for a microblaze C compiler...
>>>
>>> There's no need for a microblaze compiler. pm_cfg_obj.c is simply
>>> declaring a u32 array and some #defines, any C compiler is enough.
>>>
>>> That said, my current solution (a trivial main.c that compiles the u32
>>> array and outputs it to a binary file) is not nice at all, and it
>>> requires a pm_defs.h file.
>>>
>>> The python script you mention looks way better from a user perspective,
>>> although the parsing might be a bit fragile. I'll consider it, thanks
>>> for the suggestion.
>>>
>>> In the past I even prototyped a python script that parses the Vivado
>>> .xpr project file and produces a pm_cfg_obj.c. It avoided the need to
>>> run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
>>> extended to produce a .bin directly, or a self-standing .c file that
>>> doesn't need pm_defs.h, thus removing any licensing issue. But it never
>>> grew complete to handle all cases. Obvious, since *I* don't even know
>>> all of the cases. :)
>>
>>
>> Another approach would be to simply create and include a "god mode" config 
>> object that just allows access to all periferals. As far as I can see, such a 
>> config object would just work on all boards. There's nothing really board 
>> specific in the config object, and it's rather lame anyway to have to go and 
>> compile a new bootloader just because you want to use a SPI controller...
>>
>> The config object I compile into the pmu firmware is of that type anyway.

Oh, that's interesting, but read below.

> Most of that stuff should be the same for all boards. But there are some
> stuff which can be just board specific.

Mike, I think Michal refers to boards like Ultra96 which need special
GPIO handling for their boot sequence, whose pm_cfg_obj.c have a final
section similar to this:

	/* GPO SECTION */
	PM_CONFIG_GPO_SECTION_ID,		/* GPO Section ID */
	PM_CONFIG_GPO1_BIT_2_MASK |
	PM_CONFIG_GPO1_MIO_PIN_34_MAP |
	0,					/* State of GPO pins */

I suspect a "god mode" config cannot handle such cases.

-- 
Luca

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Michal Simek]

On 04. 04. 19 9:52, Luca Ceresoli wrote:
> Hi Mike, Michal,
> 
> On 04/04/19 08:49, Michal Simek wrote:
> [...]
>>>>>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>>>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>>    int board_early_init_f(void)
>>>>>>>>    {
>>>>>>>>    	int ret = 0;
>>>>>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>>>>>    
>>>>>>>>    int board_init(void)
>>>>>>>>    {
>>>>>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>>>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>>>>>> +					sizeof(XPm_ConfigObject));
>>>>>>>> +#endif
>>>>>>>
>>>>>>> As we discussed over IRC. I think that this should be simply bin
>>>>>>> firmware file compare to C built by u-boot.
>>>>>>
>>>>>> Sure. I have a working prototype that uses a binary blob. It still needs
>>>>>> a decent way to produce a blob and to be updated according to your review.
>>>>>
>>>>> It should be doable to write a Python script to parse the C file and create an
>>>>> equivalent binary (using "struct" module) which is just an array of integers
>>>>> in the end. That avoids the need for a microblaze C compiler...
>>>>
>>>> There's no need for a microblaze compiler. pm_cfg_obj.c is simply
>>>> declaring a u32 array and some #defines, any C compiler is enough.
>>>>
>>>> That said, my current solution (a trivial main.c that compiles the u32
>>>> array and outputs it to a binary file) is not nice at all, and it
>>>> requires a pm_defs.h file.
>>>>
>>>> The python script you mention looks way better from a user perspective,
>>>> although the parsing might be a bit fragile. I'll consider it, thanks
>>>> for the suggestion.
>>>>
>>>> In the past I even prototyped a python script that parses the Vivado
>>>> .xpr project file and produces a pm_cfg_obj.c. It avoided the need to
>>>> run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
>>>> extended to produce a .bin directly, or a self-standing .c file that
>>>> doesn't need pm_defs.h, thus removing any licensing issue. But it never
>>>> grew complete to handle all cases. Obvious, since *I* don't even know
>>>> all of the cases. :)
>>>
>>>
>>> Another approach would be to simply create and include a "god mode" config 
>>> object that just allows access to all periferals. As far as I can see, such a 
>>> config object would just work on all boards. There's nothing really board 
>>> specific in the config object, and it's rather lame anyway to have to go and 
>>> compile a new bootloader just because you want to use a SPI controller...
>>>
>>> The config object I compile into the pmu firmware is of that type anyway.
> 
> Oh, that's interesting, but read below.
> 
>> Most of that stuff should be the same for all boards. But there are some
>> stuff which can be just board specific.
> 
> Mike, I think Michal refers to boards like Ultra96 which need special
> GPIO handling for their boot sequence, whose pm_cfg_obj.c have a final
> section similar to this:
> 
> 	/* GPO SECTION */
> 	PM_CONFIG_GPO_SECTION_ID,		/* GPO Section ID */
> 	PM_CONFIG_GPO1_BIT_2_MASK |
> 	PM_CONFIG_GPO1_MIO_PIN_34_MAP |
> 	0,					/* State of GPO pins */
> 
> I suspect a "god mode" config cannot handle such cases.

That's ultra96 is one of that exception but it depends on your MIO
configs too. It means you simply not assigned that gpo pins to PMU. You
loose functionality but you should be able to fix it differently later
in boot.

M

[U-Boot] [RFC v2 2/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Mike Looijmans]

On 04-04-19 09:56, Michal Simek wrote:
> On 04. 04. 19 9:52, Luca Ceresoli wrote:
>> Hi Mike, Michal,
>>
>> On 04/04/19 08:49, Michal Simek wrote:
>> [...]
>>>>>>>>> +#ifdef ZYNQMP_LOAD_PM_CFG_OBJ
>>>>>>>>> +#include CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE
>>>>>>>>> +#endif
>>>>>>>>> +
>>>>>>>>>     int board_early_init_f(void)
>>>>>>>>>     {
>>>>>>>>>     	int ret = 0;
>>>>>>>>> @@ -332,6 +338,11 @@ int board_early_init_f(void)
>>>>>>>>>     
>>>>>>>>>     int board_init(void)
>>>>>>>>>     {
>>>>>>>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>>>>>>>> +	zynqmp_pmufw_load_config_object(XPm_ConfigObject,
>>>>>>>>> +					sizeof(XPm_ConfigObject));
>>>>>>>>> +#endif
>>>>>>>>
>>>>>>>> As we discussed over IRC. I think that this should be simply bin
>>>>>>>> firmware file compare to C built by u-boot.
>>>>>>>
>>>>>>> Sure. I have a working prototype that uses a binary blob. It still needs
>>>>>>> a decent way to produce a blob and to be updated according to your review.
>>>>>>
>>>>>> It should be doable to write a Python script to parse the C file and create an
>>>>>> equivalent binary (using "struct" module) which is just an array of integers
>>>>>> in the end. That avoids the need for a microblaze C compiler...
>>>>>
>>>>> There's no need for a microblaze compiler. pm_cfg_obj.c is simply
>>>>> declaring a u32 array and some #defines, any C compiler is enough.
>>>>>
>>>>> That said, my current solution (a trivial main.c that compiles the u32
>>>>> array and outputs it to a binary file) is not nice at all, and it
>>>>> requires a pm_defs.h file.
>>>>>
>>>>> The python script you mention looks way better from a user perspective,
>>>>> although the parsing might be a bit fragile. I'll consider it, thanks
>>>>> for the suggestion.
>>>>>
>>>>> In the past I even prototyped a python script that parses the Vivado
>>>>> .xpr project file and produces a pm_cfg_obj.c. It avoided the need to
>>>>> run the Xilinx XSDK just to generate pm_cfg_obj.c. It might also be
>>>>> extended to produce a .bin directly, or a self-standing .c file that
>>>>> doesn't need pm_defs.h, thus removing any licensing issue. But it never
>>>>> grew complete to handle all cases. Obvious, since *I* don't even know
>>>>> all of the cases. :)
>>>>
>>>>
>>>> Another approach would be to simply create and include a "god mode" config
>>>> object that just allows access to all periferals. As far as I can see, such a
>>>> config object would just work on all boards. There's nothing really board
>>>> specific in the config object, and it's rather lame anyway to have to go and
>>>> compile a new bootloader just because you want to use a SPI controller...
>>>>
>>>> The config object I compile into the pmu firmware is of that type anyway.
>>
>> Oh, that's interesting, but read below.
>>
>>> Most of that stuff should be the same for all boards. But there are some
>>> stuff which can be just board specific.
>>
>> Mike, I think Michal refers to boards like Ultra96 which need special
>> GPIO handling for their boot sequence, whose pm_cfg_obj.c have a final
>> section similar to this:
>>
>> 	/* GPO SECTION */
>> 	PM_CONFIG_GPO_SECTION_ID,		/* GPO Section ID */
>> 	PM_CONFIG_GPO1_BIT_2_MASK |
>> 	PM_CONFIG_GPO1_MIO_PIN_34_MAP |
>> 	0,					/* State of GPO pins */
>>
>> I suspect a "god mode" config cannot handle such cases.
> 
> That's ultra96 is one of that exception but it depends on your MIO
> configs too. It means you simply not assigned that gpo pins to PMU. You
> loose functionality but you should be able to fix it differently later
> in boot.

I have no clue what those bits do. If it's something like setting a pinmux or 
gpio state, that'd be easy enough to do in u-boot or kernel, they both can 
send commands to the PMU anyway.