[PATCH 00/10] xilinx: zynqmp: Support foreign vendor boards

[PATCH 00/10] xilinx: zynqmp: Support foreign vendor boards

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

At the moment the xilinx zynqmp soc is only supported by xilinx vendor
boards. Rework the xilinx zynqmp board code to support reuse by foreign
vendor boards.


Stefan Herbrechtsmeier (10):
  firmware: firmware-zynqmp: Check if rx channel dev pointer is valid
  firmware: firmware-zynqmp: Probe driver before use
  soc: xilinx: zynqmp: Add machine identification support
  xilinx: zynqmp: Use soc machine function to get silicon idcode name
  xilinx: cpuinfo: Print soc machine
  xilinx: common: Separate display cpu info function
  xilinx: zynqmp: make spi flash support optional
  tools: zynqmp_psu_init_minimize: Remove low level uart settings
  tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward
    declaration
  xilinx: zynqmp: Support vendor specific board_init

 board/xilinx/common/Makefile       |   3 +
 board/xilinx/common/board.c        |  30 +--
 board/xilinx/common/board.h        |   2 +
 board/xilinx/common/cpu-info.c     |  35 ++++
 board/xilinx/zynqmp/zynqmp.c       | 291 ++---------------------------
 drivers/firmware/firmware-zynqmp.c |  20 +-
 drivers/soc/soc_xilinx_zynqmp.c    | 289 +++++++++++++++++++++++++++-
 tools/zynqmp_psu_init_minimize.sh  |  22 +++
 8 files changed, 390 insertions(+), 302 deletions(-)
 create mode 100644 board/xilinx/common/cpu-info.c

-- 
2.30.2

[PATCH 01/10] firmware: firmware-zynqmp: Check if rx channel dev pointer is valid

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Check if rx channel dev pointer is valid and not if the address of the
pointer is valid.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 drivers/firmware/firmware-zynqmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index 0f0d2b07c0..341d7cf135 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -92,7 +92,7 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen)
 	    res_maxlen > PMUFW_PAYLOAD_ARG_CNT)
 		return -EINVAL;
 
-	if (!(zynqmp_power.tx_chan.dev) || !(&zynqmp_power.rx_chan.dev))
+	if (!(zynqmp_power.tx_chan.dev) || !(zynqmp_power.rx_chan.dev))
 		return -EINVAL;
 
 	debug("%s, Sending IPI message with ID: 0x%0x\n", __func__, req[0]);
-- 
2.30.2

[PATCH 02/10] firmware: firmware-zynqmp: Probe driver before use

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Probe the driver before use to ensure that the global data are valid.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 drivers/firmware/firmware-zynqmp.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index 341d7cf135..64b0873ed0 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -281,6 +281,20 @@ U_BOOT_DRIVER(zynqmp_power) = {
 };
 #endif
 
