Some additional register writes are required when hotplugging CPUs
on gs101, without these the system hangs when hotplugging.

Specifically a CPU_INFORM register needs to be programmed with
a hint value which is used by the EL3 firmware (el3mon) and the
pmu-intr-gen registers need to be programmed.

With this patch applied, and corresponding DT update CPU hotplug
now works as expected. e.g.

echo 0 > /sys/devices/system/cpu/cpu6/online
echo 1 > /sys/devices/system/cpu/cpu6/online

Note: to maintain compatibility with older DTs that didn't specify
pmu-intr-gen register region only a warning is issued if the
registers can't be mapped, and the old behaviour is maintained.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c            | 73 ++++++++++++++++++++++++++++-
 drivers/soc/samsung/exynos-pmu.h            |  1 +
 include/linux/soc/samsung/exynos-regs-pmu.h | 11 +++++
 3 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index d8c53cec7f37..68eb4eb3813b 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -6,6 +6,7 @@
 // Exynos - CPU PMU(Power Management Unit) support
 
 #include <linux/arm-smccc.h>
+#include <linux/cpuhotplug.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/mfd/core.h>
@@ -32,6 +33,7 @@ struct exynos_pmu_context {
 	struct device *dev;
 	const struct exynos_pmu_data *pmu_data;
 	struct regmap *pmureg;
+	void __iomem *pmuintrgen_base;
 };
 
 void __iomem *pmu_base_addr;
@@ -221,7 +223,8 @@ static const struct regmap_config regmap_smccfg = {
 };
 
 static const struct exynos_pmu_data gs101_pmu_data = {
-	.pmu_secure = true
+	.pmu_secure = true,
+	.pmu_cpuhp = true,
 };
 
 /*
@@ -325,6 +328,52 @@ struct regmap *exynos_get_pmu_regmap_by_phandle(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
 
+/*
+ * CPU_INFORM register hint values which are used by
+ * EL3 firmware (el3mon).
+ */
+#define CPU_INFORM_CLEAR	0
+#define CPU_INFORM_C2		1
+
+static int cpuhp_pmu_online(unsigned int cpu)
+{
+	void __iomem *base = pmu_context->pmuintrgen_base;
+	u32 reg;
+	u32 mask;
+
+	/* clear cpu inform hint */
+	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
+		     CPU_INFORM_CLEAR);
+
+	mask = (1 << cpu);
+
+	writel(((0 << cpu) & mask), base + GS101_GRP2_INTR_BID_ENABLE);
+
+	reg = readl(base + GS101_GRP2_INTR_BID_UPEND) & mask;
+	writel(reg & mask, base + GS101_GRP2_INTR_BID_CLEAR);
+
+	return 0;
+}
+
+static int cpuhp_pmu_offline(unsigned int cpu)
+{
+	void __iomem *base = pmu_context->pmuintrgen_base;
+	u32 reg, mask;
+
+	/* set cpu inform hint */
+	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
+		     CPU_INFORM_C2);
+
+	writel((1 << cpu), base + GS101_GRP2_INTR_BID_ENABLE);
+
+	mask = ((1 << cpu) | (1 << (cpu+8)));
+
+	reg = readl(base + GS101_GRP1_INTR_BID_UPEND) & mask;
+	writel(reg & mask, base + GS101_GRP1_INTR_BID_CLEAR);
+
+	return 0;
+}
+
 static int exynos_pmu_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -377,6 +426,28 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 	pmu_context->pmureg = regmap;
 	pmu_context->dev = dev;
 
+	if (pmu_context->pmu_data && pmu_context->pmu_data->pmu_cpuhp) {
+
+		pmu_context->pmuintrgen_base =
+			devm_platform_ioremap_resource_byname(pdev, "pmu-intr-gen");
+		/*
+		 * To maintain support for older DTs that didn't specify pmu-intr-gen
+		 * register region, just issue a warning rather than fail to probe.
+		 */
+		if (IS_ERR(pmu_context->pmuintrgen_base)) {
+			dev_warn(&pdev->dev,
+				 "failed to map pmu-intr-gen registers\n");
+		} else {
+			cpuhp_setup_state(CPUHP_BP_PREPARE_DYN,
+					"soc/exynos-pmu:prepare",
+					cpuhp_pmu_online, NULL);
+
+			cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+					"soc/exynos-pmu:online",
+					NULL, cpuhp_pmu_offline);
+		}
+	}
+
 	if (pmu_context->pmu_data && pmu_context->pmu_data->pmu_init)
 		pmu_context->pmu_data->pmu_init();
 
diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
index 0a49a2c9a08e..0938bb4fe15f 100644
--- a/drivers/soc/samsung/exynos-pmu.h
+++ b/drivers/soc/samsung/exynos-pmu.h
@@ -22,6 +22,7 @@ struct exynos_pmu_data {
 	const struct exynos_pmu_conf *pmu_config;
 	const struct exynos_pmu_conf *pmu_config_extra;
 	bool pmu_secure;
+	bool pmu_cpuhp;
 
 	void (*pmu_init)(void);
 	void (*powerdown_conf)(enum sys_powerdown);
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h b/include/linux/soc/samsung/exynos-regs-pmu.h
index ce1a3790d6fb..0d5a17ea8fb8 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -658,9 +658,20 @@
 #define EXYNOS5433_PAD_RETENTION_FSYSGENIO_OPTION		(0x32A8)
 
 /* For Tensor GS101 */
+/* PMU ALIVE */
 #define GS101_SYSIP_DAT0					(0x810)
+#define GS101_CPU0_INFORM					(0x860)
+#define GS101_CPU_INFORM(cpu)	\
+			(GS101_CPU0_INFORM + (cpu*4))
 #define GS101_SYSTEM_CONFIGURATION				(0x3A00)
 #define GS101_PHY_CTRL_USB20					(0x3EB0)
 #define GS101_PHY_CTRL_USBDP					(0x3EB4)
 
+/* PMU INTR GEN */
+#define GS101_GRP1_INTR_BID_UPEND				(0x0108)
+#define GS101_GRP1_INTR_BID_CLEAR				(0x010c)
+#define GS101_GRP2_INTR_BID_ENABLE				(0x0200)
+#define GS101_GRP2_INTR_BID_UPEND				(0x0208)
+#define GS101_GRP2_INTR_BID_CLEAR				(0x020c)
+
 #endif /* __LINUX_SOC_EXYNOS_REGS_PMU_H */

-- 
2.47.1.613.gc27f4b7a9f-goog

On 13/12/2024 17:44, Peter Griffin wrote:
>  /*
> @@ -325,6 +328,52 @@ struct regmap *exynos_get_pmu_regmap_by_phandle(struct device_node *np,
>  }
>  EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
>  
> +/*
> + * CPU_INFORM register hint values which are used by
> + * EL3 firmware (el3mon).
> + */
> +#define CPU_INFORM_CLEAR	0
> +#define CPU_INFORM_C2		1
> +
> +static int cpuhp_pmu_online(unsigned int cpu)

exynos_cpuhp_pmu_online
or gs101_cpuhp_pmu_online
same for offline

> +{
> +	void __iomem *base = pmu_context->pmuintrgen_base;
> +	u32 reg;
> +	u32 mask;
> +
> +	/* clear cpu inform hint */
> +	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
> +		     CPU_INFORM_CLEAR);
> +
> +	mask = (1 << cpu);

BIT(cpu)

> +
> +	writel(((0 << cpu) & mask), base + GS101_GRP2_INTR_BID_ENABLE);

I am not sure if I follow. You want to zero-out all other bits or enable
all other bits?

> +
> +	reg = readl(base + GS101_GRP2_INTR_BID_UPEND) & mask;
> +	writel(reg & mask, base + GS101_GRP2_INTR_BID_CLEAR);

reg is &mask twice.

I don't follow this either, are these auto-cleared? It feels like you
wanted to update some bits, but you are updating entire registers in
both cases.


> +
> +	return 0;
> +}
> +
> +static int cpuhp_pmu_offline(unsigned int cpu)
> +{
> +	void __iomem *base = pmu_context->pmuintrgen_base;
> +	u32 reg, mask;
> +
> +	/* set cpu inform hint */
> +	regmap_write(pmu_context->pmureg, GS101_CPU_INFORM(cpu),
> +		     CPU_INFORM_C2);
> +
> +	writel((1 << cpu), base + GS101_GRP2_INTR_BID_ENABLE);
> +
> +	mask = ((1 << cpu) | (1 << (cpu+8)));

What does 8 stands for?

> +
> +	reg = readl(base + GS101_GRP1_INTR_BID_UPEND) & mask;
> +	writel(reg & mask, base + GS101_GRP1_INTR_BID_CLEAR);
> +
> +	return 0;
> +}
> +
>  static int exynos_pmu_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -377,6 +426,28 @@ static int exynos_pmu_probe(struct platform_device *pdev)
>  	pmu_context->pmureg = regmap;
>  	pmu_context->dev = dev;
>  
> +	if (pmu_context->pmu_data && pmu_context->pmu_data->pmu_cpuhp) {
> +

Drop blank line

> +		pmu_context->pmuintrgen_base =
> +			devm_platform_ioremap_resource_byname(pdev, "pmu-intr-gen");
> +		/*
> +		 * To maintain support for older DTs that didn't specify pmu-intr-gen
> +		 * register region, just issue a warning rather than fail to probe.
> +		 */
> +		if (IS_ERR(pmu_context->pmuintrgen_base)) {
> +			dev_warn(&pdev->dev,
> +				 "failed to map pmu-intr-gen registers\n");

Test old DTS, I think you do write() to the ERR_PTR when
offlining/onlining...

> +		} else {
> +			cpuhp_setup_state(CPUHP_BP_PREPARE_DYN,
> +					"soc/exynos-pmu:prepare",
> +					cpuhp_pmu_online, NULL);
> +
> +			cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> +					"soc/exynos-pmu:online",
> +					NULL, cpuhp_pmu_offline);
> +		}
> +	}
> +
>  	if (pmu_context->pmu_data && pmu_context->pmu_data->pmu_init)
>  		pmu_context->pmu_data->pmu_init();
>  



Best regards,
Krzysztof