[PATCH 2/3] soc: samsung: exynos-pmu: move some gs101 related code into new file

André Draszik posted 3 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH 2/3] soc: samsung: exynos-pmu: move some gs101 related code into new file
Posted by André Draszik 2 months, 2 weeks ago
To avoid cluttering common code, move most of the gs101 code into a new
file, gs101-pmu.c

More code is going to be added for gs101 - having it all in one file
helps keeping the common code (file) more readable.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 MAINTAINERS                      |   1 +
 drivers/soc/samsung/Makefile     |   3 +-
 drivers/soc/samsung/exynos-pmu.c | 133 ------------------------------------
 drivers/soc/samsung/exynos-pmu.h |   7 ++
 drivers/soc/samsung/gs101-pmu.c  | 141 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 151 insertions(+), 134 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3439485437117aaffbe61b709468348231ca3cc4..b8908a95abc561ecf04be560f0e358c58acad693 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10599,6 +10599,7 @@ F:	Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
 F:	Documentation/devicetree/bindings/soc/google/google,gs101-pmu-intr-gen.yaml
 F:	arch/arm64/boot/dts/exynos/google/
 F:	drivers/clk/samsung/clk-gs101.c
+F:	drivers/soc/samsung/gs101-pmu.c
 F:	drivers/phy/samsung/phy-gs101-ufs.c
 F:	include/dt-bindings/clock/google,gs101.h
 K:	[gG]oogle.?[tT]ensor
diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
index 248a33d7754af1a1e5fbbbb79413eb300bbbc8e5..636a762608c9ba2c22a72d6f9597ceb015f7f36c 100644
--- a/drivers/soc/samsung/Makefile
+++ b/drivers/soc/samsung/Makefile
@@ -6,7 +6,8 @@ exynos_chipid-y			+= exynos-chipid.o exynos-asv.o
 
 obj-$(CONFIG_EXYNOS_USI)	+= exynos-usi.o
 
-obj-$(CONFIG_EXYNOS_PMU)	+= exynos-pmu.o
+obj-$(CONFIG_EXYNOS_PMU)	+= exynos_pmu.o
+exynos_pmu-y			+= exynos-pmu.o gs101-pmu.o
 
 obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)	+= exynos3250-pmu.o exynos4-pmu.o \
 					exynos5250-pmu.o exynos5420-pmu.o
diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 9f416de03610b1727d8cc77616e5c87e2525cc69..528fd4bd96f515a15b0bf8d67c505f7a84c0fc2e 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -6,7 +6,6 @@
 // Exynos - CPU PMU(Power Management Unit) support
 
 #include <linux/array_size.h>
-#include <linux/arm-smccc.h>
 #include <linux/bitmap.h>
 #include <linux/cpuhotplug.h>
 #include <linux/cpu_pm.h>
@@ -25,14 +24,6 @@
 
 #include "exynos-pmu.h"
 