+static int __maybe_unused do_pm_probe(void)
+{
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_get_device_by_driver(UCLASS_FIRMWARE,
+					  DM_DRIVER_GET(zynqmp_power),
+					  &dev);
+	if (ret)
+		debug("%s: Probing device failed: %d\n", __func__, ret);
+
+	return ret;
+}
+
 int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
 				     u32 arg3, u32 *ret_payload)
 {
@@ -296,6 +310,10 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
 		u32 regs[] = {api_id, arg0, arg1, arg2, arg3};
 		int ret;
 
+		ret = do_pm_probe();
+		if (ret)
+			return ret;
+
 		if (api_id == PM_FPGA_LOAD) {
 			/* Swap addr_hi/low because of incompatibility */
 			u32 temp = regs[1];
-- 
2.30.2

[PATCH 03/10] soc: xilinx: zynqmp: Add machine identification support

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Add machine identification support based on the
zynqmp_get_silicon_idcode_name function in board/xilinx/zynqmp/zynqmp.c.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 drivers/soc/soc_xilinx_zynqmp.c | 289 +++++++++++++++++++++++++++++++-
 1 file changed, 286 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/soc_xilinx_zynqmp.c b/drivers/soc/soc_xilinx_zynqmp.c
index a71115b17c..45592ed534 100644
--- a/drivers/soc/soc_xilinx_zynqmp.c
+++ b/drivers/soc/soc_xilinx_zynqmp.c
@@ -3,10 +3,15 @@
  * Xilinx ZynqMP SOC driver
  *
  * Copyright (C) 2021 Xilinx, Inc.
+ * Michal Simek <michal.simek@xilinx.com>
+ *
+ * Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
+ * Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
  */
 
 #include <common.h>
 #include <dm.h>
+#include <dm/device_compat.h>
 #include <asm/cache.h>
 #include <soc.h>
 #include <zynqmp_firmware.h>
@@ -20,13 +25,260 @@
  * v2 -> 2(XCZU7EV-ES1, XCZU9EG-ES2, XCZU19EG-ES1)
  * v3 -> 3(Production Level)
  */
-static const char zynqmp_family[] = "ZynqMP";
+
+#define EFUSE_VCU_DIS_SHIFT	8
+#define EFUSE_VCU_DIS_MASK	BIT(EFUSE_VCU_DIS_SHIFT)
+#define EFUSE_GPU_DIS_SHIFT	5
+#define EFUSE_GPU_DIS_MASK	BIT(EFUSE_GPU_DIS_SHIFT)
+#define IDCODE2_PL_INIT_SHIFT	9
+#define IDCODE2_PL_INIT_MASK	BIT(IDCODE2_PL_INIT_SHIFT)
+
+#define ZYNQMP_VERSION_SIZE	7
+
+enum {
+	ZYNQMP_VARIANT_EG = BIT(0),
+	ZYNQMP_VARIANT_EV = BIT(1),
+	ZYNQMP_VARIANT_CG = BIT(2),
+	ZYNQMP_VARIANT_DR = BIT(3),
+};
+
+struct zynqmp_device {
+	u32 id;
+	u8 device;
+	u8 variants;
+};
 
 struct soc_xilinx_zynqmp_priv {
 	const char *family;
+	char machine[ZYNQMP_VERSION_SIZE];
 	char revision;
 };
 
+static struct zynqmp_device zynqmp_devices[] = {
+	{
+		.id = 0x04688093,
+		.device = 1,
+		.variants = ZYNQMP_VARIANT_EG,
+	},
+	{
+		.id = 0x04711093,
+		.device = 2,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
+	},
+	{
+		.id = 0x04710093,
+		.device = 3,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
+	},
+	{
+		.id = 0x04721093,
+		.device = 4,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
+			ZYNQMP_VARIANT_EV,
+	},
+	{
+		.id = 0x04720093,
+		.device = 5,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
+			ZYNQMP_VARIANT_EV,
+	},
+	{
+		.id = 0x04739093,
+		.device = 6,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
+	},
+	{
+		.id = 0x04730093,
+		.device = 7,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
+			ZYNQMP_VARIANT_EV,
+	},
+	{
+		.id = 0x04738093,
+		.device = 9,
+		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
+	},
+	{
+		.id = 0x04740093,
+		.device = 11,
+		.variants = ZYNQMP_VARIANT_EG,
+	},
+	{
+		.id = 0x04750093,
+		.device = 15,
+		.variants = ZYNQMP_VARIANT_EG,
+	},
+	{
+		.id = 0x04759093,
+		.device = 17,
+		.variants = ZYNQMP_VARIANT_EG,
+	},
+	{
+		.id = 0x04758093,
+		.device = 19,
+		.variants = ZYNQMP_VARIANT_EG,
+	},
+	{
+		.id = 0x047E1093,
+		.device = 21,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047E3093,
+		.device = 23,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047E5093,
+		.device = 25,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047E4093,
+		.device = 27,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047E0093,
+		.device = 28,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047E2093,
+		.device = 29,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047E6093,
+		.device = 39,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047FD093,
+		.device = 43,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047F8093,
+		.device = 46,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047FF093,
+		.device = 47,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047FB093,
+		.device = 48,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x047FE093,
+		.device = 49,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x046d0093,
+		.device = 67,
+		.variants = ZYNQMP_VARIANT_DR,
+	},
+	{
+		.id = 0x04714093,
+		.device = 24,
+		.variants = 0,
+	},
+	{
+		.id = 0x04724093,
+		.device = 26,
+		.variants = 0,
+	},
+};
+
+static const char zynqmp_family[] = "ZynqMP";
+
+static const struct zynqmp_device *zynqmp_get_device(u32 idcode)
+{
+	idcode &= 0x0FFFFFFF;
+
+	for (int i = 0; i < ARRAY_SIZE(zynqmp_devices); i++) {
+		if (zynqmp_devices[i].id == idcode)
+			return &zynqmp_devices[i];
+	}
+
+	return NULL;
+}
+
+static int soc_xilinx_zynqmp_detect_machine(struct udevice *dev, u32 idcode,
+					    u32 idcode2)
+{
+	struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
+	const struct zynqmp_device *device;
+	int ret;
+
+	device = zynqmp_get_device(idcode);
+
+	if (!device)
+		return 0;
+
+	/* Add device prefix to the name */
+	ret = snprintf(priv->machine, sizeof(priv->machine), "%s%d",
+		       device->variants ? "zu" : "xck", device->device);
+	if (ret < 0)
+		return ret;
+
+	if (device->variants & ZYNQMP_VARIANT_EV) {
+		/* Devices with EV variant might be EG/CG/EV family */
+		if (idcode2 & IDCODE2_PL_INIT_MASK) {
+			u32 family = ((idcode2 & EFUSE_VCU_DIS_MASK) >>
+				      EFUSE_VCU_DIS_SHIFT) << 1 |
+				     ((idcode2 & EFUSE_GPU_DIS_MASK) >>
+				      EFUSE_GPU_DIS_SHIFT);
+
+			/*
+			 * Get family name based on extended idcode values as
+			 * determined on UG1087, EXTENDED_IDCODE register
+			 * description
+			 */
+			switch (family) {
+			case 0x00:
+				strlcat(priv->machine, "ev",
+					sizeof(priv->machine));
+				break;
+			case 0x10:
+				strlcat(priv->machine, "eg",
+					sizeof(priv->machine));
+				break;
+			case 0x11:
+				strlcat(priv->machine, "cg",
+					sizeof(priv->machine));
+				break;
+			default:
+				/* Do not append family name*/
+				break;
+			}
+		} else {
+			/*
+			 * When PL powered down the VCU Disable efuse cannot be
+			 * read. So, ignore the bit and just findout if it is CG
+			 * or EG/EV variant.
+			 */
+			strlcat(priv->machine, (idcode2 & EFUSE_GPU_DIS_MASK) ?
+				"cg" : "e", sizeof(priv->machine));
+		}
+	} else if (device->variants & ZYNQMP_VARIANT_CG) {
+		/* Devices with CG variant might be EG or CG family */
+		strlcat(priv->machine, (idcode2 & EFUSE_GPU_DIS_MASK) ?
+			"cg" : "eg", sizeof(priv->machine));
+	} else if (device->variants & ZYNQMP_VARIANT_EG) {
+		strlcat(priv->machine, "eg", sizeof(priv->machine));
+	} else if (device->variants & ZYNQMP_VARIANT_DR) {
+		strlcat(priv->machine, "dr", sizeof(priv->machine));
+	}
+
+	return 0;
+}
+
 static int soc_xilinx_zynqmp_get_family(struct udevice *dev, char *buf, int size)
 {
 	struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
@@ -34,6 +286,17 @@ static int soc_xilinx_zynqmp_get_family(struct udevice *dev, char *buf, int size
 	return snprintf(buf, size, "%s", priv->family);
 }
 
+int soc_xilinx_zynqmp_get_machine(struct udevice *dev, char *buf, int size)
+{
+	struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
+	const char *machine = priv->machine;
+
+	if (!machine[0])
+		machine = "unknown";
+
+	return snprintf(buf, size, "%s", machine);
+}
+
 static int soc_xilinx_zynqmp_get_revision(struct udevice *dev, char *buf, int size)
 {
 	struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
@@ -44,6 +307,7 @@ static int soc_xilinx_zynqmp_get_revision(struct udevice *dev, char *buf, int si
 static const struct soc_ops soc_xilinx_zynqmp_ops = {
 	.get_family = soc_xilinx_zynqmp_get_family,
 	.get_revision = soc_xilinx_zynqmp_get_revision,
+	.get_machine = soc_xilinx_zynqmp_get_machine,
 };
 
 static int soc_xilinx_zynqmp_probe(struct udevice *dev)
@@ -54,8 +318,7 @@ static int soc_xilinx_zynqmp_probe(struct udevice *dev)
 
 	priv->family = zynqmp_family;
 
-	if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3 ||
-	    !IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
+	if (!IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
 		ret = zynqmp_mmio_read(ZYNQMP_PS_VERSION, &ret_payload[2]);
 	else
 		ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,
@@ -65,6 +328,26 @@ static int soc_xilinx_zynqmp_probe(struct udevice *dev)
 
 	priv->revision = ret_payload[2] & ZYNQMP_PS_VER_MASK;
 
+	if (IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE)) {
+		/*
+		 * Firmware returns:
+		 * payload[0][31:0] = status of the operation
+		 * payload[1] = IDCODE
+		 * payload[2][19:0] = Version
+		 * payload[2][28:20] = EXTENDED_IDCODE
+		 * payload[2][29] = PL_INIT
+		 */
+		u32 idcode = ret_payload[1];
+		u32 idcode2 = ret_payload[2] >>
+				   ZYNQMP_CSU_VERSION_EMPTY_SHIFT;
+		dev_dbg(dev, "IDCODE: 0x%0x, IDCODE2: 0x%0x\n", idcode,
+			idcode2);
+
+		ret = soc_xilinx_zynqmp_detect_machine(dev, idcode, idcode2);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.30.2

[PATCH 04/10] xilinx: zynqmp: Use soc machine function to get silicon idcode name

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Use the soc_get_machine function of the soc uclass to get silicon idcode
name for the fpga init.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 board/xilinx/zynqmp/zynqmp.c | 287 ++---------------------------------
 1 file changed, 15 insertions(+), 272 deletions(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index e311aa772c..06f6dbab18 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -19,6 +19,7 @@
 #include <sata.h>
 #include <ahci.h>
 #include <scsi.h>
+#include <soc.h>
 #include <malloc.h>
 #include <memalign.h>
 #include <wdt.h>
@@ -44,278 +45,10 @@
 
 #include "pm_cfg_obj.h"
 
-#define ZYNQMP_VERSION_SIZE	7
-#define EFUSE_VCU_DIS_MASK	0x100
-#define EFUSE_VCU_DIS_SHIFT	8
-#define EFUSE_GPU_DIS_MASK	0x20
-#define EFUSE_GPU_DIS_SHIFT	5
-#define IDCODE2_PL_INIT_MASK	0x200
-#define IDCODE2_PL_INIT_SHIFT	9
-
 DECLARE_GLOBAL_DATA_PTR;
 
 #if CONFIG_IS_ENABLED(FPGA) && defined(CONFIG_FPGA_ZYNQMPPL)
 static xilinx_desc zynqmppl = XILINX_ZYNQMP_DESC;
-
-enum {
-	ZYNQMP_VARIANT_EG = BIT(0U),
-	ZYNQMP_VARIANT_EV = BIT(1U),
-	ZYNQMP_VARIANT_CG = BIT(2U),
-	ZYNQMP_VARIANT_DR = BIT(3U),
-};
-
-static const struct {
-	u32 id;
-	u8 device;
-	u8 variants;
-} zynqmp_devices[] = {
-	{
-		.id = 0x04688093,
-		.device = 1,
-		.variants = ZYNQMP_VARIANT_EG,
-	},
-	{
-		.id = 0x04711093,
-		.device = 2,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
-	},
-	{
-		.id = 0x04710093,
-		.device = 3,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
-	},
-	{
-		.id = 0x04721093,
-		.device = 4,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
-			ZYNQMP_VARIANT_EV,
-	},
-	{
-		.id = 0x04720093,
-		.device = 5,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
-			ZYNQMP_VARIANT_EV,
-	},
-	{
-		.id = 0x04739093,
-		.device = 6,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
-	},
-	{
-		.id = 0x04730093,
-		.device = 7,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
-			ZYNQMP_VARIANT_EV,
-	},
-	{
-		.id = 0x04738093,
-		.device = 9,
-		.variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
-	},
-	{
-		.id = 0x04740093,
-		.device = 11,
-		.variants = ZYNQMP_VARIANT_EG,
-	},
-	{
-		.id = 0x04750093,
-		.device = 15,
-		.variants = ZYNQMP_VARIANT_EG,
-	},
-	{
-		.id = 0x04759093,
-		.device = 17,
-		.variants = ZYNQMP_VARIANT_EG,
-	},
-	{
-		.id = 0x04758093,
-		.device = 19,
-		.variants = ZYNQMP_VARIANT_EG,
-	},
-	{
-		.id = 0x047E1093,
-		.device = 21,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047E3093,
-		.device = 23,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047E5093,
-		.device = 25,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047E4093,
-		.device = 27,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047E0093,
-		.device = 28,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047E2093,
-		.device = 29,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047E6093,
-		.device = 39,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047FD093,
-		.device = 43,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047F8093,
-		.device = 46,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047FF093,
-		.device = 47,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047FB093,
-		.device = 48,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x047FE093,
-		.device = 49,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-	{
-		.id = 0x046d0093,
-		.device = 67,
-		.variants = ZYNQMP_VARIANT_DR,
-	},
-};
-
-static const struct {
-	u32 id;
-	char *name;
-} zynqmp_svd_devices[] = {
-	{
-		.id = 0x04714093,
-		.name = "xck24"
-	},
-	{
-		.id = 0x04724093,
-		.name = "xck26",
-	},
-};
-
-static char *zynqmp_detect_svd_name(u32 idcode)
-{
-	u32 i;
-
-	for (i = 0; i < ARRAY_SIZE(zynqmp_svd_devices); i++) {
-		if (zynqmp_svd_devices[i].id == (idcode & 0x0FFFFFFF))
-			return zynqmp_svd_devices[i].name;
-	}
-
-	return "unknown";
-}
-
-static char *zynqmp_get_silicon_idcode_name(void)
-{
-	u32 i;
-	u32 idcode, idcode2;
-	char name[ZYNQMP_VERSION_SIZE];
-	u32 ret_payload[PAYLOAD_ARG_CNT];
-	int ret;
-
-	ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0, ret_payload);
-	if (ret) {
-		debug("%s: Getting chipid failed\n", __func__);
-		return "unknown";
-	}
-
-	/*
-	 * Firmware returns:
-	 * payload[0][31:0]  = status of the operation
-	 * payload[1]] = IDCODE
-	 * payload[2][19:0]  = Version
-	 * payload[2][28:20] = EXTENDED_IDCODE
-	 * payload[2][29] = PL_INIT
-	 */
-
-	idcode  = ret_payload[1];
-	idcode2 = ret_payload[2] >> ZYNQMP_CSU_VERSION_EMPTY_SHIFT;
-	debug("%s, IDCODE: 0x%0x, IDCODE2: 0x%0x\r\n", __func__, idcode,
-	      idcode2);
-
-	for (i = 0; i < ARRAY_SIZE(zynqmp_devices); i++) {
-		if (zynqmp_devices[i].id == (idcode & 0x0FFFFFFF))
-			break;
-	}
-
-	if (i >= ARRAY_SIZE(zynqmp_devices))
-		return zynqmp_detect_svd_name(idcode);
-
-	/* Add device prefix to the name */
-	ret = snprintf(name, ZYNQMP_VERSION_SIZE, "zu%d",
-		       zynqmp_devices[i].device);
-	if (ret < 0)
-		return "unknown";
-
-	if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_EV) {
-		/* Devices with EV variant might be EG/CG/EV family */
-		if (idcode2 & IDCODE2_PL_INIT_MASK) {
-			u32 family = ((idcode2 & EFUSE_VCU_DIS_MASK) >>
-				      EFUSE_VCU_DIS_SHIFT) << 1 |
-				     ((idcode2 & EFUSE_GPU_DIS_MASK) >>
-				      EFUSE_GPU_DIS_SHIFT);
-
-			/*
-			 * Get family name based on extended idcode values as
-			 * determined on UG1087, EXTENDED_IDCODE register
-			 * description
-			 */
-			switch (family) {
-			case 0x00:
-				strncat(name, "ev", 2);
-				break;
-			case 0x10:
-				strncat(name, "eg", 2);
-				break;
-			case 0x11:
-				strncat(name, "cg", 2);
-				break;
-			default:
-				/* Do not append family name*/
-				break;
-			}
-		} else {
-			/*
-			 * When PL powered down the VCU Disable efuse cannot be
-			 * read. So, ignore the bit and just findout if it is CG
-			 * or EG/EV variant.
-			 */
-			strncat(name, (idcode2 & EFUSE_GPU_DIS_MASK) ? "cg" :
-				"e", 2);
-		}
-	} else if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_CG) {
-		/* Devices with CG variant might be EG or CG family */
-		strncat(name, (idcode2 & EFUSE_GPU_DIS_MASK) ? "cg" : "eg", 2);
-	} else if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_EG) {
-		strncat(name, "eg", 2);
-	} else if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_DR) {
-		strncat(name, "dr", 2);
-	} else {
-		debug("Variant not identified\n");
-	}
-
-	return strdup(name);
-}
 #endif
 
 int __maybe_unused psu_uboot_init(void)
@@ -406,6 +139,11 @@ static void print_secure_boot(void)
 
 int board_init(void)
 {
+#if CONFIG_IS_ENABLED(FPGA) && defined(CONFIG_FPGA_ZYNQMPPL)
+	struct udevice *soc;
+	char name[SOC_MAX_STR_SIZE];
+	int ret;
+#endif
 #if defined(CONFIG_ZYNQMP_FIRMWARE)
 	struct udevice *dev;
 
@@ -432,10 +170,15 @@ int board_init(void)
 	printf("EL Level:\tEL%d\n", current_el());
 
 #if CONFIG_IS_ENABLED(FPGA) && defined(CONFIG_FPGA_ZYNQMPPL)
-	zynqmppl.name = zynqmp_get_silicon_idcode_name();
-	printf("Chip ID:\t%s\n", zynqmppl.name);
-	fpga_init();
-	fpga_add(fpga_xilinx, &zynqmppl);
+	ret = soc_get(&soc);
+	if (!ret) {
+		ret = soc_get_machine(soc, name, sizeof(name));
+		if (ret >= 0) {
+			zynqmppl.name = strdup(name);
+			fpga_init();
+			fpga_add(fpga_xilinx, &zynqmppl);
+		}
+	}
 #endif
 
 	/* display secure boot information */
-- 
2.30.2

[PATCH 05/10] xilinx: cpuinfo: Print soc machine

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Print the soc machine in the print_cpuinfo function.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 board/xilinx/common/board.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 629a6ee036..402fa77006 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -506,6 +506,10 @@ int print_cpuinfo(void)
 	if (ret)
 		printf("Silicon: %s\n", name);
 
+	ret = soc_get_machine(soc, name, SOC_MAX_STR_SIZE);
+	if (ret)
+		printf("Chip:  %s\n", name);
+
 	return 0;
 }
 #endif
-- 
2.30.2

[PATCH 06/10] xilinx: common: Separate display cpu info function

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Move the print_cpuinfo function of CONFIG_DISPLAY_CPUINFO into its own
source file to support reuse by other board vendors.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 board/xilinx/common/Makefile   |  3 +++
 board/xilinx/common/board.c    | 29 ----------------------------
 board/xilinx/common/cpu-info.c | 35 ++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 29 deletions(-)
 create mode 100644 board/xilinx/common/cpu-info.c

diff --git a/board/xilinx/common/Makefile b/board/xilinx/common/Makefile
index 212028478c..cdc3c96774 100644
--- a/board/xilinx/common/Makefile
+++ b/board/xilinx/common/Makefile
@@ -5,6 +5,9 @@
 #
 
 obj-y	+= board.o
+ifndef CONFIG_ARCH_ZYNQ
+obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
+endif
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_CMD_FRU) += fru.o fru_ops.o
 endif
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 402fa77006..5f2afb9def 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -485,35 +485,6 @@ int __maybe_unused board_fit_config_name_match(const char *name)
 	return -1;
 }
 
-#if defined(CONFIG_DISPLAY_CPUINFO) && !defined(CONFIG_ARCH_ZYNQ)
-int print_cpuinfo(void)
-{
-	struct udevice *soc;
-	char name[SOC_MAX_STR_SIZE];
-	int ret;
-
-	ret = soc_get(&soc);
-	if (ret) {
-		printf("CPU:   UNKNOWN\n");
-		return 0;
-	}
-
-	ret = soc_get_family(soc, name, SOC_MAX_STR_SIZE);
-	if (ret)
-		printf("CPU:   %s\n", name);
-
-	ret = soc_get_revision(soc, name, SOC_MAX_STR_SIZE);
-	if (ret)
-		printf("Silicon: %s\n", name);
-
-	ret = soc_get_machine(soc, name, SOC_MAX_STR_SIZE);
-	if (ret)
-		printf("Chip:  %s\n", name);
-
-	return 0;
-}
-#endif
-
 #if CONFIG_IS_ENABLED(DTB_RESELECT)
 #define MAX_NAME_LENGTH	50
 
diff --git a/board/xilinx/common/cpu-info.c b/board/xilinx/common/cpu-info.c
new file mode 100644
index 0000000000..4a863d00de
--- /dev/null
+++ b/board/xilinx/common/cpu-info.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2014 - 2020 Xilinx, Inc.
+ * Michal Simek <michal.simek@xilinx.com>
+ */
+
+#include <common.h>
+#include <soc.h>
+
+int print_cpuinfo(void)
+{
+	struct udevice *soc;
+	char name[SOC_MAX_STR_SIZE];
+	int ret;
+
+	ret = soc_get(&soc);
+	if (ret) {
+		printf("CPU:   UNKNOWN\n");
+		return 0;
+	}
+
+	ret = soc_get_family(soc, name, SOC_MAX_STR_SIZE);
+	if (ret)
+		printf("CPU:   %s\n", name);
+
+	ret = soc_get_revision(soc, name, SOC_MAX_STR_SIZE);
+	if (ret)
+		printf("Silicon: %s\n", name);
+
+	ret = soc_get_machine(soc, name, SOC_MAX_STR_SIZE);
+	if (ret)
+		printf("Chip:  %s\n", name);
+
+	return 0;
+}
-- 
2.30.2

[PATCH 07/10] xilinx: zynqmp: make spi flash support optional

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

The set_dfu_alt_info function use the CONFIG_SYS_SPI_U_BOOT_OFFS define
to set the dfu_alt_info environment variable for qspi boot mode. Guard
the usage of CONFIG_SYS_SPI_U_BOOT_OFFS to make spi flash support
optional.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 board/xilinx/zynqmp/zynqmp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 06f6dbab18..106c3953e1 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -667,6 +667,7 @@ void set_dfu_alt_info(char *interface, char *devstr)
 				 bootseq, multiboot, bootseq,
 				 CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq);
 		break;
