[PATCH 1/3] imx: power-domain: Descend into pgc subnode if present

[PATCH 1/3] imx: power-domain: Descend into pgc subnode if present

[Marek Vasut]

In case the power domain node structure is gpc@303a0000/pgc/power-domain@N,
do not bind power domain driver to the 'pgc' node, but rather descend into
it and only bind power domain drivers to power-domain@N subnodes. This way
we do not waste one useless driver instance associated with 'pgc' node.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/power/domain/imx8m-power-domain.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
index 6082ee6ff8c..ac7411f8327 100644
--- a/drivers/power/domain/imx8m-power-domain.c
+++ b/drivers/power/domain/imx8m-power-domain.c
@@ -73,6 +73,12 @@ static int imx8m_power_domain_bind(struct udevice *dev)
 		/* Bind the subnode to this driver */
 		name = fdt_get_name(gd->fdt_blob, offset, NULL);
 
+		/* Descend into 'pgc' subnode */
+		if (!strstr(name, "power-domain")) {
+			offset = fdt_first_subnode(gd->fdt_blob, offset);
+			name = fdt_get_name(gd->fdt_blob, offset, NULL);
+		}
+
 		ret = device_bind_with_driver_data(dev, dev->driver, name,
 						   dev->driver_data,
 						   offset_to_ofnode(offset),
-- 
2.35.1

[PATCH 2/3] imx: power-domain: Inline arch-imx8m/power-domain.h

[Marek Vasut]

The arch/arm/include/asm/arch-imx8m/power-domain.h is not included
anywhere except in drivers/power/domain/imx8m-power-domain.c, just
inline the content and drop the header. No functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 arch/arm/include/asm/arch-imx8m/power-domain.h | 15 ---------------
 drivers/power/domain/imx8m-power-domain.c      |  7 ++++++-
 2 files changed, 6 insertions(+), 16 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-imx8m/power-domain.h

diff --git a/arch/arm/include/asm/arch-imx8m/power-domain.h b/arch/arm/include/asm/arch-imx8m/power-domain.h
deleted file mode 100644
index 7a833e564b5..00000000000
--- a/arch/arm/include/asm/arch-imx8m/power-domain.h
+++ /dev/null
@@ -1,15 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright 2017 NXP
- */
-
-#ifndef _ASM_ARCH_IMX8M_POWER_DOMAIN_H
-#define _ASM_ARCH_IMX8M_POWER_DOMAIN_H
-
-struct imx8m_power_domain_plat {
-	int resource_id;
-	int has_pd;
-	struct power_domain pd;
-};
-
-#endif
diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
index ac7411f8327..c32dbcc31ae 100644
--- a/drivers/power/domain/imx8m-power-domain.c
+++ b/drivers/power/domain/imx8m-power-domain.c
@@ -9,7 +9,6 @@
 #include <power-domain-uclass.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
-#include <asm/arch/power-domain.h>
 #include <asm/mach-imx/sys_proto.h>
 #include <dm/device-internal.h>
 #include <dm/device.h>
@@ -18,6 +17,12 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+struct imx8m_power_domain_plat {
+	int resource_id;
+	int has_pd;
+	struct power_domain pd;
+};
+
 static int imx8m_power_domain_on(struct power_domain *power_domain)
 {
 	struct udevice *dev = power_domain->dev;
-- 
2.35.1

[PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
the driver based on Linux GPCv2 driver to directly control the GPCv2
block instead of using SMCCC calls. This way, U-Boot can operate the
i.MX8M power domains without depending on anything else.

This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
domains are not supported to save space, since they are not useful in
the bootloader. The only domains kept are ones for HSIO, PCIe, USB.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
NOTE: So far this is tested on MX8MM and MX8MN. MX8MQ is not tested.
---
 drivers/power/domain/Kconfig              |   1 +
 drivers/power/domain/imx8m-power-domain.c | 379 ++++++++++++++++++++--
 2 files changed, 361 insertions(+), 19 deletions(-)

diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
index 93d2599d83c..04fc0054323 100644
--- a/drivers/power/domain/Kconfig
+++ b/drivers/power/domain/Kconfig
@@ -35,6 +35,7 @@ config IMX8_POWER_DOMAIN
 config IMX8M_POWER_DOMAIN
 	bool "Enable i.MX8M power domain driver"
 	depends on POWER_DOMAIN && ARCH_IMX8M
+	select CLK
 	help
 	  Enable support for manipulating NXP i.MX8M on-SoC power domains via
 	  requests to the ATF.
diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
index c32dbcc31ae..e2e41cf5fee 100644
--- a/drivers/power/domain/imx8m-power-domain.c
+++ b/drivers/power/domain/imx8m-power-domain.c
@@ -4,6 +4,7 @@
  */
 
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
 #include <malloc.h>
 #include <power-domain-uclass.h>
@@ -12,52 +13,361 @@
 #include <asm/mach-imx/sys_proto.h>
 #include <dm/device-internal.h>
 #include <dm/device.h>
+#include <dm/device_compat.h>
 #include <imx_sip.h>
-#include <linux/arm-smccc.h>
+#include <linux/bitmap.h>
+#include <wait_bit.h>
+
+#include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/power/imx8mn-power.h>
+#include <dt-bindings/power/imx8mq-power.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#define GPC_PGC_CPU_MAPPING			0x0ec
+
+#define IMX8M_PCIE2_A53_DOMAIN			BIT(15)
+#define IMX8M_OTG2_A53_DOMAIN			BIT(5)
+#define IMX8M_OTG1_A53_DOMAIN			BIT(4)
+#define IMX8M_PCIE1_A53_DOMAIN			BIT(3)
+
+#define IMX8MM_OTG2_A53_DOMAIN			BIT(5)
+#define IMX8MM_OTG1_A53_DOMAIN			BIT(4)
+#define IMX8MM_PCIE_A53_DOMAIN			BIT(3)
+
+#define IMX8MN_OTG1_A53_DOMAIN			BIT(4)
+#define IMX8MN_MIPI_A53_DOMAIN			BIT(2)
+
+#define GPC_PU_PGC_SW_PUP_REQ			0x0f8
+#define GPC_PU_PGC_SW_PDN_REQ			0x104
+
+#define IMX8M_PCIE2_SW_Pxx_REQ			BIT(13)
+#define IMX8M_OTG2_SW_Pxx_REQ			BIT(3)
+#define IMX8M_OTG1_SW_Pxx_REQ			BIT(2)
+#define IMX8M_PCIE1_SW_Pxx_REQ			BIT(1)
+
+#define IMX8MM_OTG2_SW_Pxx_REQ			BIT(3)
+#define IMX8MM_OTG1_SW_Pxx_REQ			BIT(2)
+#define IMX8MM_PCIE_SW_Pxx_REQ			BIT(1)
+
+#define IMX8MN_OTG1_SW_Pxx_REQ			BIT(2)
+#define IMX8MN_MIPI_SW_Pxx_REQ			BIT(0)
+
+#define GPC_M4_PU_PDN_FLG			0x1bc
+
+#define GPC_PU_PWRHSK				0x1fc
+
+#define IMX8MM_HSIO_HSK_PWRDNACKN		(BIT(23) | BIT(24))
+#define IMX8MM_HSIO_HSK_PWRDNREQN		(BIT(5) | BIT(6))
+
+#define IMX8MN_HSIO_HSK_PWRDNACKN		BIT(23)
+#define IMX8MN_HSIO_HSK_PWRDNREQN		BIT(5)
+
+/*
+ * The PGC offset values in Reference Manual
+ * (Rev. 1, 01/2018 and the older ones) GPC chapter's
+ * GPC_PGC memory map are incorrect, below offset
+ * values are from design RTL.
+ */
+#define IMX8M_PGC_PCIE1			17
+#define IMX8M_PGC_OTG1			18
+#define IMX8M_PGC_OTG2			19
+#define IMX8M_PGC_PCIE2			29
+
+#define IMX8MM_PGC_PCIE			17
+#define IMX8MM_PGC_OTG1			18
+#define IMX8MM_PGC_OTG2			19
+
+#define IMX8MN_PGC_OTG1			18
+
+#define GPC_PGC_CTRL(n)			(0x800 + (n) * 0x40)
+#define GPC_PGC_SR(n)			(GPC_PGC_CTRL(n) + 0xc)
+
+#define GPC_PGC_CTRL_PCR		BIT(0)
+
+struct imx_pgc_regs {
+	u16 map;
+	u16 pup;
+	u16 pdn;
+	u16 hsk;
+};
+
+struct imx_pgc_domain {
+	unsigned long pgc;
+
+	const struct {
+		u32 pxx;
+		u32 map;
+		u32 hskreq;
+		u32 hskack;
+	} bits;
+
+	const bool keep_clocks;
+};
+
+struct imx_pgc_domain_data {
+	const struct imx_pgc_domain *domains;
+	size_t domains_num;
+	const struct imx_pgc_regs *pgc_regs;
+};
+
 struct imx8m_power_domain_plat {
+	struct power_domain pd;
+	const struct imx_pgc_domain *domain;
+	const struct imx_pgc_regs *regs;
+	struct clk_bulk clk;
+	void __iomem *base;
 	int resource_id;
 	int has_pd;
-	struct power_domain pd;
 };
 
+#if defined(CONFIG_IMX8MM) || defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MQ)
+static const struct imx_pgc_regs imx7_pgc_regs = {
+	.map = GPC_PGC_CPU_MAPPING,
+	.pup = GPC_PU_PGC_SW_PUP_REQ,
+	.pdn = GPC_PU_PGC_SW_PDN_REQ,
+	.hsk = GPC_PU_PWRHSK,
+};
+#endif
+
+#ifdef CONFIG_IMX8MQ
+static const struct imx_pgc_domain imx8m_pgc_domains[] = {
+	[IMX8M_POWER_DOMAIN_PCIE1] = {
+		.bits  = {
+			.pxx = IMX8M_PCIE1_SW_Pxx_REQ,
+			.map = IMX8M_PCIE1_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8M_PGC_PCIE1),
+	},
+
+	[IMX8M_POWER_DOMAIN_USB_OTG1] = {
+		.bits  = {
+			.pxx = IMX8M_OTG1_SW_Pxx_REQ,
+			.map = IMX8M_OTG1_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8M_PGC_OTG1),
+	},
+
+	[IMX8M_POWER_DOMAIN_USB_OTG2] = {
+		.bits  = {
+			.pxx = IMX8M_OTG2_SW_Pxx_REQ,
+			.map = IMX8M_OTG2_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8M_PGC_OTG2),
+	},
+
+	[IMX8M_POWER_DOMAIN_PCIE2] = {
+		.bits  = {
+			.pxx = IMX8M_PCIE2_SW_Pxx_REQ,
+			.map = IMX8M_PCIE2_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8M_PGC_PCIE2),
+	},
+};
+
+static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
+	.domains = imx8m_pgc_domains,
+	.domains_num = ARRAY_SIZE(imx8m_pgc_domains),
+	.pgc_regs = &imx7_pgc_regs,
+};
+#endif
+
+#ifdef CONFIG_IMX8MM
+static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
+	[IMX8MM_POWER_DOMAIN_HSIOMIX] = {
+		.bits  = {
+			.pxx = 0, /* no power sequence control */
+			.map = 0, /* no power sequence control */
+			.hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
+			.hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
+		},
+		.keep_clocks = true,
+	},
+
+	[IMX8MM_POWER_DOMAIN_PCIE] = {
+		.bits  = {
+			.pxx = IMX8MM_PCIE_SW_Pxx_REQ,
+			.map = IMX8MM_PCIE_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8MM_PGC_PCIE),
+	},
+
+	[IMX8MM_POWER_DOMAIN_OTG1] = {
+		.bits  = {
+			.pxx = IMX8MM_OTG1_SW_Pxx_REQ,
+			.map = IMX8MM_OTG1_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8MM_PGC_OTG1),
+	},
+
+	[IMX8MM_POWER_DOMAIN_OTG2] = {
+		.bits  = {
+			.pxx = IMX8MM_OTG2_SW_Pxx_REQ,
+			.map = IMX8MM_OTG2_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8MM_PGC_OTG2),
+	},
+};
+
+static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
+	.domains = imx8mm_pgc_domains,
+	.domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
+	.pgc_regs = &imx7_pgc_regs,
+};
+#endif
+
+#ifdef CONFIG_IMX8MN
+static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
+	[IMX8MN_POWER_DOMAIN_HSIOMIX] = {
+		.bits  = {
+			.pxx = 0, /* no power sequence control */
+			.map = 0, /* no power sequence control */
+			.hskreq = IMX8MN_HSIO_HSK_PWRDNREQN,
+			.hskack = IMX8MN_HSIO_HSK_PWRDNACKN,
+		},
+		.keep_clocks = true,
+	},
+
+	[IMX8MN_POWER_DOMAIN_OTG1] = {
+		.bits  = {
+			.pxx = IMX8MN_OTG1_SW_Pxx_REQ,
+			.map = IMX8MN_OTG1_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8MN_PGC_OTG1),
+	},
+};
+
+static const struct imx_pgc_domain_data imx8mn_pgc_domain_data = {
+	.domains = imx8mn_pgc_domains,
+	.domains_num = ARRAY_SIZE(imx8mn_pgc_domains),
+	.pgc_regs = &imx7_pgc_regs,
+};
+#endif
+
 static int imx8m_power_domain_on(struct power_domain *power_domain)
 {
 	struct udevice *dev = power_domain->dev;
-	struct imx8m_power_domain_plat *pdata;
+	struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
+	const struct imx_pgc_domain *domain = pdata->domain;
+	const struct imx_pgc_regs *regs = pdata->regs;
+	void __iomem *base = pdata->base;
+	u32 pgc;
+	int ret;
+
+	if (pdata->clk.count) {
+		ret = clk_enable_bulk(&pdata->clk);
+		if (ret) {
+			dev_err(dev, "failed to enable reset clocks\n");
+			return ret;
+		}
+	}
 
-	pdata = dev_get_plat(dev);
+	if (domain->bits.pxx) {
+		/* request the domain to power up */
+		setbits_le32(base + regs->pup, domain->bits.pxx);
 
-	if (pdata->resource_id < 0)
-		return -EINVAL;
+		/*
+		 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+		 * for PUP_REQ/PDN_REQ bit to be cleared
+		 */
+		ret = wait_for_bit_le32(base + regs->pup, domain->bits.pxx,
+					false, 1000, false);
+		if (ret) {
+			dev_err(dev, "failed to command PGC\n");
+			goto out_clk_disable;
+		}
 
-	if (pdata->has_pd)
-		power_domain_on(&pdata->pd);
+		/* disable power control */
+		for_each_set_bit(pgc, &domain->pgc, 32) {
+			clrbits_le32(base + GPC_PGC_CTRL(pgc),
+				     GPC_PGC_CTRL_PCR);
+		}
+	}
+
+	/* delay for reset to propagate */
+	udelay(5);
 
-	arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
-		      pdata->resource_id, 1, 0, 0, 0, 0, NULL);
+	/* request the ADB400 to power up */
+	if (domain->bits.hskreq)
+		setbits_le32(base + regs->hsk, domain->bits.hskreq);
+
+	/* Disable reset clocks for all devices in the domain */
+	if (!domain->keep_clocks && pdata->clk.count)
+		clk_disable_bulk(&pdata->clk);
 
 	return 0;