-#define PMUALIVE_MASK			GENMASK(13, 0)
-#define TENSOR_SET_BITS			(BIT(15) | BIT(14))
-#define TENSOR_CLR_BITS			BIT(15)
-#define TENSOR_SMC_PMU_SEC_REG		0x82000504
-#define TENSOR_PMUREG_READ		0
-#define TENSOR_PMUREG_WRITE		1
-#define TENSOR_PMUREG_RMW		2
-
 struct exynos_pmu_context {
 	struct device *dev;
 	const struct exynos_pmu_data *pmu_data;
@@ -54,125 +45,6 @@ static struct exynos_pmu_context *pmu_context;
 /* forward declaration */
 static struct platform_driver exynos_pmu_driver;
 
-/*
- * Tensor SoCs are configured so that PMU_ALIVE registers can only be written
- * from EL3, but are still read accessible. As Linux needs to write some of
- * these registers, the following functions are provided and exposed via
- * regmap.
- *
- * Note: This SMC interface is known to be implemented on gs101 and derivative
- * SoCs.
- */
-
-/* Write to a protected PMU register. */
-static int tensor_sec_reg_write(void *context, unsigned int reg,
-				unsigned int val)
-{
-	struct arm_smccc_res res;
-	unsigned long pmu_base = (unsigned long)context;
-
-	arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
-		      TENSOR_PMUREG_WRITE, val, 0, 0, 0, 0, &res);
-
-	/* returns -EINVAL if access isn't allowed or 0 */
-	if (res.a0)
-		pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
-
-	return (int)res.a0;
-}
-
-/* Read/Modify/Write a protected PMU register. */
-static int tensor_sec_reg_rmw(void *context, unsigned int reg,
-			      unsigned int mask, unsigned int val)
-{
-	struct arm_smccc_res res;
-	unsigned long pmu_base = (unsigned long)context;
-
-	arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
-		      TENSOR_PMUREG_RMW, mask, val, 0, 0, 0, &res);
-
-	/* returns -EINVAL if access isn't allowed or 0 */
-	if (res.a0)
-		pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
-
-	return (int)res.a0;
-}
-
-/*
- * Read a protected PMU register. All PMU registers can be read by Linux.
- * Note: The SMC read register is not used, as only registers that can be
- * written are readable via SMC.
- */
-static int tensor_sec_reg_read(void *context, unsigned int reg,
-			       unsigned int *val)
-{
-	*val = pmu_raw_readl(reg);
-	return 0;
-}
-
-/*
- * For SoCs that have set/clear bit hardware this function can be used when
- * the PMU register will be accessed by multiple masters.
- *
- * For example, to set bits 13:8 in PMU reg offset 0x3e80
- * tensor_set_bits_atomic(ctx, 0x3e80, 0x3f00, 0x3f00);
- *
- * Set bit 8, and clear bits 13:9 PMU reg offset 0x3e80
- * tensor_set_bits_atomic(0x3e80, 0x100, 0x3f00);
- */
-static int tensor_set_bits_atomic(void *ctx, unsigned int offset, u32 val,
-				  u32 mask)
-{
-	int ret;
-	unsigned int i;
-
-	for (i = 0; i < 32; i++) {
-		if (!(mask & BIT(i)))
-			continue;
-
-		offset &= ~TENSOR_SET_BITS;
-
-		if (val & BIT(i))
-			offset |= TENSOR_SET_BITS;
-		else
-			offset |= TENSOR_CLR_BITS;
-
-		ret = tensor_sec_reg_write(ctx, offset, i);
-		if (ret)
-			return ret;
-	}
-	return 0;
-}
-
-static bool tensor_is_atomic(unsigned int reg)
-{
-	/*
-	 * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
-	 * as the target registers can be accessed by multiple masters. SFRs
-	 * that don't support atomic are added to the switch statement below.
-	 */
-	if (reg > PMUALIVE_MASK)
-		return false;
-
-	switch (reg) {
-	case GS101_SYSIP_DAT0:
-	case GS101_SYSTEM_CONFIGURATION:
-		return false;
-	default:
-		return true;
-	}
-}
-
-static int tensor_sec_update_bits(void *ctx, unsigned int reg,
-				  unsigned int mask, unsigned int val)
-{
-
-	if (!tensor_is_atomic(reg))
-		return tensor_sec_reg_rmw(ctx, reg, mask, val);
-
-	return tensor_set_bits_atomic(ctx, reg, val, mask);
-}
-
 void pmu_raw_writel(u32 val, u32 offset)
 {
 	writel_relaxed(val, pmu_base_addr + offset);
@@ -244,11 +116,6 @@ static const struct regmap_config regmap_pmu_intr = {
 	.use_raw_spinlock = true,
 };
 
-static const struct exynos_pmu_data gs101_pmu_data = {
-	.pmu_secure = true,
-	.pmu_cpuhp = true,
-};
-
 /*
  * PMU platform driver and devicetree bindings.
  */
diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
index 113149ed32c88a09b075be82050c26970e4c0620..fe11adc4f6ac8fc8bce228d5852deaff7c438221 100644
--- a/drivers/soc/samsung/exynos-pmu.h
+++ b/drivers/soc/samsung/exynos-pmu.h
@@ -44,7 +44,14 @@ extern const struct exynos_pmu_data exynos4412_pmu_data;
 extern const struct exynos_pmu_data exynos5250_pmu_data;
 extern const struct exynos_pmu_data exynos5420_pmu_data;
 #endif
+extern const struct exynos_pmu_data gs101_pmu_data;
 
 extern void pmu_raw_writel(u32 val, u32 offset);
 extern u32 pmu_raw_readl(u32 offset);
+
+int tensor_sec_reg_write(void *context, unsigned int reg, unsigned int val);
+int tensor_sec_reg_read(void *context, unsigned int reg, unsigned int *val);
+int tensor_sec_update_bits(void *ctx, unsigned int reg, unsigned int mask,
+			   unsigned int val);
+
 #endif /* __EXYNOS_PMU_H */
diff --git a/drivers/soc/samsung/gs101-pmu.c b/drivers/soc/samsung/gs101-pmu.c
new file mode 100644
index 0000000000000000000000000000000000000000..b5a535822ec830b751e36a33121e2a03ef2ebcb2
--- /dev/null
+++ b/drivers/soc/samsung/gs101-pmu.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright 2025 Linaro Ltd.
+//
+// GS101 PMU (Power Management Unit) support
+
+#include <linux/arm-smccc.h>
+#include <linux/array_size.h>
+#include <linux/soc/samsung/exynos-pmu.h>
+#include <linux/soc/samsung/exynos-regs-pmu.h>
+
+#include "exynos-pmu.h"
+
+#define PMUALIVE_MASK			GENMASK(13, 0)
+#define TENSOR_SET_BITS			(BIT(15) | BIT(14))
+#define TENSOR_CLR_BITS			BIT(15)
+#define TENSOR_SMC_PMU_SEC_REG		0x82000504
+#define TENSOR_PMUREG_READ		0
+#define TENSOR_PMUREG_WRITE		1
+#define TENSOR_PMUREG_RMW		2
+
+const struct exynos_pmu_data gs101_pmu_data = {
+	.pmu_secure = true,
+	.pmu_cpuhp = true,
+};
+
+/*
+ * Tensor SoCs are configured so that PMU_ALIVE registers can only be written
+ * from EL3, but are still read accessible. As Linux needs to write some of
+ * these registers, the following functions are provided and exposed via
+ * regmap.
+ *
+ * Note: This SMC interface is known to be implemented on gs101 and derivative
+ * SoCs.
+ */
+
+/* Write to a protected PMU register. */
+int tensor_sec_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct arm_smccc_res res;
+	unsigned long pmu_base = (unsigned long)context;
+
+	arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
+		      TENSOR_PMUREG_WRITE, val, 0, 0, 0, 0, &res);
+
+	/* returns -EINVAL if access isn't allowed or 0 */
+	if (res.a0)
+		pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
+
+	return (int)res.a0;
+}
+
+/* Read/Modify/Write a protected PMU register. */
+static int tensor_sec_reg_rmw(void *context, unsigned int reg,
+			      unsigned int mask, unsigned int val)
+{
+	struct arm_smccc_res res;
+	unsigned long pmu_base = (unsigned long)context;
+
+	arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
+		      TENSOR_PMUREG_RMW, mask, val, 0, 0, 0, &res);
+
+	/* returns -EINVAL if access isn't allowed or 0 */
+	if (res.a0)
+		pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
+
+	return (int)res.a0;
+}
+
+/*
+ * Read a protected PMU register. All PMU registers can be read by Linux.
+ * Note: The SMC read register is not used, as only registers that can be
+ * written are readable via SMC.
+ */
+int tensor_sec_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	*val = pmu_raw_readl(reg);
+	return 0;
+}
+
+/*
+ * For SoCs that have set/clear bit hardware this function can be used when
+ * the PMU register will be accessed by multiple masters.
+ *
+ * For example, to set bits 13:8 in PMU reg offset 0x3e80
+ * tensor_set_bits_atomic(ctx, 0x3e80, 0x3f00, 0x3f00);
+ *
+ * Set bit 8, and clear bits 13:9 PMU reg offset 0x3e80
+ * tensor_set_bits_atomic(0x3e80, 0x100, 0x3f00);
+ */
+static int tensor_set_bits_atomic(void *ctx, unsigned int offset, u32 val,
+				  u32 mask)
+{
+	int ret;
+	unsigned int i;
+
+	for (i = 0; i < 32; i++) {
+		if (!(mask & BIT(i)))
+			continue;
+
+		offset &= ~TENSOR_SET_BITS;
+
+		if (val & BIT(i))
+			offset |= TENSOR_SET_BITS;
+		else
+			offset |= TENSOR_CLR_BITS;
+
+		ret = tensor_sec_reg_write(ctx, offset, i);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static bool tensor_is_atomic(unsigned int reg)
+{
+	/*
+	 * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
+	 * as the target registers can be accessed by multiple masters. SFRs
+	 * that don't support atomic are added to the switch statement below.
+	 */
+	if (reg > PMUALIVE_MASK)
+		return false;
+
+	switch (reg) {
+	case GS101_SYSIP_DAT0:
+	case GS101_SYSTEM_CONFIGURATION:
+		return false;
+	default:
+		return true;
+	}
+}
+
+int tensor_sec_update_bits(void *ctx, unsigned int reg, unsigned int mask,
+			   unsigned int val)
+{
+	if (!tensor_is_atomic(reg))
+		return tensor_sec_reg_rmw(ctx, reg, mask, val);
+
+	return tensor_set_bits_atomic(ctx, reg, val, mask);
+}

-- 
2.51.0.618.g983fd99d29-goog

Re: [PATCH 2/3] soc: samsung: exynos-pmu: move some gs101 related code into new file
Posted by Sam Protsenko 2 months, 2 weeks ago
On Thu, Oct 2, 2025 at 5:33 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> To avoid cluttering common code, move most of the gs101 code into a new
> file, gs101-pmu.c
>
> More code is going to be added for gs101 - having it all in one file
> helps keeping the common code (file) more readable.
>

Maybe add "no functional change" note for refactoring/cleanup patches like this.

> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  MAINTAINERS                      |   1 +
>  drivers/soc/samsung/Makefile     |   3 +-
>  drivers/soc/samsung/exynos-pmu.c | 133 ------------------------------------
>  drivers/soc/samsung/exynos-pmu.h |   7 ++
>  drivers/soc/samsung/gs101-pmu.c  | 141 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 151 insertions(+), 134 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3439485437117aaffbe61b709468348231ca3cc4..b8908a95abc561ecf04be560f0e358c58acad693 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10599,6 +10599,7 @@ F:      Documentation/devicetree/bindings/clock/google,gs101-clock.yaml
>  F:     Documentation/devicetree/bindings/soc/google/google,gs101-pmu-intr-gen.yaml
>  F:     arch/arm64/boot/dts/exynos/google/
>  F:     drivers/clk/samsung/clk-gs101.c
> +F:     drivers/soc/samsung/gs101-pmu.c
>  F:     drivers/phy/samsung/phy-gs101-ufs.c
>  F:     include/dt-bindings/clock/google,gs101.h
>  K:     [gG]oogle.?[tT]ensor
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 248a33d7754af1a1e5fbbbb79413eb300bbbc8e5..636a762608c9ba2c22a72d6f9597ceb015f7f36c 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -6,7 +6,8 @@ exynos_chipid-y                 += exynos-chipid.o exynos-asv.o
>
>  obj-$(CONFIG_EXYNOS_USI)       += exynos-usi.o
>
> -obj-$(CONFIG_EXYNOS_PMU)       += exynos-pmu.o
> +obj-$(CONFIG_EXYNOS_PMU)       += exynos_pmu.o
> +exynos_pmu-y                   += exynos-pmu.o gs101-pmu.o
>
>  obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)   += exynos3250-pmu.o exynos4-pmu.o \
>                                         exynos5250-pmu.o exynos5420-pmu.o
> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> index 9f416de03610b1727d8cc77616e5c87e2525cc69..528fd4bd96f515a15b0bf8d67c505f7a84c0fc2e 100644
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
> @@ -6,7 +6,6 @@
>  // Exynos - CPU PMU(Power Management Unit) support
>
>  #include <linux/array_size.h>
> -#include <linux/arm-smccc.h>
>  #include <linux/bitmap.h>
>  #include <linux/cpuhotplug.h>
>  #include <linux/cpu_pm.h>
> @@ -25,14 +24,6 @@
>
>  #include "exynos-pmu.h"
>
> -#define PMUALIVE_MASK                  GENMASK(13, 0)
> -#define TENSOR_SET_BITS                        (BIT(15) | BIT(14))
> -#define TENSOR_CLR_BITS                        BIT(15)
> -#define TENSOR_SMC_PMU_SEC_REG         0x82000504
> -#define TENSOR_PMUREG_READ             0
> -#define TENSOR_PMUREG_WRITE            1
> -#define TENSOR_PMUREG_RMW              2
> -
>  struct exynos_pmu_context {
>         struct device *dev;
>         const struct exynos_pmu_data *pmu_data;
> @@ -54,125 +45,6 @@ static struct exynos_pmu_context *pmu_context;
>  /* forward declaration */
>  static struct platform_driver exynos_pmu_driver;
>
> -/*
> - * Tensor SoCs are configured so that PMU_ALIVE registers can only be written
> - * from EL3, but are still read accessible. As Linux needs to write some of
> - * these registers, the following functions are provided and exposed via
> - * regmap.
> - *
> - * Note: This SMC interface is known to be implemented on gs101 and derivative
> - * SoCs.
> - */
> -
> -/* Write to a protected PMU register. */
> -static int tensor_sec_reg_write(void *context, unsigned int reg,
> -                               unsigned int val)
> -{
> -       struct arm_smccc_res res;
> -       unsigned long pmu_base = (unsigned long)context;
> -
> -       arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
> -                     TENSOR_PMUREG_WRITE, val, 0, 0, 0, 0, &res);
> -
> -       /* returns -EINVAL if access isn't allowed or 0 */
> -       if (res.a0)
> -               pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
> -
> -       return (int)res.a0;
> -}
> -
> -/* Read/Modify/Write a protected PMU register. */
> -static int tensor_sec_reg_rmw(void *context, unsigned int reg,
> -                             unsigned int mask, unsigned int val)
> -{
> -       struct arm_smccc_res res;
> -       unsigned long pmu_base = (unsigned long)context;
> -
> -       arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
> -                     TENSOR_PMUREG_RMW, mask, val, 0, 0, 0, &res);
> -
> -       /* returns -EINVAL if access isn't allowed or 0 */
> -       if (res.a0)
> -               pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
> -
> -       return (int)res.a0;
> -}
> -
> -/*
> - * Read a protected PMU register. All PMU registers can be read by Linux.
> - * Note: The SMC read register is not used, as only registers that can be
> - * written are readable via SMC.
> - */
> -static int tensor_sec_reg_read(void *context, unsigned int reg,
> -                              unsigned int *val)
> -{
> -       *val = pmu_raw_readl(reg);
> -       return 0;
> -}
> -
> -/*
> - * For SoCs that have set/clear bit hardware this function can be used when
> - * the PMU register will be accessed by multiple masters.
> - *
> - * For example, to set bits 13:8 in PMU reg offset 0x3e80
> - * tensor_set_bits_atomic(ctx, 0x3e80, 0x3f00, 0x3f00);
> - *
> - * Set bit 8, and clear bits 13:9 PMU reg offset 0x3e80
> - * tensor_set_bits_atomic(0x3e80, 0x100, 0x3f00);
> - */
> -static int tensor_set_bits_atomic(void *ctx, unsigned int offset, u32 val,
> -                                 u32 mask)
> -{
> -       int ret;
> -       unsigned int i;
> -
> -       for (i = 0; i < 32; i++) {
> -               if (!(mask & BIT(i)))
> -                       continue;
> -
> -               offset &= ~TENSOR_SET_BITS;
> -
> -               if (val & BIT(i))
> -                       offset |= TENSOR_SET_BITS;
> -               else
> -                       offset |= TENSOR_CLR_BITS;
> -
> -               ret = tensor_sec_reg_write(ctx, offset, i);
> -               if (ret)
> -                       return ret;
> -       }
> -       return 0;
> -}
> -
> -static bool tensor_is_atomic(unsigned int reg)
> -{
> -       /*
> -        * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
> -        * as the target registers can be accessed by multiple masters. SFRs
> -        * that don't support atomic are added to the switch statement below.
> -        */
> -       if (reg > PMUALIVE_MASK)
> -               return false;
> -
> -       switch (reg) {
> -       case GS101_SYSIP_DAT0:
> -       case GS101_SYSTEM_CONFIGURATION:
> -               return false;
> -       default:
> -               return true;
> -       }
> -}
> -
> -static int tensor_sec_update_bits(void *ctx, unsigned int reg,
> -                                 unsigned int mask, unsigned int val)
> -{
> -
> -       if (!tensor_is_atomic(reg))
> -               return tensor_sec_reg_rmw(ctx, reg, mask, val);
> -
> -       return tensor_set_bits_atomic(ctx, reg, val, mask);
> -}
> -
>  void pmu_raw_writel(u32 val, u32 offset)
>  {
>         writel_relaxed(val, pmu_base_addr + offset);
> @@ -244,11 +116,6 @@ static const struct regmap_config regmap_pmu_intr = {
>         .use_raw_spinlock = true,
>  };
>
> -static const struct exynos_pmu_data gs101_pmu_data = {
> -       .pmu_secure = true,
> -       .pmu_cpuhp = true,
> -};
> -
>  /*
>   * PMU platform driver and devicetree bindings.
>   */
> diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
> index 113149ed32c88a09b075be82050c26970e4c0620..fe11adc4f6ac8fc8bce228d5852deaff7c438221 100644
> --- a/drivers/soc/samsung/exynos-pmu.h
> +++ b/drivers/soc/samsung/exynos-pmu.h
> @@ -44,7 +44,14 @@ extern const struct exynos_pmu_data exynos4412_pmu_data;
>  extern const struct exynos_pmu_data exynos5250_pmu_data;
>  extern const struct exynos_pmu_data exynos5420_pmu_data;
>  #endif
> +extern const struct exynos_pmu_data gs101_pmu_data;
>
>  extern void pmu_raw_writel(u32 val, u32 offset);
>  extern u32 pmu_raw_readl(u32 offset);
> +
> +int tensor_sec_reg_write(void *context, unsigned int reg, unsigned int val);
> +int tensor_sec_reg_read(void *context, unsigned int reg, unsigned int *val);
> +int tensor_sec_update_bits(void *ctx, unsigned int reg, unsigned int mask,
> +                          unsigned int val);

Nitpick: just noticed the inconsistency between context/ctx wording
usage in above function arguments.

> +
>  #endif /* __EXYNOS_PMU_H */
> diff --git a/drivers/soc/samsung/gs101-pmu.c b/drivers/soc/samsung/gs101-pmu.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b5a535822ec830b751e36a33121e2a03ef2ebcb2
> --- /dev/null
> +++ b/drivers/soc/samsung/gs101-pmu.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright 2025 Linaro Ltd.
> +//
> +// GS101 PMU (Power Management Unit) support
> +

AFAIR headers like these should be made using multi-line comments (not
talking about SPDX part). Or is it the latest fashion trends in
kernel?

Anyways, those all are minor:

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> +#include <linux/arm-smccc.h>
> +#include <linux/array_size.h>
> +#include <linux/soc/samsung/exynos-pmu.h>
> +#include <linux/soc/samsung/exynos-regs-pmu.h>
> +
> +#include "exynos-pmu.h"
> +
> +#define PMUALIVE_MASK                  GENMASK(13, 0)
> +#define TENSOR_SET_BITS                        (BIT(15) | BIT(14))
> +#define TENSOR_CLR_BITS                        BIT(15)
> +#define TENSOR_SMC_PMU_SEC_REG         0x82000504
> +#define TENSOR_PMUREG_READ             0
> +#define TENSOR_PMUREG_WRITE            1
> +#define TENSOR_PMUREG_RMW              2
> +
> +const struct exynos_pmu_data gs101_pmu_data = {
> +       .pmu_secure = true,
> +       .pmu_cpuhp = true,
> +};
> +
> +/*
> + * Tensor SoCs are configured so that PMU_ALIVE registers can only be written
> + * from EL3, but are still read accessible. As Linux needs to write some of
> + * these registers, the following functions are provided and exposed via
> + * regmap.
> + *
> + * Note: This SMC interface is known to be implemented on gs101 and derivative
> + * SoCs.
> + */
> +
> +/* Write to a protected PMU register. */
> +int tensor_sec_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +       struct arm_smccc_res res;
> +       unsigned long pmu_base = (unsigned long)context;
> +
> +       arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
> +                     TENSOR_PMUREG_WRITE, val, 0, 0, 0, 0, &res);
> +
> +       /* returns -EINVAL if access isn't allowed or 0 */
> +       if (res.a0)
> +               pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
> +
> +       return (int)res.a0;
> +}
> +
> +/* Read/Modify/Write a protected PMU register. */
> +static int tensor_sec_reg_rmw(void *context, unsigned int reg,
> +                             unsigned int mask, unsigned int val)
> +{
> +       struct arm_smccc_res res;
> +       unsigned long pmu_base = (unsigned long)context;
> +
> +       arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG, pmu_base + reg,
> +                     TENSOR_PMUREG_RMW, mask, val, 0, 0, 0, &res);
> +
> +       /* returns -EINVAL if access isn't allowed or 0 */
> +       if (res.a0)
> +               pr_warn("%s(): SMC failed: %d\n", __func__, (int)res.a0);
> +
> +       return (int)res.a0;
> +}
> +
> +/*
> + * Read a protected PMU register. All PMU registers can be read by Linux.
> + * Note: The SMC read register is not used, as only registers that can be
> + * written are readable via SMC.
> + */
> +int tensor_sec_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +       *val = pmu_raw_readl(reg);
> +       return 0;
> +}
> +
> +/*
> + * For SoCs that have set/clear bit hardware this function can be used when
> + * the PMU register will be accessed by multiple masters.
> + *
> + * For example, to set bits 13:8 in PMU reg offset 0x3e80
> + * tensor_set_bits_atomic(ctx, 0x3e80, 0x3f00, 0x3f00);
> + *
> + * Set bit 8, and clear bits 13:9 PMU reg offset 0x3e80
> + * tensor_set_bits_atomic(0x3e80, 0x100, 0x3f00);
> + */
> +static int tensor_set_bits_atomic(void *ctx, unsigned int offset, u32 val,
> +                                 u32 mask)
> +{
> +       int ret;
> +       unsigned int i;
> +
> +       for (i = 0; i < 32; i++) {
> +               if (!(mask & BIT(i)))
> +                       continue;
> +
> +               offset &= ~TENSOR_SET_BITS;
> +
> +               if (val & BIT(i))
> +                       offset |= TENSOR_SET_BITS;
> +               else
> +                       offset |= TENSOR_CLR_BITS;
> +
> +               ret = tensor_sec_reg_write(ctx, offset, i);
> +               if (ret)
> +                       return ret;
> +       }
> +       return 0;
> +}
> +
> +static bool tensor_is_atomic(unsigned int reg)
> +{
> +       /*
> +        * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
> +        * as the target registers can be accessed by multiple masters. SFRs
> +        * that don't support atomic are added to the switch statement below.
> +        */
> +       if (reg > PMUALIVE_MASK)
> +               return false;
> +
> +       switch (reg) {
> +       case GS101_SYSIP_DAT0:
> +       case GS101_SYSTEM_CONFIGURATION:
> +               return false;
> +       default:
> +               return true;
> +       }
> +}
> +
> +int tensor_sec_update_bits(void *ctx, unsigned int reg, unsigned int mask,
> +                          unsigned int val)
> +{
> +       if (!tensor_is_atomic(reg))
> +               return tensor_sec_reg_rmw(ctx, reg, mask, val);
> +
> +       return tensor_set_bits_atomic(ctx, reg, val, mask);
> +}
>
> --
> 2.51.0.618.g983fd99d29-goog
>
>
Re: [PATCH 2/3] soc: samsung: exynos-pmu: move some gs101 related code into new file
Posted by André Draszik 2 months, 1 week ago
Hi Sam,