+#if defined(CONFIG_SYS_SPI_U_BOOT_OFFS)
 	case QSPI_MODE_24BIT:
 	case QSPI_MODE_32BIT:
 		snprintf(buf, DFU_ALT_BUF_LEN,
@@ -675,6 +676,7 @@ void set_dfu_alt_info(char *interface, char *devstr)
 			 multiboot * SZ_32K, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME,
 			 CONFIG_SYS_SPI_U_BOOT_OFFS);
 		break;
+#endif
 	default:
 		return;
 	}
-- 
2.30.2

[PATCH 08/10] tools: zynqmp_psu_init_minimize: Remove low level uart settings

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

There is no reason to do serial initialization. Uart driver does it
already based on DT. Good effect is that it is clear which interface is
console.
The resulting change was done in past by commit 84d2bbf082fa ("arm64:
zynqmp: Remove low level UART setting").

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 tools/zynqmp_psu_init_minimize.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/zynqmp_psu_init_minimize.sh b/tools/zynqmp_psu_init_minimize.sh
index 4ee418f07e..31fbeac327 100755
--- a/tools/zynqmp_psu_init_minimize.sh
+++ b/tools/zynqmp_psu_init_minimize.sh
@@ -2,6 +2,8 @@
 # SPDX-License-Identifier: GPL-2.0+
 # Copyright (C) 2018 Michal Simek <michal.simek@xilinx.com>
 # Copyright (C) 2019 Luca Ceresoli <luca@lucaceresoli.net>
+# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
+# Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
 
 usage()
 {
@@ -144,4 +146,19 @@ sed -i -r 's|\((._code .= [x[:xdigit:]]+)\)|\1|g' ${TMP}
 # Convert back newlines
 tr "\r" "\n" <${TMP} >${OUT}
 
+# Remove unnecessary settings
+# - Low level UART
+SETTINGS_TO_REMOVE="0xFF000000
+0xFF000004
+0xFF000018
+0xFF000034
+0xFF010000
+0xFF010004
+0xFF010018
+0xFF010034
+"
+for i in $SETTINGS_TO_REMOVE; do
+sed -i "/^\tpsu_mask_write($i,.*$/d" ${OUT}
+done
+
 rm ${TMP}
-- 
2.30.2

[PATCH 09/10] tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward declaration

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

A forward declaration for the serdes_illcalib function.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 tools/zynqmp_psu_init_minimize.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/zynqmp_psu_init_minimize.sh b/tools/zynqmp_psu_init_minimize.sh
index 31fbeac327..8411065e13 100755
--- a/tools/zynqmp_psu_init_minimize.sh
+++ b/tools/zynqmp_psu_init_minimize.sh
@@ -108,6 +108,11 @@ cat << EOF >${TMP}
 #include <asm/arch/psu_init_gpl.h>
 #include <xil_io.h>
 
+static int serdes_illcalib(u32 lane3_protocol, u32 lane3_rate,
+			   u32 lane2_protocol, u32 lane2_rate,
+			   u32 lane1_protocol, u32 lane1_rate,
+			   u32 lane0_protocol, u32 lane0_rate);
+
 EOF
 
 cat ${OUT} >>${TMP}
-- 
2.30.2

[PATCH 10/10] xilinx: zynqmp: Support vendor specific board_init

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Add a board_init_xilinx function to allow foreign vendors to reuse the
xilinx zynqmp board code and add addition code to the board_init
function.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

---

 board/xilinx/common/board.c  | 5 +++++
 board/xilinx/common/board.h  | 2 ++
 board/xilinx/zynqmp/zynqmp.c | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 5f2afb9def..643959bee7 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -401,6 +401,11 @@ void *board_fdt_blob_setup(int *err)
 }
 #endif
 
+int board_init_xilinx(void)
+{
+	return 0;
+}
+
 #if defined(CONFIG_BOARD_LATE_INIT)
 static int env_set_by_index(const char *name, int index, char *data)
 {
diff --git a/board/xilinx/common/board.h b/board/xilinx/common/board.h
index 69e642429b..3f6377d706 100644
--- a/board/xilinx/common/board.h
+++ b/board/xilinx/common/board.h
@@ -7,6 +7,8 @@
 #ifndef _BOARD_XILINX_COMMON_BOARD_H
 #define _BOARD_XILINX_COMMON_BOARD_H
 
+int board_init_xilinx(void);
+
 int board_late_init_xilinx(void);
 
 int xilinx_read_eeprom(void);
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 106c3953e1..ec195105ad 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -186,7 +186,7 @@ int board_init(void)
 	if (current_el() == 3)
 		printf("Multiboot:\t%d\n", multi_boot());
 
-	return 0;
+	return board_init_xilinx();
 }
 
 int board_early_init_r(void)
-- 
2.30.2

Re: [PATCH 02/10] firmware: firmware-zynqmp: Probe driver before use

[Michal Simek]

On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
> [CAUTION: External Email]
> 
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> Probe the driver before use to ensure that the global data are valid.

It is not clear what the issue is. And this function is called in SPL or EL3 and 
likely multiple times.
Can you please clarify?

Thanks,
Michal

Re: [PATCH 10/10] xilinx: zynqmp: Support vendor specific board_init

[Michal Simek]

On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
> [CAUTION: External Email]
> 
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> Add a board_init_xilinx function to allow foreign vendors to reuse the
> xilinx zynqmp board code and add addition code to the board_init
> function.

Do you plan to add support for that board who will be using it?

> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> ---
> 
>   board/xilinx/common/board.c  | 5 +++++
>   board/xilinx/common/board.h  | 2 ++
>   board/xilinx/zynqmp/zynqmp.c | 2 +-
>   3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index 5f2afb9def..643959bee7 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -401,6 +401,11 @@ void *board_fdt_blob_setup(int *err)
>   }
>   #endif
> 
> +int board_init_xilinx(void)
> +{
> +       return 0;
> +}

