[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPLW+4=+efttfgj9gMSGpv2sjhJQ7whtoCuitK+Ku4U7hzE+1A@mail.gmail.com>
Date: Fri, 3 Oct 2025 12:55:08 -0500
From: Sam Protsenko <semen.protsenko@...aro.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
Peter Griffin <peter.griffin@...aro.org>, Tudor Ambarus <tudor.ambarus@...aro.org>,
Will McVicker <willmcvicker@...gle.com>, kernel-team@...roid.com,
linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] soc: samsung: exynos-pmu: move some gs101 related
code into new file
On Thu, Oct 2, 2025 at 5:33 AM André Draszik <andre.draszik@...aro.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@...aro.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@...aro.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
>
>
Powered by blists - more mailing lists