+
+out_clk_disable:
+	if (pdata->clk.count)
+		clk_disable_bulk(&pdata->clk);
+	return ret;
 }
 
 static int imx8m_power_domain_off(struct power_domain *power_domain)
 {
 	struct udevice *dev = power_domain->dev;
-	struct imx8m_power_domain_plat *pdata;
-	pdata = dev_get_plat(dev);
+	struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
+	const struct imx_pgc_domain *domain = pdata->domain;
+	const struct imx_pgc_regs *regs = pdata->regs;
+	void __iomem *base = pdata->base;
+	u32 pgc;
+	int ret;
 
-	if (pdata->resource_id < 0)
-		return -EINVAL;
+	/* Enable reset clocks for all devices in the domain */
+	if (!domain->keep_clocks && pdata->clk.count) {
+		ret = clk_enable_bulk(&pdata->clk);
+		if (ret)
+			return ret;
+	}
 
-	arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
-		      pdata->resource_id, 0, 0, 0, 0, 0, NULL);
+	/* request the ADB400 to power down */
+	if (domain->bits.hskreq) {
+		clrbits_le32(base + regs->hsk, domain->bits.hskreq);
+
+		ret = wait_for_bit_le32(base + regs->hsk, domain->bits.hskack,
+					false, 1000, false);
+		if (ret) {
+			dev_err(dev, "failed to power down ADB400\n");
+			goto out_clk_disable;
+		}
+	}
+
+	if (domain->bits.pxx) {
+		/* enable power control */
+		for_each_set_bit(pgc, &domain->pgc, 32) {
+			setbits_le32(base + GPC_PGC_CTRL(pgc),
+				     GPC_PGC_CTRL_PCR);
+		}
+
+		/* request the domain to power down */
+		setbits_le32(base + regs->pdn, domain->bits.pxx);
+
+		/*
+		 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+		 * for PUP_REQ/PDN_REQ bit to be cleared
+		 */
+		ret = wait_for_bit_le32(base + regs->pdn, domain->bits.pxx,
+					false, 1000, false);
+		if (ret) {
+			dev_err(dev, "failed to command PGC\n");
+			goto out_clk_disable;
+		}
+	}
+
+	/* Disable reset clocks for all devices in the domain */
+	if (pdata->clk.count)
+		clk_disable_bulk(&pdata->clk);
 
 	if (pdata->has_pd)
 		power_domain_off(&pdata->pd);
 
 	return 0;
+
+out_clk_disable:
+	if (!domain->keep_clocks && pdata->clk.count)
+		clk_disable_bulk(&pdata->clk);
+
+	return ret;
 }
 
 static int imx8m_power_domain_of_xlate(struct power_domain *power_domain,
@@ -101,12 +411,36 @@ static int imx8m_power_domain_bind(struct udevice *dev)
 	return 0;
 }
 