This file is used by zynq/zynqmp/versal platforms. I expect there is going to be 
any message that this function is unused on zynq and versal.

M

Re: [PATCH 09/10] tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward declaration

[Michal Simek]

On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
> [CAUTION: External Email]
> 
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> A forward declaration for the serdes_illcalib function.
> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> ---
> 
>   tools/zynqmp_psu_init_minimize.sh | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/tools/zynqmp_psu_init_minimize.sh b/tools/zynqmp_psu_init_minimize.sh
> index 31fbeac327..8411065e13 100755
> --- a/tools/zynqmp_psu_init_minimize.sh
> +++ b/tools/zynqmp_psu_init_minimize.sh
> @@ -108,6 +108,11 @@ cat << EOF >${TMP}
>   #include <asm/arch/psu_init_gpl.h>
>   #include <xil_io.h>
> 
> +static int serdes_illcalib(u32 lane3_protocol, u32 lane3_rate,
> +                          u32 lane2_protocol, u32 lane2_rate,
> +                          u32 lane1_protocol, u32 lane1_rate,
> +                          u32 lane0_protocol, u32 lane0_rate);
> +

Better to resort that functions to avoid these additional lines.

M

Re: [PATCH 03/10] soc: xilinx: zynqmp: Add machine identification support

[Michal Simek]

On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
> [CAUTION: External Email]
> 
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> Add machine identification support based on the
> zynqmp_get_silicon_idcode_name function in board/xilinx/zynqmp/zynqmp.c.
> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> ---
> 
>   drivers/soc/soc_xilinx_zynqmp.c | 289 +++++++++++++++++++++++++++++++-
>   1 file changed, 286 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/soc_xilinx_zynqmp.c b/drivers/soc/soc_xilinx_zynqmp.c
> index a71115b17c..45592ed534 100644
> --- a/drivers/soc/soc_xilinx_zynqmp.c
> +++ b/drivers/soc/soc_xilinx_zynqmp.c
> @@ -3,10 +3,15 @@
>    * Xilinx ZynqMP SOC driver
>    *
>    * Copyright (C) 2021 Xilinx, Inc.
> + * Michal Simek <michal.simek@xilinx.com>
> + *
> + * Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
> + * Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>    */
> 
>   #include <common.h>
>   #include <dm.h>
> +#include <dm/device_compat.h>
>   #include <asm/cache.h>
>   #include <soc.h>
>   #include <zynqmp_firmware.h>
> @@ -20,13 +25,260 @@
>    * v2 -> 2(XCZU7EV-ES1, XCZU9EG-ES2, XCZU19EG-ES1)
>    * v3 -> 3(Production Level)
>    */
> -static const char zynqmp_family[] = "ZynqMP";

please keep this in origin location.

> +
> +#define EFUSE_VCU_DIS_SHIFT    8
> +#define EFUSE_VCU_DIS_MASK     BIT(EFUSE_VCU_DIS_SHIFT)
> +#define EFUSE_GPU_DIS_SHIFT    5
> +#define EFUSE_GPU_DIS_MASK     BIT(EFUSE_GPU_DIS_SHIFT)
> +#define IDCODE2_PL_INIT_SHIFT  9
> +#define IDCODE2_PL_INIT_MASK   BIT(IDCODE2_PL_INIT_SHIFT)
> +
> +#define ZYNQMP_VERSION_SIZE    7
> +
> +enum {
> +       ZYNQMP_VARIANT_EG = BIT(0),
> +       ZYNQMP_VARIANT_EV = BIT(1),
> +       ZYNQMP_VARIANT_CG = BIT(2),
> +       ZYNQMP_VARIANT_DR = BIT(3),
> +};
> +
> +struct zynqmp_device {
> +       u32 id;
> +       u8 device;
> +       u8 variants;
> +};
> 
>   struct soc_xilinx_zynqmp_priv {
>          const char *family;
> +       char machine[ZYNQMP_VERSION_SIZE];
>          char revision;
>   };
> 
> +static struct zynqmp_device zynqmp_devices[] = {
> +       {
> +               .id = 0x04688093,
> +               .device = 1,
> +               .variants = ZYNQMP_VARIANT_EG,
> +       },
> +       {
> +               .id = 0x04711093,
> +               .device = 2,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> +       },
> +       {
> +               .id = 0x04710093,
> +               .device = 3,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> +       },
> +       {
> +               .id = 0x04721093,
> +               .device = 4,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
> +                       ZYNQMP_VARIANT_EV,
> +       },
> +       {
> +               .id = 0x04720093,
> +               .device = 5,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
> +                       ZYNQMP_VARIANT_EV,
> +       },
> +       {
> +               .id = 0x04739093,
> +               .device = 6,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> +       },
> +       {
> +               .id = 0x04730093,
> +               .device = 7,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
> +                       ZYNQMP_VARIANT_EV,
> +       },
> +       {
> +               .id = 0x04738093,
> +               .device = 9,
> +               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> +       },
> +       {
> +               .id = 0x04740093,
> +               .device = 11,
> +               .variants = ZYNQMP_VARIANT_EG,
> +       },
> +       {
> +               .id = 0x04750093,
> +               .device = 15,
> +               .variants = ZYNQMP_VARIANT_EG,
> +       },
> +       {
> +               .id = 0x04759093,
> +               .device = 17,
> +               .variants = ZYNQMP_VARIANT_EG,
> +       },
> +       {
> +               .id = 0x04758093,
> +               .device = 19,
> +               .variants = ZYNQMP_VARIANT_EG,
> +       },
> +       {
> +               .id = 0x047E1093,
> +               .device = 21,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047E3093,
> +               .device = 23,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047E5093,
> +               .device = 25,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047E4093,
> +               .device = 27,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047E0093,
> +               .device = 28,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047E2093,
> +               .device = 29,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047E6093,
> +               .device = 39,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047FD093,
> +               .device = 43,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047F8093,
> +               .device = 46,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047FF093,
> +               .device = 47,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047FB093,
> +               .device = 48,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x047FE093,
> +               .device = 49,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x046d0093,
> +               .device = 67,
> +               .variants = ZYNQMP_VARIANT_DR,
> +       },
> +       {
> +               .id = 0x04714093,
> +               .device = 24,
> +               .variants = 0,
> +       },
> +       {
> +               .id = 0x04724093,
> +               .device = 26,
> +               .variants = 0,
> +       },

This sck merge is good but it is completely hidden in the patch.
I would prefer if you can do change in origin code and then c&p new one.

And also merged it with 4/10 to directly use it.





> +};
> +
> +static const char zynqmp_family[] = "ZynqMP";