On Fri, 2025-10-03 at 12:55 -0500, Sam Protsenko wrote:
> On Thu, Oct 2, 2025 at 5:33 AM André Draszik <andre.draszik@linaro.org> wrote:
> > 
> > To avoid cluttering common code, move most of the gs101 code into a new
> > file, gs101-pmu.c
> > 
> > More code is going to be added for gs101 - having it all in one file
> > helps keeping the common code (file) more readable.
> > 
> 
> Maybe add "no functional change" note for refactoring/cleanup patches like this.

Sure


[...]

> > 
> > diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
> > index 113149ed32c88a09b075be82050c26970e4c0620..fe11adc4f6ac8fc8bce228d5852deaff7c438221 100644
> > --- a/drivers/soc/samsung/exynos-pmu.h
> > +++ b/drivers/soc/samsung/exynos-pmu.h
> > @@ -44,7 +44,14 @@ extern const struct exynos_pmu_data exynos4412_pmu_data;
> >  extern const struct exynos_pmu_data exynos5250_pmu_data;
> >  extern const struct exynos_pmu_data exynos5420_pmu_data;
> >  #endif
> > +extern const struct exynos_pmu_data gs101_pmu_data;
> > 
> >  extern void pmu_raw_writel(u32 val, u32 offset);
> >  extern u32 pmu_raw_readl(u32 offset);
> > +
> > +int tensor_sec_reg_write(void *context, unsigned int reg, unsigned int val);
> > +int tensor_sec_reg_read(void *context, unsigned int reg, unsigned int *val);
> > +int tensor_sec_update_bits(void *ctx, unsigned int reg, unsigned int mask,
> > +                          unsigned int val);
> 
> Nitpick: just noticed the inconsistency between context/ctx wording
> usage in above function arguments.

Interesting... I'll fix it as part of the move.

> 
> > +
> >  #endif /* __EXYNOS_PMU_H */
> > diff --git a/drivers/soc/samsung/gs101-pmu.c b/drivers/soc/samsung/gs101-pmu.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..b5a535822ec830b751e36a33121e2a03ef2ebcb2
> > --- /dev/null
> > +++ b/drivers/soc/samsung/gs101-pmu.c
> > @@ -0,0 +1,141 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright 2025 Linaro Ltd.
> > +//
> > +// GS101 PMU (Power Management Unit) support
> > +
> 
> AFAIR headers like these should be made using multi-line comments (not
> talking about SPDX part). Or is it the latest fashion trends in
> kernel?

Depends on subsystem, but multi-line for most. Here I went with existing style for
the PMU-related files, though.

Cheers,
Andre'