+static int imx8m_power_domain_probe(struct udevice *dev)
+{
+	struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
+	int ret;
+
+	/* Nothing to do for non-"power-domain" driver instances. */
+	if (!strstr(dev->name, "power-domain"))
+		return 0;
+
+	/* Grab optional power domain clock. */
+	ret = clk_get_bulk(dev, &pdata->clk);
+	if (ret && ret != -ENOENT) {
+		dev_err(dev, "Failed to get domain clock (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int imx8m_power_domain_of_to_plat(struct udevice *dev)
 {
 	struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
+	struct imx_pgc_domain_data *domain_data =
+		(struct imx_pgc_domain_data *)dev_get_driver_data(dev);
 
 	pdata->resource_id = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
 					    "reg", -1);
+	pdata->domain = &domain_data->domains[pdata->resource_id];
+	pdata->regs = domain_data->pgc_regs;
+	pdata->base = dev_read_addr_ptr(dev->parent);
 
 	if (!power_domain_get(dev, &pdata->pd))
 		pdata->has_pd = 1;
@@ -115,9 +449,15 @@ static int imx8m_power_domain_of_to_plat(struct udevice *dev)
 }
 
 static const struct udevice_id imx8m_power_domain_ids[] = {
-	{ .compatible = "fsl,imx8mq-gpc" },
-	{ .compatible = "fsl,imx8mm-gpc" },
-	{ .compatible = "fsl,imx8mn-gpc" },
+#ifdef CONFIG_IMX8MQ
+	{ .compatible = "fsl,imx8mq-gpc", .data = (long)&imx8m_pgc_domain_data },
+#endif
+#ifdef CONFIG_IMX8MM
+	{ .compatible = "fsl,imx8mm-gpc", .data = (long)&imx8mm_pgc_domain_data },
+#endif
+#ifdef CONFIG_IMX8MN
+	{ .compatible = "fsl,imx8mn-gpc", .data = (long)&imx8mn_pgc_domain_data },
+#endif
 	{ }
 };
 
@@ -132,6 +472,7 @@ U_BOOT_DRIVER(imx8m_power_domain) = {
 	.id = UCLASS_POWER_DOMAIN,
 	.of_match = imx8m_power_domain_ids,
 	.bind = imx8m_power_domain_bind,
+	.probe = imx8m_power_domain_probe,
 	.of_to_plat = imx8m_power_domain_of_to_plat,
 	.plat_auto	= sizeof(struct imx8m_power_domain_plat),
 	.ops = &imx8m_power_domain_ops,
-- 
2.35.1

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
>
> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> the driver based on Linux GPCv2 driver to directly control the GPCv2
> block instead of using SMCCC calls. This way, U-Boot can operate the
> i.MX8M power domains without depending on anything else.
>
> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> domains are not supported to save space, since they are not useful in
> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.

I thought there were people who were using video in U-Boot, but maybe
I am wrong.  I was looking into doing something like this, but I was
reluctant, because I wasn't sure if people were using video and/or
what might break.  I think this a good idea, but If people are using
video for some reason, this patch series would break that since ATF
would no longer be handling the blk control stuff.

I did a  quick check of defconfigs and board files and found none,
we're probably safe, but I wonder if expanding the CC list to include
more imx8mm/imx8mn users would be in order.

adam
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> NOTE: So far this is tested on MX8MM and MX8MN. MX8MQ is not tested.
> ---
>  drivers/power/domain/Kconfig              |   1 +
>  drivers/power/domain/imx8m-power-domain.c | 379 ++++++++++++++++++++--
>  2 files changed, 361 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
> index 93d2599d83c..04fc0054323 100644
> --- a/drivers/power/domain/Kconfig
> +++ b/drivers/power/domain/Kconfig
> @@ -35,6 +35,7 @@ config IMX8_POWER_DOMAIN
>  config IMX8M_POWER_DOMAIN
>         bool "Enable i.MX8M power domain driver"
>         depends on POWER_DOMAIN && ARCH_IMX8M
> +       select CLK
>         help
>           Enable support for manipulating NXP i.MX8M on-SoC power domains via
>           requests to the ATF.
> diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
> index c32dbcc31ae..e2e41cf5fee 100644
> --- a/drivers/power/domain/imx8m-power-domain.c
> +++ b/drivers/power/domain/imx8m-power-domain.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include <common.h>
> +#include <clk.h>
>  #include <dm.h>
>  #include <malloc.h>
>  #include <power-domain-uclass.h>
> @@ -12,52 +13,361 @@
>  #include <asm/mach-imx/sys_proto.h>
>  #include <dm/device-internal.h>
>  #include <dm/device.h>
> +#include <dm/device_compat.h>
>  #include <imx_sip.h>
> -#include <linux/arm-smccc.h>
> +#include <linux/bitmap.h>
> +#include <wait_bit.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +#include <dt-bindings/power/imx8mn-power.h>
> +#include <dt-bindings/power/imx8mq-power.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +#define GPC_PGC_CPU_MAPPING                    0x0ec
> +
> +#define IMX8M_PCIE2_A53_DOMAIN                 BIT(15)
> +#define IMX8M_OTG2_A53_DOMAIN                  BIT(5)
> +#define IMX8M_OTG1_A53_DOMAIN                  BIT(4)
> +#define IMX8M_PCIE1_A53_DOMAIN                 BIT(3)
> +
> +#define IMX8MM_OTG2_A53_DOMAIN                 BIT(5)
> +#define IMX8MM_OTG1_A53_DOMAIN                 BIT(4)
> +#define IMX8MM_PCIE_A53_DOMAIN                 BIT(3)
> +
> +#define IMX8MN_OTG1_A53_DOMAIN                 BIT(4)
> +#define IMX8MN_MIPI_A53_DOMAIN                 BIT(2)
> +
> +#define GPC_PU_PGC_SW_PUP_REQ                  0x0f8
> +#define GPC_PU_PGC_SW_PDN_REQ                  0x104
> +
> +#define IMX8M_PCIE2_SW_Pxx_REQ                 BIT(13)
> +#define IMX8M_OTG2_SW_Pxx_REQ                  BIT(3)
> +#define IMX8M_OTG1_SW_Pxx_REQ                  BIT(2)
> +#define IMX8M_PCIE1_SW_Pxx_REQ                 BIT(1)
> +
> +#define IMX8MM_OTG2_SW_Pxx_REQ                 BIT(3)
> +#define IMX8MM_OTG1_SW_Pxx_REQ                 BIT(2)
> +#define IMX8MM_PCIE_SW_Pxx_REQ                 BIT(1)
> +
> +#define IMX8MN_OTG1_SW_Pxx_REQ                 BIT(2)
> +#define IMX8MN_MIPI_SW_Pxx_REQ                 BIT(0)
> +
> +#define GPC_M4_PU_PDN_FLG                      0x1bc
> +
> +#define GPC_PU_PWRHSK                          0x1fc
> +
> +#define IMX8MM_HSIO_HSK_PWRDNACKN              (BIT(23) | BIT(24))
> +#define IMX8MM_HSIO_HSK_PWRDNREQN              (BIT(5) | BIT(6))
> +
> +#define IMX8MN_HSIO_HSK_PWRDNACKN              BIT(23)
> +#define IMX8MN_HSIO_HSK_PWRDNREQN              BIT(5)
> +
> +/*
> + * The PGC offset values in Reference Manual
> + * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> + * GPC_PGC memory map are incorrect, below offset
> + * values are from design RTL.
> + */
> +#define IMX8M_PGC_PCIE1                        17
> +#define IMX8M_PGC_OTG1                 18
> +#define IMX8M_PGC_OTG2                 19
> +#define IMX8M_PGC_PCIE2                        29
> +
> +#define IMX8MM_PGC_PCIE                        17
> +#define IMX8MM_PGC_OTG1                        18
> +#define IMX8MM_PGC_OTG2                        19
> +
> +#define IMX8MN_PGC_OTG1                        18
> +
> +#define GPC_PGC_CTRL(n)                        (0x800 + (n) * 0x40)
> +#define GPC_PGC_SR(n)                  (GPC_PGC_CTRL(n) + 0xc)
> +
> +#define GPC_PGC_CTRL_PCR               BIT(0)
> +
> +struct imx_pgc_regs {
> +       u16 map;
> +       u16 pup;
> +       u16 pdn;
> +       u16 hsk;
> +};
> +
> +struct imx_pgc_domain {
> +       unsigned long pgc;
> +
> +       const struct {
> +               u32 pxx;
> +               u32 map;
> +               u32 hskreq;
> +               u32 hskack;
> +       } bits;
> +
> +       const bool keep_clocks;
> +};
> +
> +struct imx_pgc_domain_data {
> +       const struct imx_pgc_domain *domains;
> +       size_t domains_num;
> +       const struct imx_pgc_regs *pgc_regs;
> +};
> +
>  struct imx8m_power_domain_plat {
> +       struct power_domain pd;
> +       const struct imx_pgc_domain *domain;
> +       const struct imx_pgc_regs *regs;
> +       struct clk_bulk clk;
> +       void __iomem *base;
>         int resource_id;
>         int has_pd;
> -       struct power_domain pd;
>  };
>
> +#if defined(CONFIG_IMX8MM) || defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MQ)
> +static const struct imx_pgc_regs imx7_pgc_regs = {
> +       .map = GPC_PGC_CPU_MAPPING,
> +       .pup = GPC_PU_PGC_SW_PUP_REQ,
> +       .pdn = GPC_PU_PGC_SW_PDN_REQ,
> +       .hsk = GPC_PU_PWRHSK,
> +};
> +#endif
> +
> +#ifdef CONFIG_IMX8MQ
> +static const struct imx_pgc_domain imx8m_pgc_domains[] = {
> +       [IMX8M_POWER_DOMAIN_PCIE1] = {
> +               .bits  = {
> +                       .pxx = IMX8M_PCIE1_SW_Pxx_REQ,
> +                       .map = IMX8M_PCIE1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_PCIE1),
> +       },
> +
> +       [IMX8M_POWER_DOMAIN_USB_OTG1] = {
> +               .bits  = {
> +                       .pxx = IMX8M_OTG1_SW_Pxx_REQ,
> +                       .map = IMX8M_OTG1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_OTG1),
> +       },
> +
> +       [IMX8M_POWER_DOMAIN_USB_OTG2] = {
> +               .bits  = {
> +                       .pxx = IMX8M_OTG2_SW_Pxx_REQ,
> +                       .map = IMX8M_OTG2_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_OTG2),
> +       },
> +
> +       [IMX8M_POWER_DOMAIN_PCIE2] = {
> +               .bits  = {
> +                       .pxx = IMX8M_PCIE2_SW_Pxx_REQ,
> +                       .map = IMX8M_PCIE2_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_PCIE2),
> +       },
> +};
> +
> +static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
> +       .domains = imx8m_pgc_domains,
> +       .domains_num = ARRAY_SIZE(imx8m_pgc_domains),
> +       .pgc_regs = &imx7_pgc_regs,
> +};
> +#endif
> +
> +#ifdef CONFIG_IMX8MM
> +static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> +       [IMX8MM_POWER_DOMAIN_HSIOMIX] = {
> +               .bits  = {
> +                       .pxx = 0, /* no power sequence control */
> +                       .map = 0, /* no power sequence control */
> +                       .hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
> +                       .hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
> +               },
> +               .keep_clocks = true,
> +       },
> +
> +       [IMX8MM_POWER_DOMAIN_PCIE] = {
> +               .bits  = {
> +                       .pxx = IMX8MM_PCIE_SW_Pxx_REQ,
> +                       .map = IMX8MM_PCIE_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MM_PGC_PCIE),
> +       },
> +
> +       [IMX8MM_POWER_DOMAIN_OTG1] = {
> +               .bits  = {
> +                       .pxx = IMX8MM_OTG1_SW_Pxx_REQ,
> +                       .map = IMX8MM_OTG1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MM_PGC_OTG1),
> +       },
> +
> +       [IMX8MM_POWER_DOMAIN_OTG2] = {
> +               .bits  = {
> +                       .pxx = IMX8MM_OTG2_SW_Pxx_REQ,
> +                       .map = IMX8MM_OTG2_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MM_PGC_OTG2),
> +       },
> +};
> +
> +static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
> +       .domains = imx8mm_pgc_domains,
> +       .domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
> +       .pgc_regs = &imx7_pgc_regs,
> +};
> +#endif
> +
> +#ifdef CONFIG_IMX8MN
> +static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
> +       [IMX8MN_POWER_DOMAIN_HSIOMIX] = {
> +               .bits  = {
> +                       .pxx = 0, /* no power sequence control */
> +                       .map = 0, /* no power sequence control */
> +                       .hskreq = IMX8MN_HSIO_HSK_PWRDNREQN,
> +                       .hskack = IMX8MN_HSIO_HSK_PWRDNACKN,
> +               },
> +               .keep_clocks = true,
> +       },
> +
> +       [IMX8MN_POWER_DOMAIN_OTG1] = {
> +               .bits  = {
> +                       .pxx = IMX8MN_OTG1_SW_Pxx_REQ,
> +                       .map = IMX8MN_OTG1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MN_PGC_OTG1),
> +       },
> +};
> +
> +static const struct imx_pgc_domain_data imx8mn_pgc_domain_data = {
> +       .domains = imx8mn_pgc_domains,
> +       .domains_num = ARRAY_SIZE(imx8mn_pgc_domains),
> +       .pgc_regs = &imx7_pgc_regs,
> +};
> +#endif
> +
>  static int imx8m_power_domain_on(struct power_domain *power_domain)
>  {
>         struct udevice *dev = power_domain->dev;
> -       struct imx8m_power_domain_plat *pdata;
> +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       const struct imx_pgc_domain *domain = pdata->domain;
> +       const struct imx_pgc_regs *regs = pdata->regs;
> +       void __iomem *base = pdata->base;
> +       u32 pgc;
> +       int ret;
> +
> +       if (pdata->clk.count) {
> +               ret = clk_enable_bulk(&pdata->clk);
> +               if (ret) {
> +                       dev_err(dev, "failed to enable reset clocks\n");
> +                       return ret;
> +               }
> +       }
>
> -       pdata = dev_get_plat(dev);
> +       if (domain->bits.pxx) {
> +               /* request the domain to power up */
> +               setbits_le32(base + regs->pup, domain->bits.pxx);
>
> -       if (pdata->resource_id < 0)
> -               return -EINVAL;
> +               /*
> +                * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +                * for PUP_REQ/PDN_REQ bit to be cleared
> +                */
> +               ret = wait_for_bit_le32(base + regs->pup, domain->bits.pxx,
> +                                       false, 1000, false);
> +               if (ret) {
> +                       dev_err(dev, "failed to command PGC\n");
> +                       goto out_clk_disable;
> +               }
>
> -       if (pdata->has_pd)
> -               power_domain_on(&pdata->pd);
> +               /* disable power control */
> +               for_each_set_bit(pgc, &domain->pgc, 32) {
> +                       clrbits_le32(base + GPC_PGC_CTRL(pgc),
> +                                    GPC_PGC_CTRL_PCR);
> +               }
> +       }
> +
> +       /* delay for reset to propagate */
> +       udelay(5);
>
> -       arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
> -                     pdata->resource_id, 1, 0, 0, 0, 0, NULL);
> +       /* request the ADB400 to power up */
> +       if (domain->bits.hskreq)
> +               setbits_le32(base + regs->hsk, domain->bits.hskreq);
> +
> +       /* Disable reset clocks for all devices in the domain */
> +       if (!domain->keep_clocks && pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
>
>         return 0;
> +
> +out_clk_disable:
> +       if (pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
> +       return ret;
>  }
>
>  static int imx8m_power_domain_off(struct power_domain *power_domain)
>  {
>         struct udevice *dev = power_domain->dev;
> -       struct imx8m_power_domain_plat *pdata;
> -       pdata = dev_get_plat(dev);
> +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       const struct imx_pgc_domain *domain = pdata->domain;
> +       const struct imx_pgc_regs *regs = pdata->regs;
> +       void __iomem *base = pdata->base;
> +       u32 pgc;
> +       int ret;
>
> -       if (pdata->resource_id < 0)
> -               return -EINVAL;
> +       /* Enable reset clocks for all devices in the domain */
> +       if (!domain->keep_clocks && pdata->clk.count) {
> +               ret = clk_enable_bulk(&pdata->clk);
> +               if (ret)
> +                       return ret;
> +       }
>
> -       arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
> -                     pdata->resource_id, 0, 0, 0, 0, 0, NULL);
> +       /* request the ADB400 to power down */
> +       if (domain->bits.hskreq) {
> +               clrbits_le32(base + regs->hsk, domain->bits.hskreq);
> +
> +               ret = wait_for_bit_le32(base + regs->hsk, domain->bits.hskack,
> +                                       false, 1000, false);
> +               if (ret) {
> +                       dev_err(dev, "failed to power down ADB400\n");
> +                       goto out_clk_disable;
> +               }
> +       }
> +
> +       if (domain->bits.pxx) {
> +               /* enable power control */
> +               for_each_set_bit(pgc, &domain->pgc, 32) {
> +                       setbits_le32(base + GPC_PGC_CTRL(pgc),
> +                                    GPC_PGC_CTRL_PCR);
> +               }
> +
> +               /* request the domain to power down */
> +               setbits_le32(base + regs->pdn, domain->bits.pxx);
> +
> +               /*
> +                * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +                * for PUP_REQ/PDN_REQ bit to be cleared
> +                */
> +               ret = wait_for_bit_le32(base + regs->pdn, domain->bits.pxx,
> +                                       false, 1000, false);
> +               if (ret) {
> +                       dev_err(dev, "failed to command PGC\n");
> +                       goto out_clk_disable;
> +               }
> +       }
> +
> +       /* Disable reset clocks for all devices in the domain */
> +       if (pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
>
>         if (pdata->has_pd)
>                 power_domain_off(&pdata->pd);
>
>         return 0;
> +
> +out_clk_disable:
> +       if (!domain->keep_clocks && pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
> +
> +       return ret;
>  }
>
>  static int imx8m_power_domain_of_xlate(struct power_domain *power_domain,
> @@ -101,12 +411,36 @@ static int imx8m_power_domain_bind(struct udevice *dev)
>         return 0;
>  }
>
> +static int imx8m_power_domain_probe(struct udevice *dev)
> +{
> +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       int ret;
> +
> +       /* Nothing to do for non-"power-domain" driver instances. */
> +       if (!strstr(dev->name, "power-domain"))
> +               return 0;
> +
> +       /* Grab optional power domain clock. */
> +       ret = clk_get_bulk(dev, &pdata->clk);
> +       if (ret && ret != -ENOENT) {
> +               dev_err(dev, "Failed to get domain clock (%d)\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  static int imx8m_power_domain_of_to_plat(struct udevice *dev)
>  {
>         struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       struct imx_pgc_domain_data *domain_data =
> +               (struct imx_pgc_domain_data *)dev_get_driver_data(dev);
>
>         pdata->resource_id = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
>                                             "reg", -1);
> +       pdata->domain = &domain_data->domains[pdata->resource_id];
> +       pdata->regs = domain_data->pgc_regs;
> +       pdata->base = dev_read_addr_ptr(dev->parent);
>
>         if (!power_domain_get(dev, &pdata->pd))
>                 pdata->has_pd = 1;
> @@ -115,9 +449,15 @@ static int imx8m_power_domain_of_to_plat(struct udevice *dev)
>  }
>
>  static const struct udevice_id imx8m_power_domain_ids[] = {
> -       { .compatible = "fsl,imx8mq-gpc" },
> -       { .compatible = "fsl,imx8mm-gpc" },
> -       { .compatible = "fsl,imx8mn-gpc" },
> +#ifdef CONFIG_IMX8MQ
> +       { .compatible = "fsl,imx8mq-gpc", .data = (long)&imx8m_pgc_domain_data },
> +#endif
> +#ifdef CONFIG_IMX8MM
> +       { .compatible = "fsl,imx8mm-gpc", .data = (long)&imx8mm_pgc_domain_data },
> +#endif
> +#ifdef CONFIG_IMX8MN
> +       { .compatible = "fsl,imx8mn-gpc", .data = (long)&imx8mn_pgc_domain_data },
> +#endif
>         { }
>  };
>
> @@ -132,6 +472,7 @@ U_BOOT_DRIVER(imx8m_power_domain) = {
>         .id = UCLASS_POWER_DOMAIN,
>         .of_match = imx8m_power_domain_ids,
>         .bind = imx8m_power_domain_bind,
> +       .probe = imx8m_power_domain_probe,
>         .of_to_plat = imx8m_power_domain_of_to_plat,
>         .plat_auto      = sizeof(struct imx8m_power_domain_plat),
>         .ops = &imx8m_power_domain_ops,
> --
> 2.35.1
>

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/4/22 14:51, Adam Ford wrote:
> On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
>>
>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
>> the driver based on Linux GPCv2 driver to directly control the GPCv2
>> block instead of using SMCCC calls. This way, U-Boot can operate the
>> i.MX8M power domains without depending on anything else.
>>
>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
>> domains are not supported to save space, since they are not useful in
>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> 
> I thought there were people who were using video in U-Boot, but maybe
> I am wrong.

There are no video drivers for MX8M in U-Boot, it's all USB and maybe 
sometimes in the future PCIe.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Mon, Apr 4, 2022 at 8:01 AM Marek Vasut <marex@denx.de> wrote:
>
> On 4/4/22 14:51, Adam Ford wrote:
> > On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> >> the driver based on Linux GPCv2 driver to directly control the GPCv2
> >> block instead of using SMCCC calls. This way, U-Boot can operate the
> >> i.MX8M power domains without depending on anything else.
> >>
> >> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> >> domains are not supported to save space, since they are not useful in
> >> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> >
> > I thought there were people who were using video in U-Boot, but maybe
> > I am wrong.
>
> There are no video drivers for MX8M in U-Boot, it's all USB and maybe
> sometimes in the future PCIe.

Oh good.

I'll try to test it on an imx8mq when I get some time.

adam

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/4/22 16:15, Adam Ford wrote:
> On Mon, Apr 4, 2022 at 8:01 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 4/4/22 14:51, Adam Ford wrote:
>>> On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
>>>> the driver based on Linux GPCv2 driver to directly control the GPCv2
>>>> block instead of using SMCCC calls. This way, U-Boot can operate the
>>>> i.MX8M power domains without depending on anything else.
>>>>
>>>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
>>>> domains are not supported to save space, since they are not useful in
>>>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
>>>
>>> I thought there were people who were using video in U-Boot, but maybe
>>> I am wrong.
>>
>> There are no video drivers for MX8M in U-Boot, it's all USB and maybe
>> sometimes in the future PCIe.
> 
> Oh good.
> 
> I'll try to test it on an imx8mq when I get some time.

The entire stack of patches is at:

https://source.denx.de/u-boot/custodians/u-boot-usb/-/commits/imx-8mp

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Tim Harvey]

On Mon, Apr 4, 2022 at 7:25 AM Marek Vasut <marex@denx.de> wrote:
>
> On 4/4/22 16:15, Adam Ford wrote:
> > On Mon, Apr 4, 2022 at 8:01 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 4/4/22 14:51, Adam Ford wrote:
> >>> On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> >>>> the driver based on Linux GPCv2 driver to directly control the GPCv2
> >>>> block instead of using SMCCC calls. This way, U-Boot can operate the
> >>>> i.MX8M power domains without depending on anything else.
> >>>>
> >>>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> >>>> domains are not supported to save space, since they are not useful in
> >>>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> >>>
> >>> I thought there were people who were using video in U-Boot, but maybe
> >>> I am wrong.
> >>
> >> There are no video drivers for MX8M in U-Boot, it's all USB and maybe
> >> sometimes in the future PCIe.
> >
> > Oh good.
> >
> > I'll try to test it on an imx8mq when I get some time.
>
> The entire stack of patches is at:
>
> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commits/imx-8mp

For the series:

Tested-By: Tim Harvey <tharvey@gateworks.com> #imx8mp-venice-defconfig

This was tested on an IMX8MP board that I'm bringing up
(imx8mp-venice-defconfig) but have not yet submitted.

Best Regards,

Tim

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/5/22 22:14, Tim Harvey wrote:
> On Mon, Apr 4, 2022 at 7:25 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 4/4/22 16:15, Adam Ford wrote:
>>> On Mon, Apr 4, 2022 at 8:01 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 4/4/22 14:51, Adam Ford wrote:
>>>>> On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
>>>>>> the driver based on Linux GPCv2 driver to directly control the GPCv2
>>>>>> block instead of using SMCCC calls. This way, U-Boot can operate the
>>>>>> i.MX8M power domains without depending on anything else.
>>>>>>
>>>>>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
>>>>>> domains are not supported to save space, since they are not useful in
>>>>>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
>>>>>
>>>>> I thought there were people who were using video in U-Boot, but maybe
>>>>> I am wrong.
>>>>
>>>> There are no video drivers for MX8M in U-Boot, it's all USB and maybe
>>>> sometimes in the future PCIe.
>>>
>>> Oh good.
>>>
>>> I'll try to test it on an imx8mq when I get some time.
>>
>> The entire stack of patches is at:
>>
>> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commits/imx-8mp
> 
> For the series:
> 
> Tested-By: Tim Harvey <tharvey@gateworks.com> #imx8mp-venice-defconfig
> 
> This was tested on an IMX8MP board that I'm bringing up
> (imx8mp-venice-defconfig) but have not yet submitted.

Nice. If you want to add TB for any other patches you tested, that would 
be welcome. I think there are a few more involved.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Tue, Apr 5, 2022 at 6:00 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/5/22 22:14, Tim Harvey wrote:
> > On Mon, Apr 4, 2022 at 7:25 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 4/4/22 16:15, Adam Ford wrote:
> >>> On Mon, Apr 4, 2022 at 8:01 AM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 4/4/22 14:51, Adam Ford wrote:
> >>>>> On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
> >>>>>>
> >>>>>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> >>>>>> the driver based on Linux GPCv2 driver to directly control the GPCv2
> >>>>>> block instead of using SMCCC calls. This way, U-Boot can operate the
> >>>>>> i.MX8M power domains without depending on anything else.
> >>>>>>
> >>>>>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> >>>>>> domains are not supported to save space, since they are not useful in
> >>>>>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> >>>>>
> >>>>> I thought there were people who were using video in U-Boot, but maybe
> >>>>> I am wrong.
> >>>>
> >>>> There are no video drivers for MX8M in U-Boot, it's all USB and maybe
> >>>> sometimes in the future PCIe.
> >>>
> >>> Oh good.
> >>>
> >>> I'll try to test it on an imx8mq when I get some time.
> >>
> >> The entire stack of patches is at:
> >>
> >> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commits/imx-8mp
> >
> > For the series:
> >
> > Tested-By: Tim Harvey <tharvey@gateworks.com> #imx8mp-venice-defconfig
> >
> > This was tested on an IMX8MP board that I'm bringing up
> > (imx8mp-venice-defconfig) but have not yet submitted.
>
> Nice. If you want to add TB for any other patches you tested, that would
> be welcome. I think there are a few more involved.

I tried to build the imx8mq last night, but I ran into issues.  I
never built an imx8mq bootloader before, so I think I'm missing
something.  I'll try to get to it again this week.  Sorry for the
delays.

adam

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Fabio Estevam]

Hi Adam,

On Wed, Apr 6, 2022 at 9:22 AM Adam Ford <aford173@gmail.com> wrote:

> I tried to build the imx8mq last night, but I ran into issues.  I
> never built an imx8mq bootloader before, so I think I'm missing
> something.  I'll try to get to it again this week.  Sorry for the
> delays.

Please check doc/board/nxp/imx8mq_evk.rst for a reference.

The flash.bin offset is 33K instead of 32K on imx8mm.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Wed, Apr 6, 2022 at 7:27 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Adam,
>
> On Wed, Apr 6, 2022 at 9:22 AM Adam Ford <aford173@gmail.com> wrote:
>
> > I tried to build the imx8mq last night, but I ran into issues.  I
> > never built an imx8mq bootloader before, so I think I'm missing
> > something.  I'll try to get to it again this week.  Sorry for the
> > delays.
>
> Please check doc/board/nxp/imx8mq_evk.rst for a reference.

Thanks for the heads up on the help file.  I was looking in the wrong
place for it.
>
> The flash.bin offset is 33K instead of 32K on imx8mm.

Unfortunately, the board hangs:

U-Boot SPL 2022.04-rc5-gfd0c0181d8 (Apr 06 2022 - 07:36:33 -0500)
PMIC:  PFUZE100 ID=0x10
Normal Boot
Trying to boot from MMC2
<hang>

I am not sure if it's related to your changes or something else is
broken.  I have to switch gears for now, but I'll try the master
branch to see if there are any differences in behavior once I have
some more time.

adam

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/6/22 14:40, Adam Ford wrote:
> On Wed, Apr 6, 2022 at 7:27 AM Fabio Estevam <festevam@gmail.com> wrote:
>>
>> Hi Adam,
>>
>> On Wed, Apr 6, 2022 at 9:22 AM Adam Ford <aford173@gmail.com> wrote:
>>
>>> I tried to build the imx8mq last night, but I ran into issues.  I
>>> never built an imx8mq bootloader before, so I think I'm missing
>>> something.  I'll try to get to it again this week.  Sorry for the
>>> delays.
>>
>> Please check doc/board/nxp/imx8mq_evk.rst for a reference.
> 
> Thanks for the heads up on the help file.  I was looking in the wrong
> place for it.
>>
>> The flash.bin offset is 33K instead of 32K on imx8mm.
> 
> Unfortunately, the board hangs:
> 
> U-Boot SPL 2022.04-rc5-gfd0c0181d8 (Apr 06 2022 - 07:36:33 -0500)
> PMIC:  PFUZE100 ID=0x10
> Normal Boot
> Trying to boot from MMC2
> <hang>
> 
> I am not sure if it's related to your changes or something else is
> broken.  I have to switch gears for now, but I'll try the master
> branch to see if there are any differences in behavior once I have
> some more time.

Did you use the correct ATF blob ? Did you set correct ATF_LOAD_ADDR 
during build ? Did you use the correct set of DRAM PHY blobs ?

This hang looks very much like hang due to bad ATF , unrelated to this 
series.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Tue, Apr 5, 2022 at 3:14 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Mon, Apr 4, 2022 at 7:25 AM Marek Vasut <marex@denx.de> wrote:
> >
> > On 4/4/22 16:15, Adam Ford wrote:
> > > On Mon, Apr 4, 2022 at 8:01 AM Marek Vasut <marex@denx.de> wrote:
> > >>
> > >> On 4/4/22 14:51, Adam Ford wrote:
> > >>> On Wed, Mar 30, 2022 at 10:04 PM Marek Vasut <marex@denx.de> wrote:
> > >>>>
> > >>>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> > >>>> the driver based on Linux GPCv2 driver to directly control the GPCv2
> > >>>> block instead of using SMCCC calls. This way, U-Boot can operate the
> > >>>> i.MX8M power domains without depending on anything else.
> > >>>>
> > >>>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> > >>>> domains are not supported to save space, since they are not useful in
> > >>>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> > >>>
> > >>> I thought there were people who were using video in U-Boot, but maybe
> > >>> I am wrong.
> > >>
> > >> There are no video drivers for MX8M in U-Boot, it's all USB and maybe
> > >> sometimes in the future PCIe.
> > >
> > > Oh good.
> > >
> > > I'll try to test it on an imx8mq when I get some time.
> >
> > The entire stack of patches is at:
> >
> > https://source.denx.de/u-boot/custodians/u-boot-usb/-/commits/imx-8mp
>
> For the series:
>
> Tested-By: Tim Harvey <tharvey@gateworks.com> #imx8mp-venice-defconfig
>
> This was tested on an IMX8MP board that I'm bringing up
> (imx8mp-venice-defconfig) but have not yet submitted.
>

There needs to be a subsequent patch because the 8MQ clocks and CCF
stuff don't appear to be enabled by default, but Kconfig looks like it
should.
Once I have figured it out, I'll send a subsequent patch to address that.

For the series:

Tested-by: Adam Ford <aford173@gmail.com> #imx8mq-evk

adam

> Best Regards,
>
> Tim

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Tim Harvey]

On Wed, Mar 30, 2022 at 8:04 PM Marek Vasut <marex@denx.de> wrote:
>
> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> the driver based on Linux GPCv2 driver to directly control the GPCv2
> block instead of using SMCCC calls. This way, U-Boot can operate the
> i.MX8M power domains without depending on anything else.
>
> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> domains are not supported to save space, since they are not useful in
> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> NOTE: So far this is tested on MX8MM and MX8MN. MX8MQ is not tested.
> ---
>  drivers/power/domain/Kconfig              |   1 +
>  drivers/power/domain/imx8m-power-domain.c | 379 ++++++++++++++++++++--
>  2 files changed, 361 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
> index 93d2599d83c..04fc0054323 100644
> --- a/drivers/power/domain/Kconfig
> +++ b/drivers/power/domain/Kconfig
> @@ -35,6 +35,7 @@ config IMX8_POWER_DOMAIN
>  config IMX8M_POWER_DOMAIN
>         bool "Enable i.MX8M power domain driver"
>         depends on POWER_DOMAIN && ARCH_IMX8M
> +       select CLK
>         help
>           Enable support for manipulating NXP i.MX8M on-SoC power domains via
>           requests to the ATF.
> diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
> index c32dbcc31ae..e2e41cf5fee 100644
> --- a/drivers/power/domain/imx8m-power-domain.c
> +++ b/drivers/power/domain/imx8m-power-domain.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include <common.h>
> +#include <clk.h>
>  #include <dm.h>
>  #include <malloc.h>
>  #include <power-domain-uclass.h>
> @@ -12,52 +13,361 @@
>  #include <asm/mach-imx/sys_proto.h>
>  #include <dm/device-internal.h>
>  #include <dm/device.h>
> +#include <dm/device_compat.h>
>  #include <imx_sip.h>
> -#include <linux/arm-smccc.h>
> +#include <linux/bitmap.h>
> +#include <wait_bit.h>
> +
> +#include <dt-bindings/power/imx8mm-power.h>
> +#include <dt-bindings/power/imx8mn-power.h>
> +#include <dt-bindings/power/imx8mq-power.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +#define GPC_PGC_CPU_MAPPING                    0x0ec
> +
> +#define IMX8M_PCIE2_A53_DOMAIN                 BIT(15)
> +#define IMX8M_OTG2_A53_DOMAIN                  BIT(5)
> +#define IMX8M_OTG1_A53_DOMAIN                  BIT(4)
> +#define IMX8M_PCIE1_A53_DOMAIN                 BIT(3)
> +
> +#define IMX8MM_OTG2_A53_DOMAIN                 BIT(5)
> +#define IMX8MM_OTG1_A53_DOMAIN                 BIT(4)
> +#define IMX8MM_PCIE_A53_DOMAIN                 BIT(3)
> +
> +#define IMX8MN_OTG1_A53_DOMAIN                 BIT(4)
> +#define IMX8MN_MIPI_A53_DOMAIN                 BIT(2)
> +
> +#define GPC_PU_PGC_SW_PUP_REQ                  0x0f8
> +#define GPC_PU_PGC_SW_PDN_REQ                  0x104
> +
> +#define IMX8M_PCIE2_SW_Pxx_REQ                 BIT(13)
> +#define IMX8M_OTG2_SW_Pxx_REQ                  BIT(3)
> +#define IMX8M_OTG1_SW_Pxx_REQ                  BIT(2)
> +#define IMX8M_PCIE1_SW_Pxx_REQ                 BIT(1)
> +
> +#define IMX8MM_OTG2_SW_Pxx_REQ                 BIT(3)
> +#define IMX8MM_OTG1_SW_Pxx_REQ                 BIT(2)
> +#define IMX8MM_PCIE_SW_Pxx_REQ                 BIT(1)
> +
> +#define IMX8MN_OTG1_SW_Pxx_REQ                 BIT(2)
> +#define IMX8MN_MIPI_SW_Pxx_REQ                 BIT(0)
> +
> +#define GPC_M4_PU_PDN_FLG                      0x1bc
> +
> +#define GPC_PU_PWRHSK                          0x1fc
> +
> +#define IMX8MM_HSIO_HSK_PWRDNACKN              (BIT(23) | BIT(24))
> +#define IMX8MM_HSIO_HSK_PWRDNREQN              (BIT(5) | BIT(6))
> +
> +#define IMX8MN_HSIO_HSK_PWRDNACKN              BIT(23)
> +#define IMX8MN_HSIO_HSK_PWRDNREQN              BIT(5)
> +
> +/*
> + * The PGC offset values in Reference Manual
> + * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> + * GPC_PGC memory map are incorrect, below offset
> + * values are from design RTL.
> + */
> +#define IMX8M_PGC_PCIE1                        17
> +#define IMX8M_PGC_OTG1                 18
> +#define IMX8M_PGC_OTG2                 19
> +#define IMX8M_PGC_PCIE2                        29
> +
> +#define IMX8MM_PGC_PCIE                        17
> +#define IMX8MM_PGC_OTG1                        18
> +#define IMX8MM_PGC_OTG2                        19
> +
> +#define IMX8MN_PGC_OTG1                        18
> +
> +#define GPC_PGC_CTRL(n)                        (0x800 + (n) * 0x40)
> +#define GPC_PGC_SR(n)                  (GPC_PGC_CTRL(n) + 0xc)
> +
> +#define GPC_PGC_CTRL_PCR               BIT(0)
> +
> +struct imx_pgc_regs {
> +       u16 map;
> +       u16 pup;
> +       u16 pdn;
> +       u16 hsk;
> +};
> +
> +struct imx_pgc_domain {
> +       unsigned long pgc;
> +
> +       const struct {
> +               u32 pxx;
> +               u32 map;
> +               u32 hskreq;
> +               u32 hskack;
> +       } bits;
> +
> +       const bool keep_clocks;
> +};
> +
> +struct imx_pgc_domain_data {
> +       const struct imx_pgc_domain *domains;
> +       size_t domains_num;
> +       const struct imx_pgc_regs *pgc_regs;
> +};
> +
>  struct imx8m_power_domain_plat {
> +       struct power_domain pd;
> +       const struct imx_pgc_domain *domain;
> +       const struct imx_pgc_regs *regs;
> +       struct clk_bulk clk;
> +       void __iomem *base;
>         int resource_id;
>         int has_pd;
> -       struct power_domain pd;
>  };
>
> +#if defined(CONFIG_IMX8MM) || defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MQ)
> +static const struct imx_pgc_regs imx7_pgc_regs = {
> +       .map = GPC_PGC_CPU_MAPPING,
> +       .pup = GPC_PU_PGC_SW_PUP_REQ,
> +       .pdn = GPC_PU_PGC_SW_PDN_REQ,
> +       .hsk = GPC_PU_PWRHSK,
> +};
> +#endif
> +
> +#ifdef CONFIG_IMX8MQ
> +static const struct imx_pgc_domain imx8m_pgc_domains[] = {
> +       [IMX8M_POWER_DOMAIN_PCIE1] = {
> +               .bits  = {
> +                       .pxx = IMX8M_PCIE1_SW_Pxx_REQ,
> +                       .map = IMX8M_PCIE1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_PCIE1),
> +       },
> +
> +       [IMX8M_POWER_DOMAIN_USB_OTG1] = {
> +               .bits  = {
> +                       .pxx = IMX8M_OTG1_SW_Pxx_REQ,
> +                       .map = IMX8M_OTG1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_OTG1),
> +       },
> +
> +       [IMX8M_POWER_DOMAIN_USB_OTG2] = {
> +               .bits  = {
> +                       .pxx = IMX8M_OTG2_SW_Pxx_REQ,
> +                       .map = IMX8M_OTG2_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_OTG2),
> +       },
> +
> +       [IMX8M_POWER_DOMAIN_PCIE2] = {
> +               .bits  = {
> +                       .pxx = IMX8M_PCIE2_SW_Pxx_REQ,
> +                       .map = IMX8M_PCIE2_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8M_PGC_PCIE2),
> +       },
> +};
> +
> +static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
> +       .domains = imx8m_pgc_domains,
> +       .domains_num = ARRAY_SIZE(imx8m_pgc_domains),
> +       .pgc_regs = &imx7_pgc_regs,
> +};
> +#endif
> +
> +#ifdef CONFIG_IMX8MM
> +static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> +       [IMX8MM_POWER_DOMAIN_HSIOMIX] = {
> +               .bits  = {
> +                       .pxx = 0, /* no power sequence control */
> +                       .map = 0, /* no power sequence control */
> +                       .hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
> +                       .hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
> +               },
> +               .keep_clocks = true,
> +       },
> +
> +       [IMX8MM_POWER_DOMAIN_PCIE] = {
> +               .bits  = {
> +                       .pxx = IMX8MM_PCIE_SW_Pxx_REQ,
> +                       .map = IMX8MM_PCIE_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MM_PGC_PCIE),
> +       },
> +
> +       [IMX8MM_POWER_DOMAIN_OTG1] = {
> +               .bits  = {
> +                       .pxx = IMX8MM_OTG1_SW_Pxx_REQ,
> +                       .map = IMX8MM_OTG1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MM_PGC_OTG1),
> +       },
> +
> +       [IMX8MM_POWER_DOMAIN_OTG2] = {
> +               .bits  = {
> +                       .pxx = IMX8MM_OTG2_SW_Pxx_REQ,
> +                       .map = IMX8MM_OTG2_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MM_PGC_OTG2),
> +       },
> +};
> +
> +static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
> +       .domains = imx8mm_pgc_domains,
> +       .domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
> +       .pgc_regs = &imx7_pgc_regs,
> +};
> +#endif
> +
> +#ifdef CONFIG_IMX8MN
> +static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
> +       [IMX8MN_POWER_DOMAIN_HSIOMIX] = {
> +               .bits  = {
> +                       .pxx = 0, /* no power sequence control */
> +                       .map = 0, /* no power sequence control */
> +                       .hskreq = IMX8MN_HSIO_HSK_PWRDNREQN,
> +                       .hskack = IMX8MN_HSIO_HSK_PWRDNACKN,
> +               },
> +               .keep_clocks = true,
> +       },
> +
> +       [IMX8MN_POWER_DOMAIN_OTG1] = {
> +               .bits  = {
> +                       .pxx = IMX8MN_OTG1_SW_Pxx_REQ,
> +                       .map = IMX8MN_OTG1_A53_DOMAIN,
> +               },
> +               .pgc   = BIT(IMX8MN_PGC_OTG1),
> +       },
> +};
> +
> +static const struct imx_pgc_domain_data imx8mn_pgc_domain_data = {
> +       .domains = imx8mn_pgc_domains,
> +       .domains_num = ARRAY_SIZE(imx8mn_pgc_domains),
> +       .pgc_regs = &imx7_pgc_regs,
> +};
> +#endif
> +
>  static int imx8m_power_domain_on(struct power_domain *power_domain)
>  {
>         struct udevice *dev = power_domain->dev;
> -       struct imx8m_power_domain_plat *pdata;
> +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       const struct imx_pgc_domain *domain = pdata->domain;
> +       const struct imx_pgc_regs *regs = pdata->regs;
> +       void __iomem *base = pdata->base;
> +       u32 pgc;
> +       int ret;
> +
> +       if (pdata->clk.count) {
> +               ret = clk_enable_bulk(&pdata->clk);
> +               if (ret) {
> +                       dev_err(dev, "failed to enable reset clocks\n");
> +                       return ret;
> +               }
> +       }
>
> -       pdata = dev_get_plat(dev);
> +       if (domain->bits.pxx) {
> +               /* request the domain to power up */
> +               setbits_le32(base + regs->pup, domain->bits.pxx);
>
> -       if (pdata->resource_id < 0)
> -               return -EINVAL;
> +               /*
> +                * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +                * for PUP_REQ/PDN_REQ bit to be cleared
> +                */
> +               ret = wait_for_bit_le32(base + regs->pup, domain->bits.pxx,
> +                                       false, 1000, false);
> +               if (ret) {
> +                       dev_err(dev, "failed to command PGC\n");
> +                       goto out_clk_disable;
> +               }
>
> -       if (pdata->has_pd)
> -               power_domain_on(&pdata->pd);
> +               /* disable power control */
> +               for_each_set_bit(pgc, &domain->pgc, 32) {
> +                       clrbits_le32(base + GPC_PGC_CTRL(pgc),
> +                                    GPC_PGC_CTRL_PCR);
> +               }
> +       }
> +
> +       /* delay for reset to propagate */
> +       udelay(5);
>
> -       arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
> -                     pdata->resource_id, 1, 0, 0, 0, 0, NULL);
> +       /* request the ADB400 to power up */
> +       if (domain->bits.hskreq)
> +               setbits_le32(base + regs->hsk, domain->bits.hskreq);
> +
> +       /* Disable reset clocks for all devices in the domain */
> +       if (!domain->keep_clocks && pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
>
>         return 0;
> +
> +out_clk_disable:
> +       if (pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
> +       return ret;
>  }
>
>  static int imx8m_power_domain_off(struct power_domain *power_domain)
>  {
>         struct udevice *dev = power_domain->dev;
> -       struct imx8m_power_domain_plat *pdata;
> -       pdata = dev_get_plat(dev);
> +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       const struct imx_pgc_domain *domain = pdata->domain;
> +       const struct imx_pgc_regs *regs = pdata->regs;
> +       void __iomem *base = pdata->base;
> +       u32 pgc;
> +       int ret;
>
> -       if (pdata->resource_id < 0)
> -               return -EINVAL;
> +       /* Enable reset clocks for all devices in the domain */
> +       if (!domain->keep_clocks && pdata->clk.count) {
> +               ret = clk_enable_bulk(&pdata->clk);
> +               if (ret)
> +                       return ret;
> +       }
>
> -       arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
> -                     pdata->resource_id, 0, 0, 0, 0, 0, NULL);
> +       /* request the ADB400 to power down */
> +       if (domain->bits.hskreq) {
> +               clrbits_le32(base + regs->hsk, domain->bits.hskreq);
> +
> +               ret = wait_for_bit_le32(base + regs->hsk, domain->bits.hskack,
> +                                       false, 1000, false);
> +               if (ret) {
> +                       dev_err(dev, "failed to power down ADB400\n");
> +                       goto out_clk_disable;
> +               }
> +       }
> +
> +       if (domain->bits.pxx) {
> +               /* enable power control */
> +               for_each_set_bit(pgc, &domain->pgc, 32) {
> +                       setbits_le32(base + GPC_PGC_CTRL(pgc),
> +                                    GPC_PGC_CTRL_PCR);
> +               }
> +
> +               /* request the domain to power down */
> +               setbits_le32(base + regs->pdn, domain->bits.pxx);
> +
> +               /*
> +                * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +                * for PUP_REQ/PDN_REQ bit to be cleared
> +                */
> +               ret = wait_for_bit_le32(base + regs->pdn, domain->bits.pxx,
> +                                       false, 1000, false);
> +               if (ret) {
> +                       dev_err(dev, "failed to command PGC\n");
> +                       goto out_clk_disable;
> +               }
> +       }
> +
> +       /* Disable reset clocks for all devices in the domain */
> +       if (pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
>
>         if (pdata->has_pd)
>                 power_domain_off(&pdata->pd);
>
>         return 0;
> +
> +out_clk_disable:
> +       if (!domain->keep_clocks && pdata->clk.count)
> +               clk_disable_bulk(&pdata->clk);
> +
> +       return ret;
>  }
>
>  static int imx8m_power_domain_of_xlate(struct power_domain *power_domain,
> @@ -101,12 +411,36 @@ static int imx8m_power_domain_bind(struct udevice *dev)
>         return 0;
>  }
>
> +static int imx8m_power_domain_probe(struct udevice *dev)
> +{
> +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       int ret;
> +
> +       /* Nothing to do for non-"power-domain" driver instances. */
> +       if (!strstr(dev->name, "power-domain"))
> +               return 0;
> +
> +       /* Grab optional power domain clock. */
> +       ret = clk_get_bulk(dev, &pdata->clk);
> +       if (ret && ret != -ENOENT) {
> +               dev_err(dev, "Failed to get domain clock (%d)\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  static int imx8m_power_domain_of_to_plat(struct udevice *dev)
>  {
>         struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> +       struct imx_pgc_domain_data *domain_data =
> +               (struct imx_pgc_domain_data *)dev_get_driver_data(dev);
>
>         pdata->resource_id = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
>                                             "reg", -1);
> +       pdata->domain = &domain_data->domains[pdata->resource_id];
> +       pdata->regs = domain_data->pgc_regs;
> +       pdata->base = dev_read_addr_ptr(dev->parent);
>
>         if (!power_domain_get(dev, &pdata->pd))
>                 pdata->has_pd = 1;
> @@ -115,9 +449,15 @@ static int imx8m_power_domain_of_to_plat(struct udevice *dev)
>  }
>
>  static const struct udevice_id imx8m_power_domain_ids[] = {
> -       { .compatible = "fsl,imx8mq-gpc" },
> -       { .compatible = "fsl,imx8mm-gpc" },
> -       { .compatible = "fsl,imx8mn-gpc" },
> +#ifdef CONFIG_IMX8MQ
> +       { .compatible = "fsl,imx8mq-gpc", .data = (long)&imx8m_pgc_domain_data },
> +#endif
> +#ifdef CONFIG_IMX8MM
> +       { .compatible = "fsl,imx8mm-gpc", .data = (long)&imx8mm_pgc_domain_data },
> +#endif
> +#ifdef CONFIG_IMX8MN
> +       { .compatible = "fsl,imx8mn-gpc", .data = (long)&imx8mn_pgc_domain_data },
> +#endif
>         { }
>  };
>
> @@ -132,6 +472,7 @@ U_BOOT_DRIVER(imx8m_power_domain) = {
>         .id = UCLASS_POWER_DOMAIN,
>         .of_match = imx8m_power_domain_ids,
>         .bind = imx8m_power_domain_bind,
> +       .probe = imx8m_power_domain_probe,
>         .of_to_plat = imx8m_power_domain_of_to_plat,
>         .plat_auto      = sizeof(struct imx8m_power_domain_plat),
>         .ops = &imx8m_power_domain_ops,
> --
> 2.35.1
>

Marek,

I was able to test this without any issue on IMX8MM/IMX8MN/IMX8MP. I
tested with NXP ATF lf_v2.4.

Tested-By: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x
#imx8mn-venice-gw7902 #imx8mp-venice-gw74xx

Best Regards,

Tim

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Thu, Apr 7, 2022 at 5:21 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Wed, Mar 30, 2022 at 8:04 PM Marek Vasut <marex@denx.de> wrote:
> >
> > This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> > the driver based on Linux GPCv2 driver to directly control the GPCv2
> > block instead of using SMCCC calls. This way, U-Boot can operate the
> > i.MX8M power domains without depending on anything else.
> >
> > This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> > domains are not supported to save space, since they are not useful in
> > the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: Peng Fan <peng.fan@nxp.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
> > NOTE: So far this is tested on MX8MM and MX8MN. MX8MQ is not tested.

Sorry for the delay, I tested positive for Covid and needed to recover a bit.

I have confirmed this makes the 8MQ hang during boot.

Before patch series:

U-Boot SPL 2022.04-00812-g9859465bfe (Apr 16 2022 - 15:41:46 -0500)
PMIC:  PFUZE100 ID=0x10
SEC0:  RNG instantiated
Normal Boot
Trying to boot from MMC2


U-Boot 2022.04-00812-g9859465bfe (Apr 16 2022 - 15:41:46 -0500)

CPU:   Freescale i.MX8MQ rev2.1 at 1000 MHz
Reset cause: POR
Model: NXP i.MX8MQ EVK
DRAM:  3 GiB
Core:  59 devices, 16 uclasses, devicetree: separate
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
Loading Environment from MMC... *** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
SEC0:  RNG instantiated
Net:   eth0: ethernet@30be0000
Hit any key to stop autoboot:  0
u-boot=>

After the patch:

U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
PMIC:  PFUZE100 ID=0x10
SEC0:  RNG instantiated
Normal Boot
Trying to boot from MMC2

<hang>

I can test it on 8MN and 8MM, but it sounds like that's already been done.

adam


> > ---
> >  drivers/power/domain/Kconfig              |   1 +
> >  drivers/power/domain/imx8m-power-domain.c | 379 ++++++++++++++++++++--
> >  2 files changed, 361 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
> > index 93d2599d83c..04fc0054323 100644
> > --- a/drivers/power/domain/Kconfig
> > +++ b/drivers/power/domain/Kconfig
> > @@ -35,6 +35,7 @@ config IMX8_POWER_DOMAIN
> >  config IMX8M_POWER_DOMAIN
> >         bool "Enable i.MX8M power domain driver"
> >         depends on POWER_DOMAIN && ARCH_IMX8M
> > +       select CLK
> >         help
> >           Enable support for manipulating NXP i.MX8M on-SoC power domains via
> >           requests to the ATF.
> > diff --git a/drivers/power/domain/imx8m-power-domain.c b/drivers/power/domain/imx8m-power-domain.c
> > index c32dbcc31ae..e2e41cf5fee 100644
> > --- a/drivers/power/domain/imx8m-power-domain.c
> > +++ b/drivers/power/domain/imx8m-power-domain.c
> > @@ -4,6 +4,7 @@
> >   */
> >
> >  #include <common.h>
> > +#include <clk.h>
> >  #include <dm.h>
> >  #include <malloc.h>
> >  #include <power-domain-uclass.h>
> > @@ -12,52 +13,361 @@
> >  #include <asm/mach-imx/sys_proto.h>
> >  #include <dm/device-internal.h>
> >  #include <dm/device.h>
> > +#include <dm/device_compat.h>
> >  #include <imx_sip.h>
> > -#include <linux/arm-smccc.h>
> > +#include <linux/bitmap.h>
> > +#include <wait_bit.h>
> > +
> > +#include <dt-bindings/power/imx8mm-power.h>
> > +#include <dt-bindings/power/imx8mn-power.h>
> > +#include <dt-bindings/power/imx8mq-power.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > +#define GPC_PGC_CPU_MAPPING                    0x0ec
> > +
> > +#define IMX8M_PCIE2_A53_DOMAIN                 BIT(15)
> > +#define IMX8M_OTG2_A53_DOMAIN                  BIT(5)
> > +#define IMX8M_OTG1_A53_DOMAIN                  BIT(4)
> > +#define IMX8M_PCIE1_A53_DOMAIN                 BIT(3)
> > +
> > +#define IMX8MM_OTG2_A53_DOMAIN                 BIT(5)
> > +#define IMX8MM_OTG1_A53_DOMAIN                 BIT(4)
> > +#define IMX8MM_PCIE_A53_DOMAIN                 BIT(3)
> > +
> > +#define IMX8MN_OTG1_A53_DOMAIN                 BIT(4)
> > +#define IMX8MN_MIPI_A53_DOMAIN                 BIT(2)
> > +
> > +#define GPC_PU_PGC_SW_PUP_REQ                  0x0f8
> > +#define GPC_PU_PGC_SW_PDN_REQ                  0x104
> > +
> > +#define IMX8M_PCIE2_SW_Pxx_REQ                 BIT(13)
> > +#define IMX8M_OTG2_SW_Pxx_REQ                  BIT(3)
> > +#define IMX8M_OTG1_SW_Pxx_REQ                  BIT(2)
> > +#define IMX8M_PCIE1_SW_Pxx_REQ                 BIT(1)
> > +
> > +#define IMX8MM_OTG2_SW_Pxx_REQ                 BIT(3)
> > +#define IMX8MM_OTG1_SW_Pxx_REQ                 BIT(2)
> > +#define IMX8MM_PCIE_SW_Pxx_REQ                 BIT(1)
> > +
> > +#define IMX8MN_OTG1_SW_Pxx_REQ                 BIT(2)
> > +#define IMX8MN_MIPI_SW_Pxx_REQ                 BIT(0)
> > +
> > +#define GPC_M4_PU_PDN_FLG                      0x1bc
> > +
> > +#define GPC_PU_PWRHSK                          0x1fc
> > +
> > +#define IMX8MM_HSIO_HSK_PWRDNACKN              (BIT(23) | BIT(24))
> > +#define IMX8MM_HSIO_HSK_PWRDNREQN              (BIT(5) | BIT(6))
> > +
> > +#define IMX8MN_HSIO_HSK_PWRDNACKN              BIT(23)
> > +#define IMX8MN_HSIO_HSK_PWRDNREQN              BIT(5)
> > +
> > +/*
> > + * The PGC offset values in Reference Manual
> > + * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> > + * GPC_PGC memory map are incorrect, below offset
> > + * values are from design RTL.
> > + */
> > +#define IMX8M_PGC_PCIE1                        17
> > +#define IMX8M_PGC_OTG1                 18
> > +#define IMX8M_PGC_OTG2                 19
> > +#define IMX8M_PGC_PCIE2                        29
> > +
> > +#define IMX8MM_PGC_PCIE                        17
> > +#define IMX8MM_PGC_OTG1                        18
> > +#define IMX8MM_PGC_OTG2                        19
> > +
> > +#define IMX8MN_PGC_OTG1                        18
> > +
> > +#define GPC_PGC_CTRL(n)                        (0x800 + (n) * 0x40)
> > +#define GPC_PGC_SR(n)                  (GPC_PGC_CTRL(n) + 0xc)
> > +
> > +#define GPC_PGC_CTRL_PCR               BIT(0)
> > +
> > +struct imx_pgc_regs {
> > +       u16 map;
> > +       u16 pup;
> > +       u16 pdn;
> > +       u16 hsk;
> > +};
> > +
> > +struct imx_pgc_domain {
> > +       unsigned long pgc;
> > +
> > +       const struct {
> > +               u32 pxx;
> > +               u32 map;
> > +               u32 hskreq;
> > +               u32 hskack;
> > +       } bits;
> > +
> > +       const bool keep_clocks;
> > +};
> > +
> > +struct imx_pgc_domain_data {
> > +       const struct imx_pgc_domain *domains;
> > +       size_t domains_num;
> > +       const struct imx_pgc_regs *pgc_regs;
> > +};
> > +
> >  struct imx8m_power_domain_plat {
> > +       struct power_domain pd;
> > +       const struct imx_pgc_domain *domain;
> > +       const struct imx_pgc_regs *regs;
> > +       struct clk_bulk clk;
> > +       void __iomem *base;
> >         int resource_id;
> >         int has_pd;
> > -       struct power_domain pd;
> >  };
> >
> > +#if defined(CONFIG_IMX8MM) || defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MQ)
> > +static const struct imx_pgc_regs imx7_pgc_regs = {
> > +       .map = GPC_PGC_CPU_MAPPING,
> > +       .pup = GPC_PU_PGC_SW_PUP_REQ,
> > +       .pdn = GPC_PU_PGC_SW_PDN_REQ,
> > +       .hsk = GPC_PU_PWRHSK,
> > +};
> > +#endif
> > +
> > +#ifdef CONFIG_IMX8MQ
> > +static const struct imx_pgc_domain imx8m_pgc_domains[] = {
> > +       [IMX8M_POWER_DOMAIN_PCIE1] = {
> > +               .bits  = {
> > +                       .pxx = IMX8M_PCIE1_SW_Pxx_REQ,
> > +                       .map = IMX8M_PCIE1_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8M_PGC_PCIE1),
> > +       },
> > +
> > +       [IMX8M_POWER_DOMAIN_USB_OTG1] = {
> > +               .bits  = {
> > +                       .pxx = IMX8M_OTG1_SW_Pxx_REQ,
> > +                       .map = IMX8M_OTG1_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8M_PGC_OTG1),
> > +       },
> > +
> > +       [IMX8M_POWER_DOMAIN_USB_OTG2] = {
> > +               .bits  = {
> > +                       .pxx = IMX8M_OTG2_SW_Pxx_REQ,
> > +                       .map = IMX8M_OTG2_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8M_PGC_OTG2),
> > +       },
> > +
> > +       [IMX8M_POWER_DOMAIN_PCIE2] = {
> > +               .bits  = {
> > +                       .pxx = IMX8M_PCIE2_SW_Pxx_REQ,
> > +                       .map = IMX8M_PCIE2_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8M_PGC_PCIE2),
> > +       },
> > +};
> > +
> > +static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
> > +       .domains = imx8m_pgc_domains,
> > +       .domains_num = ARRAY_SIZE(imx8m_pgc_domains),
> > +       .pgc_regs = &imx7_pgc_regs,
> > +};
> > +#endif
> > +
> > +#ifdef CONFIG_IMX8MM
> > +static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> > +       [IMX8MM_POWER_DOMAIN_HSIOMIX] = {
> > +               .bits  = {
> > +                       .pxx = 0, /* no power sequence control */
> > +                       .map = 0, /* no power sequence control */
> > +                       .hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
> > +                       .hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
> > +               },
> > +               .keep_clocks = true,
> > +       },
> > +
> > +       [IMX8MM_POWER_DOMAIN_PCIE] = {
> > +               .bits  = {
> > +                       .pxx = IMX8MM_PCIE_SW_Pxx_REQ,
> > +                       .map = IMX8MM_PCIE_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8MM_PGC_PCIE),
> > +       },
> > +
> > +       [IMX8MM_POWER_DOMAIN_OTG1] = {
> > +               .bits  = {
> > +                       .pxx = IMX8MM_OTG1_SW_Pxx_REQ,
> > +                       .map = IMX8MM_OTG1_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8MM_PGC_OTG1),
> > +       },
> > +
> > +       [IMX8MM_POWER_DOMAIN_OTG2] = {
> > +               .bits  = {
> > +                       .pxx = IMX8MM_OTG2_SW_Pxx_REQ,
> > +                       .map = IMX8MM_OTG2_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8MM_PGC_OTG2),
> > +       },
> > +};
> > +
> > +static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
> > +       .domains = imx8mm_pgc_domains,
> > +       .domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
> > +       .pgc_regs = &imx7_pgc_regs,
> > +};
> > +#endif
> > +
> > +#ifdef CONFIG_IMX8MN
> > +static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
> > +       [IMX8MN_POWER_DOMAIN_HSIOMIX] = {
> > +               .bits  = {
> > +                       .pxx = 0, /* no power sequence control */
> > +                       .map = 0, /* no power sequence control */
> > +                       .hskreq = IMX8MN_HSIO_HSK_PWRDNREQN,
> > +                       .hskack = IMX8MN_HSIO_HSK_PWRDNACKN,
> > +               },
> > +               .keep_clocks = true,
> > +       },
> > +
> > +       [IMX8MN_POWER_DOMAIN_OTG1] = {
> > +               .bits  = {
> > +                       .pxx = IMX8MN_OTG1_SW_Pxx_REQ,
> > +                       .map = IMX8MN_OTG1_A53_DOMAIN,
> > +               },
> > +               .pgc   = BIT(IMX8MN_PGC_OTG1),
> > +       },
> > +};
> > +
> > +static const struct imx_pgc_domain_data imx8mn_pgc_domain_data = {
> > +       .domains = imx8mn_pgc_domains,
> > +       .domains_num = ARRAY_SIZE(imx8mn_pgc_domains),
> > +       .pgc_regs = &imx7_pgc_regs,
> > +};
> > +#endif
> > +
> >  static int imx8m_power_domain_on(struct power_domain *power_domain)
> >  {
> >         struct udevice *dev = power_domain->dev;
> > -       struct imx8m_power_domain_plat *pdata;
> > +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> > +       const struct imx_pgc_domain *domain = pdata->domain;
> > +       const struct imx_pgc_regs *regs = pdata->regs;
> > +       void __iomem *base = pdata->base;
> > +       u32 pgc;
> > +       int ret;
> > +
> > +       if (pdata->clk.count) {
> > +               ret = clk_enable_bulk(&pdata->clk);
> > +               if (ret) {
> > +                       dev_err(dev, "failed to enable reset clocks\n");
> > +                       return ret;
> > +               }
> > +       }
> >
> > -       pdata = dev_get_plat(dev);
> > +       if (domain->bits.pxx) {
> > +               /* request the domain to power up */
> > +               setbits_le32(base + regs->pup, domain->bits.pxx);
> >
> > -       if (pdata->resource_id < 0)
> > -               return -EINVAL;
> > +               /*
> > +                * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> > +                * for PUP_REQ/PDN_REQ bit to be cleared
> > +                */
> > +               ret = wait_for_bit_le32(base + regs->pup, domain->bits.pxx,
> > +                                       false, 1000, false);
> > +               if (ret) {
> > +                       dev_err(dev, "failed to command PGC\n");
> > +                       goto out_clk_disable;
> > +               }
> >
> > -       if (pdata->has_pd)
> > -               power_domain_on(&pdata->pd);
> > +               /* disable power control */
> > +               for_each_set_bit(pgc, &domain->pgc, 32) {
> > +                       clrbits_le32(base + GPC_PGC_CTRL(pgc),
> > +                                    GPC_PGC_CTRL_PCR);
> > +               }
> > +       }
> > +
> > +       /* delay for reset to propagate */
> > +       udelay(5);
> >
> > -       arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
> > -                     pdata->resource_id, 1, 0, 0, 0, 0, NULL);
> > +       /* request the ADB400 to power up */
> > +       if (domain->bits.hskreq)
> > +               setbits_le32(base + regs->hsk, domain->bits.hskreq);
> > +
> > +       /* Disable reset clocks for all devices in the domain */
> > +       if (!domain->keep_clocks && pdata->clk.count)
> > +               clk_disable_bulk(&pdata->clk);
> >
> >         return 0;
> > +
> > +out_clk_disable:
> > +       if (pdata->clk.count)
> > +               clk_disable_bulk(&pdata->clk);
> > +       return ret;
> >  }
> >
> >  static int imx8m_power_domain_off(struct power_domain *power_domain)
> >  {
> >         struct udevice *dev = power_domain->dev;
> > -       struct imx8m_power_domain_plat *pdata;
> > -       pdata = dev_get_plat(dev);
> > +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> > +       const struct imx_pgc_domain *domain = pdata->domain;
> > +       const struct imx_pgc_regs *regs = pdata->regs;
> > +       void __iomem *base = pdata->base;
> > +       u32 pgc;
> > +       int ret;
> >
> > -       if (pdata->resource_id < 0)
> > -               return -EINVAL;
> > +       /* Enable reset clocks for all devices in the domain */
> > +       if (!domain->keep_clocks && pdata->clk.count) {
> > +               ret = clk_enable_bulk(&pdata->clk);
> > +               if (ret)
> > +                       return ret;
> > +       }
> >
> > -       arm_smccc_smc(IMX_SIP_GPC, IMX_SIP_GPC_PM_DOMAIN,
> > -                     pdata->resource_id, 0, 0, 0, 0, 0, NULL);
> > +       /* request the ADB400 to power down */
> > +       if (domain->bits.hskreq) {
> > +               clrbits_le32(base + regs->hsk, domain->bits.hskreq);
> > +
> > +               ret = wait_for_bit_le32(base + regs->hsk, domain->bits.hskack,
> > +                                       false, 1000, false);
> > +               if (ret) {
> > +                       dev_err(dev, "failed to power down ADB400\n");
> > +                       goto out_clk_disable;
> > +               }
> > +       }
> > +
> > +       if (domain->bits.pxx) {
> > +               /* enable power control */
> > +               for_each_set_bit(pgc, &domain->pgc, 32) {
> > +                       setbits_le32(base + GPC_PGC_CTRL(pgc),
> > +                                    GPC_PGC_CTRL_PCR);
> > +               }
> > +
> > +               /* request the domain to power down */
> > +               setbits_le32(base + regs->pdn, domain->bits.pxx);
> > +
> > +               /*
> > +                * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> > +                * for PUP_REQ/PDN_REQ bit to be cleared
> > +                */
> > +               ret = wait_for_bit_le32(base + regs->pdn, domain->bits.pxx,
> > +                                       false, 1000, false);
> > +               if (ret) {
> > +                       dev_err(dev, "failed to command PGC\n");
> > +                       goto out_clk_disable;
> > +               }
> > +       }
> > +
> > +       /* Disable reset clocks for all devices in the domain */
> > +       if (pdata->clk.count)
> > +               clk_disable_bulk(&pdata->clk);
> >
> >         if (pdata->has_pd)
> >                 power_domain_off(&pdata->pd);
> >
> >         return 0;
> > +
> > +out_clk_disable:
> > +       if (!domain->keep_clocks && pdata->clk.count)
> > +               clk_disable_bulk(&pdata->clk);
> > +
> > +       return ret;
> >  }
> >
> >  static int imx8m_power_domain_of_xlate(struct power_domain *power_domain,
> > @@ -101,12 +411,36 @@ static int imx8m_power_domain_bind(struct udevice *dev)
> >         return 0;
> >  }
> >
> > +static int imx8m_power_domain_probe(struct udevice *dev)
> > +{
> > +       struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> > +       int ret;
> > +
> > +       /* Nothing to do for non-"power-domain" driver instances. */
> > +       if (!strstr(dev->name, "power-domain"))
> > +               return 0;
> > +
> > +       /* Grab optional power domain clock. */
> > +       ret = clk_get_bulk(dev, &pdata->clk);
> > +       if (ret && ret != -ENOENT) {
> > +               dev_err(dev, "Failed to get domain clock (%d)\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static int imx8m_power_domain_of_to_plat(struct udevice *dev)
> >  {
> >         struct imx8m_power_domain_plat *pdata = dev_get_plat(dev);
> > +       struct imx_pgc_domain_data *domain_data =
> > +               (struct imx_pgc_domain_data *)dev_get_driver_data(dev);
> >
> >         pdata->resource_id = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
> >                                             "reg", -1);
> > +       pdata->domain = &domain_data->domains[pdata->resource_id];
> > +       pdata->regs = domain_data->pgc_regs;
> > +       pdata->base = dev_read_addr_ptr(dev->parent);
> >
> >         if (!power_domain_get(dev, &pdata->pd))
> >                 pdata->has_pd = 1;
> > @@ -115,9 +449,15 @@ static int imx8m_power_domain_of_to_plat(struct udevice *dev)
> >  }
> >
> >  static const struct udevice_id imx8m_power_domain_ids[] = {
> > -       { .compatible = "fsl,imx8mq-gpc" },
> > -       { .compatible = "fsl,imx8mm-gpc" },
> > -       { .compatible = "fsl,imx8mn-gpc" },
> > +#ifdef CONFIG_IMX8MQ
> > +       { .compatible = "fsl,imx8mq-gpc", .data = (long)&imx8m_pgc_domain_data },
> > +#endif
> > +#ifdef CONFIG_IMX8MM
> > +       { .compatible = "fsl,imx8mm-gpc", .data = (long)&imx8mm_pgc_domain_data },
> > +#endif
> > +#ifdef CONFIG_IMX8MN
> > +       { .compatible = "fsl,imx8mn-gpc", .data = (long)&imx8mn_pgc_domain_data },
> > +#endif
> >         { }
> >  };
> >
> > @@ -132,6 +472,7 @@ U_BOOT_DRIVER(imx8m_power_domain) = {
> >         .id = UCLASS_POWER_DOMAIN,
> >         .of_match = imx8m_power_domain_ids,
> >         .bind = imx8m_power_domain_bind,
> > +       .probe = imx8m_power_domain_probe,
> >         .of_to_plat = imx8m_power_domain_of_to_plat,
> >         .plat_auto      = sizeof(struct imx8m_power_domain_plat),
> >         .ops = &imx8m_power_domain_ops,
> > --
> > 2.35.1
> >
>
> Marek,
>
> I was able to test this without any issue on IMX8MM/IMX8MN/IMX8MP. I
> tested with NXP ATF lf_v2.4.
>
> Tested-By: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x
> #imx8mn-venice-gw7902 #imx8mp-venice-gw74xx
>
> Best Regards,
>
> Tim

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/16/22 22:49, Adam Ford wrote:
> On Thu, Apr 7, 2022 at 5:21 PM Tim Harvey <tharvey@gateworks.com> wrote:
>>
>> On Wed, Mar 30, 2022 at 8:04 PM Marek Vasut <marex@denx.de> wrote:
>>>
>>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
>>> the driver based on Linux GPCv2 driver to directly control the GPCv2
>>> block instead of using SMCCC calls. This way, U-Boot can operate the
>>> i.MX8M power domains without depending on anything else.
>>>
>>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
>>> domains are not supported to save space, since they are not useful in
>>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Peng Fan <peng.fan@nxp.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> ---
>>> NOTE: So far this is tested on MX8MM and MX8MN. MX8MQ is not tested.
> 
> Sorry for the delay, I tested positive for Covid and needed to recover a bit.
> 
> I have confirmed this makes the 8MQ hang during boot.
> 
> Before patch series:
> 
> U-Boot SPL 2022.04-00812-g9859465bfe (Apr 16 2022 - 15:41:46 -0500)
> PMIC:  PFUZE100 ID=0x10
> SEC0:  RNG instantiated
> Normal Boot
> Trying to boot from MMC2
> 
> 
> U-Boot 2022.04-00812-g9859465bfe (Apr 16 2022 - 15:41:46 -0500)
> 
> CPU:   Freescale i.MX8MQ rev2.1 at 1000 MHz
> Reset cause: POR
> Model: NXP i.MX8MQ EVK
> DRAM:  3 GiB
> Core:  59 devices, 16 uclasses, devicetree: separate
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> Loading Environment from MMC... *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> SEC0:  RNG instantiated
> Net:   eth0: ethernet@30be0000
> Hit any key to stop autoboot:  0
> u-boot=>
> 
> After the patch:
> 
> U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
> PMIC:  PFUZE100 ID=0x10
> SEC0:  RNG instantiated
> Normal Boot
> Trying to boot from MMC2

There should be some sort of output from ATF here, which version did you 
use during your tests ?

Did you set IMX_BOOT_UART_BASE correctly ?

Did you set ATF_LOAD_ADDR correctly ?

Do you get a hang when you revert only this specific patch ?

The GPC is only used for USB and PCI on the MX8MQ, so you shouldn't be 
getting hang so early as to get no output at all.

> <hang>
> 
> I can test it on 8MN and 8MM, but it sounds like that's already been done.

[...]

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Sat, Apr 16, 2022 at 7:35 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/16/22 22:49, Adam Ford wrote:
> > On Thu, Apr 7, 2022 at 5:21 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >>
> >> On Wed, Mar 30, 2022 at 8:04 PM Marek Vasut <marex@denx.de> wrote:
> >>>
> >>> This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
> >>> the driver based on Linux GPCv2 driver to directly control the GPCv2
> >>> block instead of using SMCCC calls. This way, U-Boot can operate the
> >>> i.MX8M power domains without depending on anything else.
> >>>
> >>> This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
> >>> domains are not supported to save space, since they are not useful in
> >>> the bootloader. The only domains kept are ones for HSIO, PCIe, USB.
> >>>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Fabio Estevam <festevam@gmail.com>
> >>> Cc: Peng Fan <peng.fan@nxp.com>
> >>> Cc: Stefano Babic <sbabic@denx.de>
> >>> ---
> >>> NOTE: So far this is tested on MX8MM and MX8MN. MX8MQ is not tested.
> >
> > Sorry for the delay, I tested positive for Covid and needed to recover a bit.
> >
> > I have confirmed this makes the 8MQ hang during boot.
> >
> > Before patch series:
> >
> > U-Boot SPL 2022.04-00812-g9859465bfe (Apr 16 2022 - 15:41:46 -0500)
> > PMIC:  PFUZE100 ID=0x10
> > SEC0:  RNG instantiated
> > Normal Boot
> > Trying to boot from MMC2
> >
> >
> > U-Boot 2022.04-00812-g9859465bfe (Apr 16 2022 - 15:41:46 -0500)
> >
> > CPU:   Freescale i.MX8MQ rev2.1 at 1000 MHz
> > Reset cause: POR
> > Model: NXP i.MX8MQ EVK
> > DRAM:  3 GiB
> > Core:  59 devices, 16 uclasses, devicetree: separate
> > MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> > Loading Environment from MMC... *** Warning - bad CRC, using default environment
> >
> > In:    serial
> > Out:   serial
> > Err:   serial
> > SEC0:  RNG instantiated
> > Net:   eth0: ethernet@30be0000
> > Hit any key to stop autoboot:  0
> > u-boot=>
> >
> > After the patch:
> >
> > U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
> > PMIC:  PFUZE100 ID=0x10
> > SEC0:  RNG instantiated
> > Normal Boot
> > Trying to boot from MMC2
>
> There should be some sort of output from ATF here, which version did you
> use during your tests ?

I used the version from the imx8mq_evk.rst:

Note: srctree is U-Boot source directory
Get ATF from: https://source.codeaurora.org/external/imx/imx-atf
branch: imx_5.4.47_2.2.0

This works with U-Boot master.  I realize it has the ATF power domain
code, but with the SMCC stuff removed, it should be irrelevant.

>
> Did you set IMX_BOOT_UART_BASE correctly ?
>
> Did you set ATF_LOAD_ADDR correctly ?
>
If I didn't build it right, I don't think 'master' would have booted,
but it did.  I think I built it correctly.

> Do you get a hang when you revert only this specific patch ?

When I reverted the series, it booted again, but I can do a bisect
tomorrow to narrow down the specific patch in the series that causes
the issue.

adam
>
> The GPC is only used for USB and PCI on the MX8MQ, so you shouldn't be
> getting hang so early as to get no output at all.
>
> > <hang>
> >
> > I can test it on 8MN and 8MM, but it sounds like that's already been done.
>
> [...]

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/17/22 03:01, Adam Ford wrote:

Hi,

>>> After the patch:
>>>
>>> U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
>>> PMIC:  PFUZE100 ID=0x10
>>> SEC0:  RNG instantiated
>>> Normal Boot
>>> Trying to boot from MMC2
>>
>> There should be some sort of output from ATF here, which version did you
>> use during your tests ?
> 
> I used the version from the imx8mq_evk.rst:
> 
> Note: srctree is U-Boot source directory
> Get ATF from: https://source.codeaurora.org/external/imx/imx-atf
> branch: imx_5.4.47_2.2.0
> 
> This works with U-Boot master.  I realize it has the ATF power domain
> code, but with the SMCC stuff removed, it should be irrelevant.
> 
>>
>> Did you set IMX_BOOT_UART_BASE correctly ?
>>
>> Did you set ATF_LOAD_ADDR correctly ?
>>
> If I didn't build it right, I don't think 'master' would have booted,
> but it did.  I think I built it correctly.

I find it odd that there is no print from the ATF, do check these two 
variables, I have seen hangs in ATF like this when they were not set right.

>> Do you get a hang when you revert only this specific patch ?
> 
> When I reverted the series, it booted again, but I can do a bisect
> tomorrow to narrow down the specific patch in the series that causes
> the issue.

Please do, thanks.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Sat, Apr 16, 2022 at 9:13 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/17/22 03:01, Adam Ford wrote:
>
> Hi,
>
> >>> After the patch:
> >>>
> >>> U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
> >>> PMIC:  PFUZE100 ID=0x10
> >>> SEC0:  RNG instantiated
> >>> Normal Boot
> >>> Trying to boot from MMC2
> >>
> >> There should be some sort of output from ATF here, which version did you
> >> use during your tests ?

When I look at the ATF from NXP, it appears the debug console is
disabled by default.  I think it's because the 8MQ has less OCRAM than
the rest of the 8M family, but I don't know.

> >
> > I used the version from the imx8mq_evk.rst:
> >
> > Note: srctree is U-Boot source directory
> > Get ATF from: https://source.codeaurora.org/external/imx/imx-atf
> > branch: imx_5.4.47_2.2.0
> >
> > This works with U-Boot master.  I realize it has the ATF power domain
> > code, but with the SMCC stuff removed, it should be irrelevant.
> >
> >>
> >> Did you set IMX_BOOT_UART_BASE correctly ?
> >>

0x30860000

> >> Did you set ATF_LOAD_ADDR correctly ?

ATF_LOAD_ADDR=0x00910000

> >>
> > If I didn't build it right, I don't think 'master' would have booted,
> > but it did.  I think I built it correctly.
>
> I find it odd that there is no print from the ATF, do check these twoU-Boot SPL 2022.04-00822-ge5aeb301b2 (Apr 17 2022 - 08:18:32 -0500)

With the console enabled and your patch series applied:

PMIC:  PFUZE100 ID=0x10
SEC0:  RNG instantiated
Normal Boot
Trying to boot from MMC2
NOTICE:  BL31: v2.2(release):rel_imx_5.4.47_2.2.0-0-gc949a888e-dirty
NOTICE:  BL31: Built : 08:16:33, Apr 17 2022

<hang>

It appears to me that both SPL and ATF are running.

See below for bisect conversation...
> variables, I have seen hangs in ATF like this when they were not set right.
>
> >> Do you get a hang when you revert only this specific patch ?
> >
> > When I reverted the series, it booted again, but I can do a bisect
> > tomorrow to narrow down the specific patch in the series that causes
> > the issue.
>
> Please do, thanks.

28e5debc019b347436bdebd8978a971ce5a6582c is the first bad commit
commit 28e5debc019b347436bdebd8978a971ce5a6582c
Author: Marek Vasut <marex@denx.de>
Date:   Wed Apr 13 00:42:51 2022 +0200

    imx: power-domain: Get rid of SMCCC dependency

    This driver is the only SMCCC dependency in iMX8M U-Boot port. Rework
    the driver based on Linux GPCv2 driver to directly control the GPCv2
    block instead of using SMCCC calls. This way, U-Boot can operate the
    i.MX8M power domains without depending on anything else.

    This is losely based on Linux GPCv2 driver. The GPU, VPU, MIPI power
    domains are not supported to save space, since they are not useful in
    the bootloader. The only domains kept are ones for HSIO, PCIe, USB.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/17/22 15:21, Adam Ford wrote:
> On Sat, Apr 16, 2022 at 9:13 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 4/17/22 03:01, Adam Ford wrote:
>>
>> Hi,
>>
>>>>> After the patch:
>>>>>
>>>>> U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
>>>>> PMIC:  PFUZE100 ID=0x10
>>>>> SEC0:  RNG instantiated
>>>>> Normal Boot
>>>>> Trying to boot from MMC2
>>>>
>>>> There should be some sort of output from ATF here, which version did you
>>>> use during your tests ?
> 
> When I look at the ATF from NXP, it appears the debug console is
> disabled by default.  I think it's because the 8MQ has less OCRAM than
> the rest of the 8M family, but I don't know.

Hum, I see. I never used the NXP fork, I only ever used upstream.

>>> I used the version from the imx8mq_evk.rst:
>>>
>>> Note: srctree is U-Boot source directory
>>> Get ATF from: https://source.codeaurora.org/external/imx/imx-atf
>>> branch: imx_5.4.47_2.2.0
>>>
>>> This works with U-Boot master.  I realize it has the ATF power domain
>>> code, but with the SMCC stuff removed, it should be irrelevant.
>>>
>>>>
>>>> Did you set IMX_BOOT_UART_BASE correctly ?
>>>>
> 
> 0x30860000
> 
>>>> Did you set ATF_LOAD_ADDR correctly ?
> 
> ATF_LOAD_ADDR=0x00910000

This should be OK.

>>> If I didn't build it right, I don't think 'master' would have booted,
>>> but it did.  I think I built it correctly.
>>
>> I find it odd that there is no print from the ATF, do check these twoU-Boot SPL 2022.04-00822-ge5aeb301b2 (Apr 17 2022 - 08:18:32 -0500)
> 
> With the console enabled and your patch series applied:
> 
> PMIC:  PFUZE100 ID=0x10
> SEC0:  RNG instantiated
> Normal Boot
> Trying to boot from MMC2
> NOTICE:  BL31: v2.2(release):rel_imx_5.4.47_2.2.0-0-gc949a888e-dirty
> NOTICE:  BL31: Built : 08:16:33, Apr 17 2022
> 
> <hang>
> 
> It appears to me that both SPL and ATF are running.
> 
> See below for bisect conversation...
>> variables, I have seen hangs in ATF like this when they were not set right.
>>
>>>> Do you get a hang when you revert only this specific patch ?
>>>
>>> When I reverted the series, it booted again, but I can do a bisect
>>> tomorrow to narrow down the specific patch in the series that causes
>>> the issue.
>>
>> Please do, thanks.
> 
> 28e5debc019b347436bdebd8978a971ce5a6582c is the first bad commit
> commit 28e5debc019b347436bdebd8978a971ce5a6582c

This commit does not exist in upstream ?

u-boot$ git describe 28e5debc019b347436bdebd8978a971ce5a6582c
fatal: 28e5debc019b347436bdebd8978a971ce5a6582c is neither a commit nor blob

Can you try and force-deselect CONFIG_CLK=y (and possibly the same for 
SPL) ? I think that gets pulled in by this commit too, it might've not 
been selected on your board before. And if that's not easily possible, 
try and revert these three:

129f5102d29 ("clk: imx8m: reduce rate table duplication")
11c8ab01f3e ("clk: imx8mq: Add a clock driver for the imx8mq")
a375c6f3fbe ("dt-bindings: imx8mq-clock: add mainline definitions")

I would like to know whether pulling in the clock support might be the 
actual source of problems.

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Sun, Apr 17, 2022 at 10:16 AM Marek Vasut <marex@denx.de> wrote:
>
> On 4/17/22 15:21, Adam Ford wrote:
> > On Sat, Apr 16, 2022 at 9:13 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 4/17/22 03:01, Adam Ford wrote:
> >>
> >> Hi,
> >>
> >>>>> After the patch:
> >>>>>
> >>>>> U-Boot SPL 2022.04-00822-gc2a24a7ce5 (Apr 16 2022 - 15:45:29 -0500)
> >>>>> PMIC:  PFUZE100 ID=0x10
> >>>>> SEC0:  RNG instantiated
> >>>>> Normal Boot
> >>>>> Trying to boot from MMC2
> >>>>
> >>>> There should be some sort of output from ATF here, which version did you
> >>>> use during your tests ?
> >
> > When I look at the ATF from NXP, it appears the debug console is
> > disabled by default.  I think it's because the 8MQ has less OCRAM than
> > the rest of the 8M family, but I don't know.
>
> Hum, I see. I never used the NXP fork, I only ever used upstream.
>
> >>> I used the version from the imx8mq_evk.rst:
> >>>
> >>> Note: srctree is U-Boot source directory
> >>> Get ATF from: https://source.codeaurora.org/external/imx/imx-atf
> >>> branch: imx_5.4.47_2.2.0
> >>>
> >>> This works with U-Boot master.  I realize it has the ATF power domain
> >>> code, but with the SMCC stuff removed, it should be irrelevant.
> >>>
> >>>>
> >>>> Did you set IMX_BOOT_UART_BASE correctly ?
> >>>>
> >
> > 0x30860000
> >
> >>>> Did you set ATF_LOAD_ADDR correctly ?
> >
> > ATF_LOAD_ADDR=0x00910000
>
> This should be OK.
>
> >>> If I didn't build it right, I don't think 'master' would have booted,
> >>> but it did.  I think I built it correctly.
> >>
> >> I find it odd that there is no print from the ATF, do check these twoU-Boot SPL 2022.04-00822-ge5aeb301b2 (Apr 17 2022 - 08:18:32 -0500)
> >
> > With the console enabled and your patch series applied:
> >
> > PMIC:  PFUZE100 ID=0x10
> > SEC0:  RNG instantiated
> > Normal Boot
> > Trying to boot from MMC2
> > NOTICE:  BL31: v2.2(release):rel_imx_5.4.47_2.2.0-0-gc949a888e-dirty
> > NOTICE:  BL31: Built : 08:16:33, Apr 17 2022
> >
> > <hang>
> >
> > It appears to me that both SPL and ATF are running.
> >
> > See below for bisect conversation...
> >> variables, I have seen hangs in ATF like this when they were not set right.
> >>
> >>>> Do you get a hang when you revert only this specific patch ?
> >>>
> >>> When I reverted the series, it booted again, but I can do a bisect
> >>> tomorrow to narrow down the specific patch in the series that causes
> >>> the issue.
> >>
> >> Please do, thanks.
> >
> > 28e5debc019b347436bdebd8978a971ce5a6582c is the first bad commit
> > commit 28e5debc019b347436bdebd8978a971ce5a6582c
>
> This commit does not exist in upstream ?
>
> u-boot$ git describe 28e5debc019b347436bdebd8978a971ce5a6582c
> fatal: 28e5debc019b347436bdebd8978a971ce5a6582c is neither a commit nor blob
>

I took your patch series from patchwork and applied it with git am on
top of the master.  I am guessing the number generated won't match
something upstream because we have different starting points.  I left
the description so you could see it corresponded to the patch in
question.  Would you prefer I use a specific branch instead?

> Can you try and force-deselect CONFIG_CLK=y (and possibly the same for
> SPL) ? I think that gets pulled in by this commit too, it might've not
> been selected on your board before. And if that's not easily possible,
> try and revert these three:
>
> 129f5102d29 ("clk: imx8m: reduce rate table duplication")
> 11c8ab01f3e ("clk: imx8mq: Add a clock driver for the imx8mq")
> a375c6f3fbe ("dt-bindings: imx8mq-clock: add mainline definitions")
>
> I would like to know whether pulling in the clock support might be the
> actual source of problems.

When I just reverted all three of those, it still hangs.  However,
when I removed CONFIG_CLK, it recovered.  I'll examine which clocks
may be missing or if there is some other clock dependency.

adam

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/17/22 18:37, Adam Ford wrote:

[...]

> When I just reverted all three of those, it still hangs.  However,
> when I removed CONFIG_CLK, it recovered.  I'll examine which clocks
> may be missing or if there is some other clock dependency.

Hum, clock sounds indeed more plausible cause of the hang because the 
SMC calls (or the power domain driver) does not come up so early in the 
process. (the other possibility is that you're running of MALLOC_F space 
due to the extra clock).

The obvious other question I have would be -- does it hang if you only 
enable CONFIG_CLK=y without these patches here ?

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Adam Ford]

On Sun, Apr 17, 2022 at 1:15 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/17/22 18:37, Adam Ford wrote:
>
> [...]
>
> > When I just reverted all three of those, it still hangs.  However,
> > when I removed CONFIG_CLK, it recovered.  I'll examine which clocks
> > may be missing or if there is some other clock dependency.
>
> Hum, clock sounds indeed more plausible cause of the hang because the
> SMC calls (or the power domain driver) does not come up so early in the
> process. (the other possibility is that you're running of MALLOC_F space
> due to the extra clock).
>
> The obvious other question I have would be -- does it hang if you only
> enable CONFIG_CLK=y without these patches here ?

It appears that I just needed to enable the CCF, CCF composite, and
the IMX8MQ clock driver which were previously disabled by default.
With those enabled, it booted just fine.
Once it booted, I was able to start the USB system and it detected a
USB thumb drive.

I did not enable them in SPL, but I can try it if you want.

Should we expand the power domain dependency to include these clocks,
or should these clocks just turn on by default if/when the 8MQ is the
target SOC?

At this point, I don't think it's a MALLOC_F issue, but I considered that.

I can mark your patch with a t-b, but I am not sure if you want to do
a V2 with these clocks enabled or do something different.

As a side note, after I enabled those clocks, U-Boot generated some
new splat.  It's unrelated to your patch. I think it bears some
investigation as a separate thread in itself, but I don't think it
should stop your patch series.

U-Boot 2022.04-00822-g70faebd20c (Apr 17 2022 - 13:12:53 -0500)

CPU:   Freescale i.MX8MQ rev2.1 at 1000 MHz
Reset cause: POR
Model: NXP i.MX8MQ EVK
DRAM:  3 GiB
clk_register: failed to get <NULL> device (parent of ckil)
clk_register: failed to get <NULL> device (parent of clock-osc-27m)
clk_register: failed to get <NULL> device (parent of sys1_pll)
clk_register: failed to get <NULL> device (parent of sys2_pll)
clk_register: failed to get <NULL> device (parent of sys3_pll)
Core:  134 devices, 17 uclasses, devicetree: separate
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
Loading Environment from MMC... *** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
SEC0:  RNG instantiated
Net:   eth0: ethernet@30be0000
Hit any key to stop autoboot:  0
u-boot=> usb start
starting USB...
Bus usb@38200000: Register 2000140 NbrPorts 2
Starting the controller
USB XHCI 1.10
scanning bus usb@38200000 for devices... 2 USB Device(s) found
       scanning usb for storage devices... 1 Storage Device(s) found
u-boot=>

Re: [PATCH 3/3] imx: power-domain: Get rid of SMCCC dependency

[Marek Vasut]

On 4/17/22 20:24, Adam Ford wrote:

Hi,

>>> When I just reverted all three of those, it still hangs.  However,
>>> when I removed CONFIG_CLK, it recovered.  I'll examine which clocks
>>> may be missing or if there is some other clock dependency.
>>
>> Hum, clock sounds indeed more plausible cause of the hang because the
>> SMC calls (or the power domain driver) does not come up so early in the
>> process. (the other possibility is that you're running of MALLOC_F space
>> due to the extra clock).
>>
>> The obvious other question I have would be -- does it hang if you only
>> enable CONFIG_CLK=y without these patches here ?
> 
> It appears that I just needed to enable the CCF, CCF composite, and
> the IMX8MQ clock driver which were previously disabled by default.
> With those enabled, it booted just fine.
> Once it booted, I was able to start the USB system and it detected a
> USB thumb drive.
> 
> I did not enable them in SPL, but I can try it if you want.

Oh, right, now you actually can bring up USB in SPL, that's a nice bonus.

> Should we expand the power domain dependency to include these clocks,
> or should these clocks just turn on by default if/when the 8MQ is the
> target SOC?

Probably add something like this into the Kconfig file:

select CLK_IMX8MQ if IMX8MQ

for all four MX8M variants and for all SPL variants (so 8 entries total) 
and that should fix the clock dependency in general.

> At this point, I don't think it's a MALLOC_F issue, but I considered that.
> 
> I can mark your patch with a t-b, but I am not sure if you want to do
> a V2 with these clocks enabled or do something different.

Maybe just send a subsequent patch. That's likely easier on Stefano, 
since this series is already buries deep in his master-next branch.