As mentioned above.

> +
> +static const struct zynqmp_device *zynqmp_get_device(u32 idcode)
> +{
> +       idcode &= 0x0FFFFFFF;

We should create macro for this. I know in origin code this value is used but 
macro would be better.

> +
> +       for (int i = 0; i < ARRAY_SIZE(zynqmp_devices); i++) {
> +               if (zynqmp_devices[i].id == idcode)
> +                       return &zynqmp_devices[i];
> +       }
> +
> +       return NULL;
> +}
> +
> +static int soc_xilinx_zynqmp_detect_machine(struct udevice *dev, u32 idcode,
> +                                           u32 idcode2)
> +{
> +       struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
> +       const struct zynqmp_device *device;
> +       int ret;
> +
> +       device = zynqmp_get_device(idcode);
> +

remove this newline.

> +       if (!device)
> +               return 0;
> +
> +       /* Add device prefix to the name */
> +       ret = snprintf(priv->machine, sizeof(priv->machine), "%s%d",
> +                      device->variants ? "zu" : "xck", device->device);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (device->variants & ZYNQMP_VARIANT_EV) {
> +               /* Devices with EV variant might be EG/CG/EV family */
> +               if (idcode2 & IDCODE2_PL_INIT_MASK) {
> +                       u32 family = ((idcode2 & EFUSE_VCU_DIS_MASK) >>
> +                                     EFUSE_VCU_DIS_SHIFT) << 1 |
> +                                    ((idcode2 & EFUSE_GPU_DIS_MASK) >>
> +                                     EFUSE_GPU_DIS_SHIFT);
> +
> +                       /*
> +                        * Get family name based on extended idcode values as
> +                        * determined on UG1087, EXTENDED_IDCODE register
> +                        * description
> +                        */
> +                       switch (family) {
> +                       case 0x00:
> +                               strlcat(priv->machine, "ev",
> +                                       sizeof(priv->machine));
> +                               break;
> +                       case 0x10:
> +                               strlcat(priv->machine, "eg",
> +                                       sizeof(priv->machine));
> +                               break;
> +                       case 0x11:
> +                               strlcat(priv->machine, "cg",
> +                                       sizeof(priv->machine));
> +                               break;
> +                       default:
> +                               /* Do not append family name*/
> +                               break;
> +                       }
> +               } else {
> +                       /*
> +                        * When PL powered down the VCU Disable efuse cannot be
> +                        * read. So, ignore the bit and just findout if it is CG
> +                        * or EG/EV variant.
> +                        */
> +                       strlcat(priv->machine, (idcode2 & EFUSE_GPU_DIS_MASK) ?
> +                               "cg" : "e", sizeof(priv->machine));
> +               }
> +       } else if (device->variants & ZYNQMP_VARIANT_CG) {
> +               /* Devices with CG variant might be EG or CG family */
> +               strlcat(priv->machine, (idcode2 & EFUSE_GPU_DIS_MASK) ?
> +                       "cg" : "eg", sizeof(priv->machine));
> +       } else if (device->variants & ZYNQMP_VARIANT_EG) {
> +               strlcat(priv->machine, "eg", sizeof(priv->machine));
> +       } else if (device->variants & ZYNQMP_VARIANT_DR) {
> +               strlcat(priv->machine, "dr", sizeof(priv->machine));
> +       }
> +
> +       return 0;
> +}
> +
>   static int soc_xilinx_zynqmp_get_family(struct udevice *dev, char *buf, int size)
>   {
>          struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
> @@ -34,6 +286,17 @@ static int soc_xilinx_zynqmp_get_family(struct udevice *dev, char *buf, int size
>          return snprintf(buf, size, "%s", priv->family);
>   }
> 
> +int soc_xilinx_zynqmp_get_machine(struct udevice *dev, char *buf, int size)
> +{
> +       struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
> +       const char *machine = priv->machine;
> +
> +       if (!machine[0])
> +               machine = "unknown";
> +
> +       return snprintf(buf, size, "%s", machine);
> +}
> +
>   static int soc_xilinx_zynqmp_get_revision(struct udevice *dev, char *buf, int size)
>   {
>          struct soc_xilinx_zynqmp_priv *priv = dev_get_priv(dev);
> @@ -44,6 +307,7 @@ static int soc_xilinx_zynqmp_get_revision(struct udevice *dev, char *buf, int si
>   static const struct soc_ops soc_xilinx_zynqmp_ops = {
>          .get_family = soc_xilinx_zynqmp_get_family,
>          .get_revision = soc_xilinx_zynqmp_get_revision,
> +       .get_machine = soc_xilinx_zynqmp_get_machine,
>   };
> 
>   static int soc_xilinx_zynqmp_probe(struct udevice *dev)
> @@ -54,8 +318,7 @@ static int soc_xilinx_zynqmp_probe(struct udevice *dev)
> 
>          priv->family = zynqmp_family;
> 
> -       if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3 ||
> -           !IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
> +       if (!IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
>                  ret = zynqmp_mmio_read(ZYNQMP_PS_VERSION, &ret_payload[2]);
>          else
>                  ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,

I was looking at code and this change is very interesting.
I think that it can be just
ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,...
because message can be sent via IPI directly.

That means that this should be completely separate patch.


> @@ -65,6 +328,26 @@ static int soc_xilinx_zynqmp_probe(struct udevice *dev)
> 
>          priv->revision = ret_payload[2] & ZYNQMP_PS_VER_MASK;
> 
> +       if (IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE)) {

When above change is there you should be able to remove this checking because
you should get all payloads back in proper shape.



> +               /*
> +                * Firmware returns:
> +                * payload[0][31:0] = status of the operation
> +                * payload[1] = IDCODE
> +                * payload[2][19:0] = Version
> +                * payload[2][28:20] = EXTENDED_IDCODE
> +                * payload[2][29] = PL_INIT
> +                */
> +               u32 idcode = ret_payload[1];
> +               u32 idcode2 = ret_payload[2] >>
> +                                  ZYNQMP_CSU_VERSION_EMPTY_SHIFT;
> +               dev_dbg(dev, "IDCODE: 0x%0x, IDCODE2: 0x%0x\n", idcode,
> +                       idcode2);
> +
> +               ret = soc_xilinx_zynqmp_detect_machine(dev, idcode, idcode2);
> +               if (ret)
> +                       return ret;
> +       }
> +
>          return 0;
>   }
> 
> --
> 2.30.2
> 

Thanks,
Michal

Re: [PATCH 04/10] xilinx: zynqmp: Use soc machine function to get silicon idcode name

[Michal Simek]

On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
> [CAUTION: External Email]
> 
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> Use the soc_get_machine function of the soc uclass to get silicon idcode
> name for the fpga init.
> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> ---
> 
>   board/xilinx/zynqmp/zynqmp.c | 287 ++---------------------------------
>   1 file changed, 15 insertions(+), 272 deletions(-)
> 
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index e311aa772c..06f6dbab18 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -19,6 +19,7 @@
>   #include <sata.h>
>   #include <ahci.h>
>   #include <scsi.h>
> +#include <soc.h>
>   #include <malloc.h>
>   #include <memalign.h>
>   #include <wdt.h>
> @@ -44,278 +45,10 @@
> 
>   #include "pm_cfg_obj.h"
> 
> -#define ZYNQMP_VERSION_SIZE    7
> -#define EFUSE_VCU_DIS_MASK     0x100
> -#define EFUSE_VCU_DIS_SHIFT    8
> -#define EFUSE_GPU_DIS_MASK     0x20
> -#define EFUSE_GPU_DIS_SHIFT    5
> -#define IDCODE2_PL_INIT_MASK   0x200
> -#define IDCODE2_PL_INIT_SHIFT  9
> -
>   DECLARE_GLOBAL_DATA_PTR;
> 
>   #if CONFIG_IS_ENABLED(FPGA) && defined(CONFIG_FPGA_ZYNQMPPL)
>   static xilinx_desc zynqmppl = XILINX_ZYNQMP_DESC;
> -
> -enum {
> -       ZYNQMP_VARIANT_EG = BIT(0U),
> -       ZYNQMP_VARIANT_EV = BIT(1U),
> -       ZYNQMP_VARIANT_CG = BIT(2U),
> -       ZYNQMP_VARIANT_DR = BIT(3U),
> -};
> -
> -static const struct {
> -       u32 id;
> -       u8 device;
> -       u8 variants;
> -} zynqmp_devices[] = {
> -       {
> -               .id = 0x04688093,
> -               .device = 1,
> -               .variants = ZYNQMP_VARIANT_EG,
> -       },
> -       {
> -               .id = 0x04711093,
> -               .device = 2,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> -       },
> -       {
> -               .id = 0x04710093,
> -               .device = 3,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> -       },
> -       {
> -               .id = 0x04721093,
> -               .device = 4,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
> -                       ZYNQMP_VARIANT_EV,
> -       },
> -       {
> -               .id = 0x04720093,
> -               .device = 5,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
> -                       ZYNQMP_VARIANT_EV,
> -       },
> -       {
> -               .id = 0x04739093,
> -               .device = 6,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> -       },
> -       {
> -               .id = 0x04730093,
> -               .device = 7,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG |
> -                       ZYNQMP_VARIANT_EV,
> -       },
> -       {
> -               .id = 0x04738093,
> -               .device = 9,
> -               .variants = ZYNQMP_VARIANT_EG | ZYNQMP_VARIANT_CG,
> -       },
> -       {
> -               .id = 0x04740093,
> -               .device = 11,
> -               .variants = ZYNQMP_VARIANT_EG,
> -       },
> -       {
> -               .id = 0x04750093,
> -               .device = 15,
> -               .variants = ZYNQMP_VARIANT_EG,
> -       },
> -       {
> -               .id = 0x04759093,
> -               .device = 17,
> -               .variants = ZYNQMP_VARIANT_EG,
> -       },
> -       {
> -               .id = 0x04758093,
> -               .device = 19,
> -               .variants = ZYNQMP_VARIANT_EG,
> -       },
> -       {
> -               .id = 0x047E1093,
> -               .device = 21,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047E3093,
> -               .device = 23,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047E5093,
> -               .device = 25,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047E4093,
> -               .device = 27,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047E0093,
> -               .device = 28,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047E2093,
> -               .device = 29,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047E6093,
> -               .device = 39,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047FD093,
> -               .device = 43,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047F8093,
> -               .device = 46,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047FF093,
> -               .device = 47,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047FB093,
> -               .device = 48,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x047FE093,
> -               .device = 49,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -       {
> -               .id = 0x046d0093,
> -               .device = 67,
> -               .variants = ZYNQMP_VARIANT_DR,
> -       },
> -};
> -
> -static const struct {
> -       u32 id;
> -       char *name;
> -} zynqmp_svd_devices[] = {
> -       {
> -               .id = 0x04714093,
> -               .name = "xck24"
> -       },
> -       {
> -               .id = 0x04724093,
> -               .name = "xck26",
> -       },
> -};
> -
> -static char *zynqmp_detect_svd_name(u32 idcode)
> -{
> -       u32 i;
> -
> -       for (i = 0; i < ARRAY_SIZE(zynqmp_svd_devices); i++) {
> -               if (zynqmp_svd_devices[i].id == (idcode & 0x0FFFFFFF))
> -                       return zynqmp_svd_devices[i].name;
> -       }
> -
> -       return "unknown";
> -}
> -
> -static char *zynqmp_get_silicon_idcode_name(void)
> -{
> -       u32 i;
> -       u32 idcode, idcode2;
> -       char name[ZYNQMP_VERSION_SIZE];
> -       u32 ret_payload[PAYLOAD_ARG_CNT];
> -       int ret;
> -
> -       ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0, ret_payload);
> -       if (ret) {
> -               debug("%s: Getting chipid failed\n", __func__);
> -               return "unknown";
> -       }
> -
> -       /*
> -        * Firmware returns:
> -        * payload[0][31:0]  = status of the operation
> -        * payload[1]] = IDCODE
> -        * payload[2][19:0]  = Version
> -        * payload[2][28:20] = EXTENDED_IDCODE
> -        * payload[2][29] = PL_INIT
> -        */
> -
> -       idcode  = ret_payload[1];
> -       idcode2 = ret_payload[2] >> ZYNQMP_CSU_VERSION_EMPTY_SHIFT;
> -       debug("%s, IDCODE: 0x%0x, IDCODE2: 0x%0x\r\n", __func__, idcode,
> -             idcode2);
> -
> -       for (i = 0; i < ARRAY_SIZE(zynqmp_devices); i++) {
> -               if (zynqmp_devices[i].id == (idcode & 0x0FFFFFFF))
> -                       break;
> -       }
> -
> -       if (i >= ARRAY_SIZE(zynqmp_devices))
> -               return zynqmp_detect_svd_name(idcode);
> -
> -       /* Add device prefix to the name */
> -       ret = snprintf(name, ZYNQMP_VERSION_SIZE, "zu%d",
> -                      zynqmp_devices[i].device);
> -       if (ret < 0)
> -               return "unknown";
> -
> -       if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_EV) {
> -               /* Devices with EV variant might be EG/CG/EV family */
> -               if (idcode2 & IDCODE2_PL_INIT_MASK) {
> -                       u32 family = ((idcode2 & EFUSE_VCU_DIS_MASK) >>
> -                                     EFUSE_VCU_DIS_SHIFT) << 1 |
> -                                    ((idcode2 & EFUSE_GPU_DIS_MASK) >>
> -                                     EFUSE_GPU_DIS_SHIFT);
> -
> -                       /*
> -                        * Get family name based on extended idcode values as
> -                        * determined on UG1087, EXTENDED_IDCODE register
> -                        * description
> -                        */
> -                       switch (family) {
> -                       case 0x00:
> -                               strncat(name, "ev", 2);
> -                               break;
> -                       case 0x10:
> -                               strncat(name, "eg", 2);
> -                               break;
> -                       case 0x11:
> -                               strncat(name, "cg", 2);
> -                               break;
> -                       default:
> -                               /* Do not append family name*/
> -                               break;
> -                       }
> -               } else {
> -                       /*
> -                        * When PL powered down the VCU Disable efuse cannot be
> -                        * read. So, ignore the bit and just findout if it is CG
> -                        * or EG/EV variant.
> -                        */
> -                       strncat(name, (idcode2 & EFUSE_GPU_DIS_MASK) ? "cg" :
> -                               "e", 2);
> -               }
> -       } else if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_CG) {
> -               /* Devices with CG variant might be EG or CG family */
> -               strncat(name, (idcode2 & EFUSE_GPU_DIS_MASK) ? "cg" : "eg", 2);
> -       } else if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_EG) {
> -               strncat(name, "eg", 2);
> -       } else if (zynqmp_devices[i].variants & ZYNQMP_VARIANT_DR) {
> -               strncat(name, "dr", 2);
> -       } else {
> -               debug("Variant not identified\n");
> -       }
> -
> -       return strdup(name);
> -}
>   #endif
> 
>   int __maybe_unused psu_uboot_init(void)
> @@ -406,6 +139,11 @@ static void print_secure_boot(void)
> 
>   int board_init(void)
>   {
> +#if CONFIG_IS_ENABLED(FPGA) && defined(CONFIG_FPGA_ZYNQMPPL)
> +       struct udevice *soc;
> +       char name[SOC_MAX_STR_SIZE];
> +       int ret;
> +#endif
>   #if defined(CONFIG_ZYNQMP_FIRMWARE)
>          struct udevice *dev;
> 
> @@ -432,10 +170,15 @@ int board_init(void)
>          printf("EL Level:\tEL%d\n", current_el());
> 
>   #if CONFIG_IS_ENABLED(FPGA) && defined(CONFIG_FPGA_ZYNQMPPL)
> -       zynqmppl.name = zynqmp_get_silicon_idcode_name();
> -       printf("Chip ID:\t%s\n", zynqmppl.name);
> -       fpga_init();
> -       fpga_add(fpga_xilinx, &zynqmppl);
> +       ret = soc_get(&soc);
> +       if (!ret) {
> +               ret = soc_get_machine(soc, name, sizeof(name));
> +               if (ret >= 0) {
> +                       zynqmppl.name = strdup(name);
> +                       fpga_init();
> +                       fpga_add(fpga_xilinx, &zynqmppl);
> +               }
> +       }
>   #endif
> 
>          /* display secure boot information */
> --
> 2.30.2
> 

As I said please merge it with 3/10.

M

Re: [PATCH 00/10] xilinx: zynqmp: Support foreign vendor boards

[Michal Simek]

Hi,

On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
> [CAUTION: External Email]
> 
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> 
> At the moment the xilinx zynqmp soc is only supported by xilinx vendor
> boards. Rework the xilinx zynqmp board code to support reuse by foreign
> vendor boards.
> 
> 
> Stefan Herbrechtsmeier (10):
>    firmware: firmware-zynqmp: Check if rx channel dev pointer is valid
>    firmware: firmware-zynqmp: Probe driver before use
>    soc: xilinx: zynqmp: Add machine identification support
>    xilinx: zynqmp: Use soc machine function to get silicon idcode name
>    xilinx: cpuinfo: Print soc machine
>    xilinx: common: Separate display cpu info function
>    xilinx: zynqmp: make spi flash support optional
>    tools: zynqmp_psu_init_minimize: Remove low level uart settings
>    tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward
>      declaration
>    xilinx: zynqmp: Support vendor specific board_init
> 
>   board/xilinx/common/Makefile       |   3 +
>   board/xilinx/common/board.c        |  30 +--
>   board/xilinx/common/board.h        |   2 +
>   board/xilinx/common/cpu-info.c     |  35 ++++
>   board/xilinx/zynqmp/zynqmp.c       | 291 ++---------------------------
>   drivers/firmware/firmware-zynqmp.c |  20 +-
>   drivers/soc/soc_xilinx_zynqmp.c    | 289 +++++++++++++++++++++++++++-
>   tools/zynqmp_psu_init_minimize.sh  |  22 +++
>   8 files changed, 390 insertions(+), 302 deletions(-)
>   create mode 100644 board/xilinx/common/cpu-info.c
> 
> --
> 2.30.2
> 

The rest of patches are fine.

Thanks,
Michal

Re: [PATCH 02/10] firmware: firmware-zynqmp: Probe driver before use

[Stefan Herbrechtsmeier]

Am 16.06.2022 um 16:22 schrieb Michal Simek:
> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>> [CAUTION: External Email]
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> Probe the driver before use to ensure that the global data are valid.
> 
> It is not clear what the issue is. And this function is called in SPL or 
> EL3 and likely multiple times.
> Can you please clarify?

The driver only works if it is used after u-boot,dm-pre-reloc. This 
change is needed to support a usage by other drivers like 
xlnx,zynqmp-firmware.

Regards
   Stefan

Re: [PATCH 10/10] xilinx: zynqmp: Support vendor specific board_init

[Stefan Herbrechtsmeier]

Am 16.06.2022 um 17:12 schrieb Michal Simek:
> 
> 
> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>> [CAUTION: External Email]
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> Add a board_init_xilinx function to allow foreign vendors to reuse the
>> xilinx zynqmp board code and add addition code to the board_init
>> function.
> 
> Do you plan to add support for that board who will be using it?

Yes. You could drop the patch for now but it will be good to know if 
this is the right direction to reuse common code from the xilinx board 
files.

>> Signed-off-by: Stefan Herbrechtsmeier 
>> <stefan.herbrechtsmeier@weidmueller.com>
>>
>> ---
>>
>>   board/xilinx/common/board.c  | 5 +++++
>>   board/xilinx/common/board.h  | 2 ++
>>   board/xilinx/zynqmp/zynqmp.c | 2 +-
>>   3 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
>> index 5f2afb9def..643959bee7 100644
>> --- a/board/xilinx/common/board.c
>> +++ b/board/xilinx/common/board.c
>> @@ -401,6 +401,11 @@ void *board_fdt_blob_setup(int *err)
>>   }
>>   #endif
>>
>> +int board_init_xilinx(void)
>> +{
>> +       return 0;
>> +}
> 
> This file is used by zynq/zynqmp/versal platforms. I expect there is 
> going to be any message that this function is unused on zynq and versal.

You are right. Should we add it to all platforms?

Is this the correct direction or should we split 
board/xilinx/zynqmp/zynqmp.c into common soc and specific board code?

Regards
   Stefan

Re: [PATCH 02/10] firmware: firmware-zynqmp: Probe driver before use

[Michal Simek]

Hi,

On 6/20/22 08:40, Stefan Herbrechtsmeier wrote:
> Am 16.06.2022 um 16:22 schrieb Michal Simek:
>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>> [CAUTION: External Email]
>>>
>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>
>>> Probe the driver before use to ensure that the global data are valid.
>>
>> It is not clear what the issue is. And this function is called in SPL or EL3 
>> and likely multiple times.
>> Can you please clarify?
> 
> The driver only works if it is used after u-boot,dm-pre-reloc. This change is 
> needed to support a usage by other drivers like xlnx,zynqmp-firmware.

Can you please describe the case, execution path where it is used before?

Also that calling multiple times is just adding overhead for boot up time. We 
should try to avoid it as much as possible.

M

Re: [PATCH 10/10] xilinx: zynqmp: Support vendor specific board_init

[Michal Simek]

On 6/20/22 08:48, Stefan Herbrechtsmeier wrote:
> Am 16.06.2022 um 17:12 schrieb Michal Simek:
>>
>>
>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>> [CAUTION: External Email]
>>>
>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>
>>> Add a board_init_xilinx function to allow foreign vendors to reuse the
>>> xilinx zynqmp board code and add addition code to the board_init
>>> function.
>>
>> Do you plan to add support for that board who will be using it?
> 
> Yes. You could drop the patch for now but it will be good to know if this is the 
> right direction to reuse common code from the xilinx board files.

Hard to say without seeing what others need to do at this stage.
I am not saying it is bad directly but question I have is what you have there 
that you need it. Maybe this is something what others want to use too and we can 
integrated it directly to common xilinx files.

Thanks,
Michal

Re: [PATCH 09/10] tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward declaration

[Stefan Herbrechtsmeier]

Am 16.06.2022 um 17:13 schrieb Michal Simek:
> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>> [CAUTION: External Email]
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> A forward declaration for the serdes_illcalib function.
>>
>> Signed-off-by: Stefan Herbrechtsmeier 
>> <stefan.herbrechtsmeier@weidmueller.com>
>> ---
>>
>>   tools/zynqmp_psu_init_minimize.sh | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/zynqmp_psu_init_minimize.sh 
>> b/tools/zynqmp_psu_init_minimize.sh
>> index 31fbeac327..8411065e13 100755
>> --- a/tools/zynqmp_psu_init_minimize.sh
>> +++ b/tools/zynqmp_psu_init_minimize.sh
>> @@ -108,6 +108,11 @@ cat << EOF >${TMP}
>>   #include <asm/arch/psu_init_gpl.h>
>>   #include <xil_io.h>
>>
>> +static int serdes_illcalib(u32 lane3_protocol, u32 lane3_rate,
>> +                          u32 lane2_protocol, u32 lane2_rate,
>> +                          u32 lane1_protocol, u32 lane1_rate,
>> +                          u32 lane0_protocol, u32 lane0_rate);
>> +
> 
> Better to resort that functions to avoid these additional lines.

Do you propose to move the serdes_illcalib and serdes_illcalib_pcie_gen1 
functions via sed?

The psu_init_gpl.c for e-a2197-00-revA, zcu208-revA and zcu216-revA 
already use a forward declaration. Would it be okay to add the forward 
declaration if the function is used inside the source file?

Regards
   Stefan

Re: [PATCH 09/10] tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward declaration

[Michal Simek]

On 6/20/22 09:07, Stefan Herbrechtsmeier wrote:
> Am 16.06.2022 um 17:13 schrieb Michal Simek:
>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>> [CAUTION: External Email]
>>>
>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>
>>> A forward declaration for the serdes_illcalib function.
>>>
>>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>> ---
>>>
>>>   tools/zynqmp_psu_init_minimize.sh | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/tools/zynqmp_psu_init_minimize.sh 
>>> b/tools/zynqmp_psu_init_minimize.sh
>>> index 31fbeac327..8411065e13 100755
>>> --- a/tools/zynqmp_psu_init_minimize.sh
>>> +++ b/tools/zynqmp_psu_init_minimize.sh
>>> @@ -108,6 +108,11 @@ cat << EOF >${TMP}
>>>   #include <asm/arch/psu_init_gpl.h>
>>>   #include <xil_io.h>
>>>
>>> +static int serdes_illcalib(u32 lane3_protocol, u32 lane3_rate,
>>> +                          u32 lane2_protocol, u32 lane2_rate,
>>> +                          u32 lane1_protocol, u32 lane1_rate,
>>> +                          u32 lane0_protocol, u32 lane0_rate);
>>> +
>>
>> Better to resort that functions to avoid these additional lines.
> 
> Do you propose to move the serdes_illcalib and serdes_illcalib_pcie_gen1 
> functions via sed?

swapping that functions should be enough. Whatever tool which does this job is fine.

> 
> The psu_init_gpl.c for e-a2197-00-revA, zcu208-revA and zcu216-revA already use 
> a forward declaration. Would it be okay to add the forward declaration if the 
> function is used inside the source file?

Better to just fix this. Can you please send the patch?

Thanks,
Michal

Re: [PATCH 03/10] soc: xilinx: zynqmp: Add machine identification support

[Stefan Herbrechtsmeier]

Am 17.06.2022 um 12:41 schrieb Michal Simek:
> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>> [CAUTION: External Email]
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> Add machine identification support based on the
>> zynqmp_get_silicon_idcode_name function in board/xilinx/zynqmp/zynqmp.c.
>>
>> Signed-off-by: Stefan Herbrechtsmeier 
>> <stefan.herbrechtsmeier@weidmueller.com>
>> ---
>>
>>   drivers/soc/soc_xilinx_zynqmp.c | 289 +++++++++++++++++++++++++++++++-
>>   1 file changed, 286 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/soc_xilinx_zynqmp.c 
>> b/drivers/soc/soc_xilinx_zynqmp.c
>> index a71115b17c..45592ed534 100644
>> --- a/drivers/soc/soc_xilinx_zynqmp.c
>> +++ b/drivers/soc/soc_xilinx_zynqmp.c

>> @@ -54,8 +318,7 @@ static int soc_xilinx_zynqmp_probe(struct udevice 
>> *dev)
>>
>>          priv->family = zynqmp_family;
>>
>> -       if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3 ||
>> -           !IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
>> +       if (!IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
>>                  ret = zynqmp_mmio_read(ZYNQMP_PS_VERSION, 
>> &ret_payload[2]);
>>          else
>>                  ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,
> 
> I was looking at code and this change is very interesting.
> I think that it can be just
> ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,...
> because message can be sent via IPI directly.

Without zynqmp_mmio_read this driver depends on the 
CONFIG_ZYNQMP_FIRMWARE driver.

> That means that this should be completely separate patch.

I will add a separate patch.

> 
> 
>> @@ -65,6 +328,26 @@ static int soc_xilinx_zynqmp_probe(struct udevice 
>> *dev)
>>
>>          priv->revision = ret_payload[2] & ZYNQMP_PS_VER_MASK;
>>
>> +       if (IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE)) {
> 
> When above change is there you should be able to remove this checking 
> because
> you should get all payloads back in proper shape.
> 
> 
> 
>> +               /*
>> +                * Firmware returns:
>> +                * payload[0][31:0] = status of the operation
>> +                * payload[1] = IDCODE
>> +                * payload[2][19:0] = Version
>> +                * payload[2][28:20] = EXTENDED_IDCODE
>> +                * payload[2][29] = PL_INIT
>> +                */
>> +               u32 idcode = ret_payload[1];
>> +               u32 idcode2 = ret_payload[2] >>
>> +                                  ZYNQMP_CSU_VERSION_EMPTY_SHIFT;
>> +               dev_dbg(dev, "IDCODE: 0x%0x, IDCODE2: 0x%0x\n", idcode,
>> +                       idcode2);
>> +
>> +               ret = soc_xilinx_zynqmp_detect_machine(dev, idcode, 
>> idcode2);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>>          return 0;
>>   }
>>
>> -- 
>> 2.30.2
>>

Regards
   Stefan

Re: [PATCH 02/10] firmware: firmware-zynqmp: Probe driver before use

[Stefan Herbrechtsmeier]

Am 20.06.2022 um 08:51 schrieb Michal Simek:
> Hi,
> 
> On 6/20/22 08:40, Stefan Herbrechtsmeier wrote:
>> Am 16.06.2022 um 16:22 schrieb Michal Simek:
>>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>>> [CAUTION: External Email]
>>>>
>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>
>>>> Probe the driver before use to ensure that the global data are valid.
>>>
>>> It is not clear what the issue is. And this function is called in SPL 
>>> or EL3 and likely multiple times.
>>> Can you please clarify?
>>
>> The driver only works if it is used after u-boot,dm-pre-reloc. This 
>> change is needed to support a usage by other drivers like 
>> xlnx,zynqmp-firmware.
> 
> Can you please describe the case, execution path where it is used before?

I released the problem with the soc_xilinx_zynqmp driver. The driver was 
probed before the firmware-zynqmp driver.

At the moment every user of xilinx_pm_request function have to ensure 
that the firmware-zynqmp driver was probed.

> 
> Also that calling multiple times is just adding overhead for boot up 
> time. We should try to avoid it as much as possible.

Should I zero zynqmp_power and only probe if it is zero?

The solution is based on the psci driver.

Regards
   Stefan

Re: [PATCH 10/10] xilinx: zynqmp: Support vendor specific board_init

[Stefan Herbrechtsmeier]

Am 20.06.2022 um 08:53 schrieb Michal Simek:
> On 6/20/22 08:48, Stefan Herbrechtsmeier wrote:
>> Am 16.06.2022 um 17:12 schrieb Michal Simek:
>>>
>>>
>>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>>> [CAUTION: External Email]
>>>>
>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>
>>>> Add a board_init_xilinx function to allow foreign vendors to reuse the
>>>> xilinx zynqmp board code and add addition code to the board_init
>>>> function.
>>>
>>> Do you plan to add support for that board who will be using it?
>>
>> Yes. You could drop the patch for now but it will be good to know if 
>> this is the right direction to reuse common code from the xilinx board 
>> files.
> 
> Hard to say without seeing what others need to do at this stage.
> I am not saying it is bad directly but question I have is what you have 
> there that you need it. Maybe this is something what others want to use 
> too and we can integrated it directly to common xilinx files.

I will drop it for now. Let's discuss it together with code that use it.

Regards
   Stefan

Re: [PATCH 09/10] tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward declaration

[Stefan Herbrechtsmeier]

Am 20.06.2022 um 09:18 schrieb Michal Simek:
> On 6/20/22 09:07, Stefan Herbrechtsmeier wrote:
>> Am 16.06.2022 um 17:13 schrieb Michal Simek:
>>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>>> [CAUTION: External Email]
>>>>
>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>
>>>> A forward declaration for the serdes_illcalib function.
>>>>
>>>> Signed-off-by: Stefan Herbrechtsmeier 
>>>> <stefan.herbrechtsmeier@weidmueller.com>
>>>> ---
>>>>
>>>>   tools/zynqmp_psu_init_minimize.sh | 5 +++++
>>>>   1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/tools/zynqmp_psu_init_minimize.sh 
>>>> b/tools/zynqmp_psu_init_minimize.sh
>>>> index 31fbeac327..8411065e13 100755
>>>> --- a/tools/zynqmp_psu_init_minimize.sh
>>>> +++ b/tools/zynqmp_psu_init_minimize.sh
>>>> @@ -108,6 +108,11 @@ cat << EOF >${TMP}
>>>>   #include <asm/arch/psu_init_gpl.h>
>>>>   #include <xil_io.h>
>>>>
>>>> +static int serdes_illcalib(u32 lane3_protocol, u32 lane3_rate,
>>>> +                          u32 lane2_protocol, u32 lane2_rate,
>>>> +                          u32 lane1_protocol, u32 lane1_rate,
>>>> +                          u32 lane0_protocol, u32 lane0_rate);
>>>> +
>>>
>>> Better to resort that functions to avoid these additional lines.
>>
>> Do you propose to move the serdes_illcalib and 
>> serdes_illcalib_pcie_gen1 functions via sed?
> 
> swapping that functions should be enough. Whatever tool which does this 
> job is fine.

Does the swap need to check for the serdes_illcalib function inside the 
psu_serdes_init_data function or is a general reorder okay?

>> The psu_init_gpl.c for e-a2197-00-revA, zcu208-revA and zcu216-revA 
>> already use a forward declaration. Would it be okay to add the forward 
>> declaration if the function is used inside the source file?
> 
> Better to just fix this. Can you please send the patch?

The e-a2197-00-revA/psu_init_gpl.c file has an additional forward 
declaration of the dpll_prog function.

Regards
   Stefan

Re: [PATCH 09/10] tools: zynqmp_psu_init_minimize: Add serdes_illcalib forward declaration

[Michal Simek]

On 6/20/22 15:38, Stefan Herbrechtsmeier wrote:
> Am 20.06.2022 um 09:18 schrieb Michal Simek:
>> On 6/20/22 09:07, Stefan Herbrechtsmeier wrote:
>>> Am 16.06.2022 um 17:13 schrieb Michal Simek:
>>>> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>>>>> [CAUTION: External Email]
>>>>>
>>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>>
>>>>> A forward declaration for the serdes_illcalib function.
>>>>>
>>>>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>> ---
>>>>>
>>>>>   tools/zynqmp_psu_init_minimize.sh | 5 +++++
>>>>>   1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/tools/zynqmp_psu_init_minimize.sh 
>>>>> b/tools/zynqmp_psu_init_minimize.sh
>>>>> index 31fbeac327..8411065e13 100755
>>>>> --- a/tools/zynqmp_psu_init_minimize.sh
>>>>> +++ b/tools/zynqmp_psu_init_minimize.sh
>>>>> @@ -108,6 +108,11 @@ cat << EOF >${TMP}
>>>>>   #include <asm/arch/psu_init_gpl.h>
>>>>>   #include <xil_io.h>
>>>>>
>>>>> +static int serdes_illcalib(u32 lane3_protocol, u32 lane3_rate,
>>>>> +                          u32 lane2_protocol, u32 lane2_rate,
>>>>> +                          u32 lane1_protocol, u32 lane1_rate,
>>>>> +                          u32 lane0_protocol, u32 lane0_rate);
>>>>> +
>>>>
>>>> Better to resort that functions to avoid these additional lines.
>>>
>>> Do you propose to move the serdes_illcalib and serdes_illcalib_pcie_gen1 
>>> functions via sed?
>>
>> swapping that functions should be enough. Whatever tool which does this job is 
>> fine.
> 
> Does the swap need to check for the serdes_illcalib function inside the 
> psu_serdes_init_data function or is a general reorder okay?

Everybody should call checkpatch and compile it. Tool is prepares that files but 
at the end it is up to user to fix the rest.
Taht's why general reorder is fine for me.

> 
>>> The psu_init_gpl.c for e-a2197-00-revA, zcu208-revA and zcu216-revA already 
>>> use a forward declaration. Would it be okay to add the forward declaration if 
>>> the function is used inside the source file?
>>
>> Better to just fix this. Can you please send the patch?
> 
> The e-a2197-00-revA/psu_init_gpl.c file has an additional forward declaration of 
> the dpll_prog function.

Please fix all of these.

Thanks,
Michal