diff mbox series

[2/9] soc: samsung: exynos-pmu: Add exynos_pmu_update/read/write APIs and SoC quirks

Message ID 20240122225710.1952066-3-peter.griffin@linaro.org
State New
Headers show
Series Add exynos_pmu_update/read/write() APIs to exynos-pmu | expand

Commit Message

Peter Griffin Jan. 22, 2024, 10:57 p.m. UTC 
Newer Exynos SoCs have atomic set/clear bit hardware for PMU registers as
these registers can be accessed by multiple masters. Some platforms also
protect the PMU registers for security hardening reasons so they can't be
written by normal world and are only write acessible in el3 via a SMC call.

Add support for both of these usecases using SoC specific quirks that are
determined from the DT compatible string.

Drivers which need to read and write PMU registers should now use these
new exynos_pmu_*() APIs instead of obtaining a regmap using
syscon_regmap_lookup_by_phandle()

Depending on the SoC specific quirks, the exynos_pmu_*() APIs will access
the PMU register in the appropriate way.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c       | 209 ++++++++++++++++++++++++-
 drivers/soc/samsung/exynos-pmu.h       |   4 +
 include/linux/soc/samsung/exynos-pmu.h |  28 ++++
 3 files changed, 234 insertions(+), 7 deletions(-)

Comments

Sam Protsenko Jan. 23, 2024, 6:56 p.m. UTC | #1 
On Mon, Jan 22, 2024 at 4:57 PM Peter Griffin <peter.griffin@linaro.org> wrote:
>
> Newer Exynos SoCs have atomic set/clear bit hardware for PMU registers as
> these registers can be accessed by multiple masters. Some platforms also
> protect the PMU registers for security hardening reasons so they can't be
> written by normal world and are only write acessible in el3 via a SMC call.
>
> Add support for both of these usecases using SoC specific quirks that are
> determined from the DT compatible string.
>
> Drivers which need to read and write PMU registers should now use these
> new exynos_pmu_*() APIs instead of obtaining a regmap using
> syscon_regmap_lookup_by_phandle()
>
> Depending on the SoC specific quirks, the exynos_pmu_*() APIs will access
> the PMU register in the appropriate way.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  drivers/soc/samsung/exynos-pmu.c       | 209 ++++++++++++++++++++++++-
>  drivers/soc/samsung/exynos-pmu.h       |   4 +
>  include/linux/soc/samsung/exynos-pmu.h |  28 ++++
>  3 files changed, 234 insertions(+), 7 deletions(-)
>

[snip]

> +
> +int exynos_pmu_update_bits(unsigned int offset, unsigned int mask,
> +                          unsigned int val)
> +{
> +       if (pmu_context->pmu_data &&
> +           pmu_context->pmu_data->quirks & QUIRK_PMU_ALIVE_WRITE_SEC)
> +               return rmw_priv_reg(pmu_context->pmu_base_pa + offset,
> +                                   mask, val);
> +
> +       return regmap_update_bits(pmu_context->pmureg, offset, mask, val);
> +}
> +EXPORT_SYMBOL(exynos_pmu_update_bits);
> +

This seems a bit hacky, from the design perspective. This way the user
will have to worry about things like driver dependencies, making sure
everything is instantiated in a correct order, etc. It also hides the
details otherwise visible through "syscon-phandle" property in the
device tree. Can we instead rework it by overriding regmap
implementation for Exynos specifics, and then continue to use it in
the leaf drivers via "syscon-phandle" property?

[snip]
Peter Griffin Jan. 24, 2024, 10:02 a.m. UTC | #2 
Hi Sam,

Thanks for the review feedback.

On Tue, 23 Jan 2024 at 18:56, Sam Protsenko <semen.protsenko@linaro.org> wrote:
>
> On Mon, Jan 22, 2024 at 4:57 PM Peter Griffin <peter.griffin@linaro.org> wrote:
> >
> > Newer Exynos SoCs have atomic set/clear bit hardware for PMU registers as
> > these registers can be accessed by multiple masters. Some platforms also
> > protect the PMU registers for security hardening reasons so they can't be
> > written by normal world and are only write acessible in el3 via a SMC call.
> >
> > Add support for both of these usecases using SoC specific quirks that are
> > determined from the DT compatible string.
> >
> > Drivers which need to read and write PMU registers should now use these
> > new exynos_pmu_*() APIs instead of obtaining a regmap using
> > syscon_regmap_lookup_by_phandle()
> >
> > Depending on the SoC specific quirks, the exynos_pmu_*() APIs will access
> > the PMU register in the appropriate way.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/soc/samsung/exynos-pmu.c       | 209 ++++++++++++++++++++++++-
> >  drivers/soc/samsung/exynos-pmu.h       |   4 +
> >  include/linux/soc/samsung/exynos-pmu.h |  28 ++++
> >  3 files changed, 234 insertions(+), 7 deletions(-)
> >
>
> [snip]
>
> > +
> > +int exynos_pmu_update_bits(unsigned int offset, unsigned int mask,
> > +                          unsigned int val)
> > +{
> > +       if (pmu_context->pmu_data &&
> > +           pmu_context->pmu_data->quirks & QUIRK_PMU_ALIVE_WRITE_SEC)
> > +               return rmw_priv_reg(pmu_context->pmu_base_pa + offset,
> > +                                   mask, val);
> > +
> > +       return regmap_update_bits(pmu_context->pmureg, offset, mask, val);
> > +}
> > +EXPORT_SYMBOL(exynos_pmu_update_bits);
> > +
>
> This seems a bit hacky, from the design perspective. This way the user
> will have to worry about things like driver dependencies, making sure
> everything is instantiated in a correct order, etc. It also hides the
> details otherwise visible through "syscon-phandle" property in the
> device tree.

In v2 I will keep the phandle to pmu_system_controller in DT, and add
some -EPROBE_DEFER logic (See my email with Krzysztof).

> Can we instead rework it by overriding regmap
> implementation for Exynos specifics, and then continue to use it in
> the leaf drivers via "syscon-phandle" property?

I did look at that possibility first, as like you say it would avoid
updating the leaf drivers to use the new API. Unfortunately a SMC
backend to regmap was already tried and nacked upstream pretty hard.
See here https://lore.kernel.org/lkml/20210723163759.GI5221@sirena.org.uk/T/

regards,

Peter.
Peter Griffin Jan. 24, 2024, 10:21 a.m. UTC | #3 
Hi Arnd,

On Tue, 23 Jan 2024 at 08:11, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Jan 22, 2024, at 23:57, Peter Griffin wrote:
>
> > --- a/include/linux/soc/samsung/exynos-pmu.h
> > +++ b/include/linux/soc/samsung/exynos-pmu.h
> > @@ -21,11 +21,39 @@ enum sys_powerdown {
> >  extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
> >  #ifdef CONFIG_EXYNOS_PMU
> >  extern struct regmap *exynos_get_pmu_regmap(void);
> > +extern int exynos_pmu_update_bits(unsigned int offset, unsigned int
> > mask,
> > +                               unsigned int val);
> > +extern int exynos_pmu_update(unsigned int offset, unsigned int mask,
> > +                          unsigned int val);
> > +extern int exynos_pmu_write(unsigned int offset, unsigned int val);
> > +extern int exynos_pmu_read(unsigned int offset, unsigned int *val);
> >  #else
> >  static inline struct regmap *exynos_get_pmu_regmap(void)
> >  {
> >       return ERR_PTR(-ENODEV);
> >  }
> > +
> > +static inline int exynos_pmu_update_bits(unsigned int offset, unsigned
> > int mask,
> > +                                      unsigned int val);
> > +{
> > +     return ERR_PTR(-ENODEV);
> > +}
> > +
> > +static inline int exynos_pmu_update(unsigned int offset, unsigned int
> > mask,
> > +                                 unsigned int val);
> > +{
> > +     return ERR_PTR(-ENODEV);
> > +}
>
> This won't build since you have the wrong return type.
> I would suggest you just remove the #ifdef check entirely
> and instead require drivers using this to have correct
> dependencies.

Whoops, will fix it in v2. We need those stubs for platforms like
ARCH_S3C64XX that don't have a PMU but use some of the same drivers.

Thanks,

Peter.
diff mbox series

Patch

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 250537d7cfd6..e9e933ede568 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -5,6 +5,7 @@ 
 //
 // Exynos - CPU PMU(Power Management Unit) support
 
+#include <linux/arm-smccc.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/mfd/core.h>
@@ -12,29 +13,204 @@ 
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
+#include <linux/regmap.h>
 
 #include <linux/soc/samsung/exynos-regs-pmu.h>
 #include <linux/soc/samsung/exynos-pmu.h>
 
 #include "exynos-pmu.h"
 
+/**
+ * DOC: Quirk flags for different Exynos PMU IP-cores
+ *
+ * This driver supports multiple Exynos based SoCs, each of which might have a
+ * different set of registers and features supported.
+ *
+ * Quirk flags described below serve the purpose of telling the driver about
+ * mentioned SoC traits, and can be specified in driver data for each particular
+ * supported device.
+ *
+ * %QUIRK_HAS_ATOMIC_BITSETHW: PMU IP has special atomic bit set/clear HW
+ * to protect against PMU registers being accessed from multiple bus masters.
+ *
+ * %QUIRK_PMU_ALIVE_WRITE_SEC: PMU registers are *not* write accesible from
+ * normal world. This is found on some SoCs as a security hardening measure. PMU
+ * registers on these SoCs can only be written via a SMC call and registers are
+ * checked by EL3 firmware against an allowlist before the write can procede.
+ * Note: This quirk should only be set for platforms whose el3 firmware
+ * implements the TENSOR_SMC_PMU_SEC_REG interface below.
+ */
+
+#define QUIRK_HAS_ATOMIC_BITSETHW		BIT(0)
+#define QUIRK_PMU_ALIVE_WRITE_SEC		BIT(1)
+
+#define PMUALIVE_MASK GENMASK(14, 0)
+
 struct exynos_pmu_context {
 	struct device *dev;
 	const struct exynos_pmu_data *pmu_data;
+	struct regmap *pmureg;
+	void __iomem *pmu_base_addr;
+	phys_addr_t pmu_base_pa;
+	/* protect PMU reg atomic update operations */
+	spinlock_t update_lock;
 };
 
-void __iomem *pmu_base_addr;
 static struct exynos_pmu_context *pmu_context;
 
+/*
+ * Some SoCs are configured so that PMU_ALIVE registers can only be written
+ * from el3. As Linux needs to write some of these registers, the following
+ * SMC register read/write/read,write,modify interface is used.
+ *
+ * Note: This SMC interface is known to be implemented on gs101 and derivative
+ * SoCs.
+ */
+#define TENSOR_SMC_PMU_SEC_REG			(0x82000504)
+#define TENSOR_PMUREG_READ			0
+#define TENSOR_PMUREG_WRITE			1
+#define TENSOR_PMUREG_RMW			2
+
+int set_priv_reg(phys_addr_t reg, u32 val)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG,
+		      reg,
+		      TENSOR_PMUREG_WRITE,
+		      val, 0, 0, 0, 0, &res);
+
+	if (res.a0)
+		pr_warn("%s(): SMC failed: %lu\n", __func__, res.a0);
+
+	return (int)res.a0;
+}
+
+int rmw_priv_reg(phys_addr_t reg, u32 mask, u32 val)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(TENSOR_SMC_PMU_SEC_REG,
+		      reg,
+		      TENSOR_PMUREG_RMW,
+		      mask, val, 0, 0, 0, &res);
+
+	if (res.a0)
+		pr_warn("%s(): SMC failed: %lu\n", __func__, res.a0);
+
+	return (int)res.a0;
+}
+
+/*
+ * For SoCs that have set/clear bit hardware (as indicated by
+ * QUIRK_HAS_ATOMIC_BITSETHW) 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
+ * exynos_pmu_set_bit_atomic(0x3e80, 0x3f00, 0x3f00);
+ *
+ * To clear bits 13:8 in PMU offset 0x3e80
+ * exynos_pmu_set_bit_atomic(0x3e80, 0x0, 0x3f00);
+ */
+static inline void exynos_pmu_set_bit_atomic(unsigned int offset,
+					     u32 val, u32 mask)
+{
+	unsigned long flags;
+	unsigned int i;
+
+	spin_lock_irqsave(&pmu_context->update_lock, flags);
+	for (i = 0; i < 32; i++) {
+		if (mask & BIT(i)) {
+			if (val & BIT(i)) {
+				offset |= 0xc000;
+				pmu_raw_writel(i, offset);
+			} else {
+				offset |= 0x8000;
+				pmu_raw_writel(i, offset);
+			}
+		}
+	}
+	spin_unlock_irqrestore(&pmu_context->update_lock, flags);
+}
+
+int exynos_pmu_update_bits(unsigned int offset, unsigned int mask,
+			   unsigned int val)
+{
+	if (pmu_context->pmu_data &&
+	    pmu_context->pmu_data->quirks & QUIRK_PMU_ALIVE_WRITE_SEC)
+		return rmw_priv_reg(pmu_context->pmu_base_pa + offset,
+				    mask, val);
+
+	return regmap_update_bits(pmu_context->pmureg, offset, mask, val);
+}
+EXPORT_SYMBOL(exynos_pmu_update_bits);
+
 void pmu_raw_writel(u32 val, u32 offset)
 {
-	writel_relaxed(val, pmu_base_addr + offset);
+	if (pmu_context->pmu_data &&
+	    pmu_context->pmu_data->quirks & QUIRK_PMU_ALIVE_WRITE_SEC)
+		return (void)set_priv_reg(pmu_context->pmu_base_pa + offset,
+					  val);
+
+	return writel_relaxed(val, pmu_context->pmu_base_addr + offset);
 }
 
 u32 pmu_raw_readl(u32 offset)
 {
-	return readl_relaxed(pmu_base_addr + offset);
+	return readl_relaxed(pmu_context->pmu_base_addr + offset);
+}
+
+int exynos_pmu_read(unsigned int offset, unsigned int *val)
+{
+	if (!pmu_context)
+		return -ENODEV;
+
+	/*
+	 * For platforms that protect PMU registers they
+	 * are still accessible to read from normal world
+	 */
+	return regmap_read(pmu_context->pmureg, offset, val);
+}
+EXPORT_SYMBOL(exynos_pmu_read);
+
+int exynos_pmu_write(unsigned int offset, unsigned int val)
+{
+	if (!pmu_context)
+		return -ENODEV;
+
+	if (pmu_context->pmu_data &&
+	    pmu_context->pmu_data->quirks & QUIRK_PMU_ALIVE_WRITE_SEC)
+		return set_priv_reg(pmu_context->pmu_base_pa + offset, val);
+
+	return regmap_write(pmu_context->pmureg, offset, val);
+}
+EXPORT_SYMBOL(exynos_pmu_write);
+
+int exynos_pmu_update(unsigned int offset, unsigned int mask, unsigned int val)
+{
+	int ret = 0;
+
+	if (!pmu_context)
+		return -ENODEV;
+
+	if (pmu_context->pmu_data &&
+	    pmu_context->pmu_data->quirks & QUIRK_HAS_ATOMIC_BITSETHW) {
+		/*
+		 * Use atomic operations for PMU_ALIVE registers (offset 0~0x3FFF)
+		 * as the target registers can be accessed by multiple masters.
+		 */
+		if (offset > PMUALIVE_MASK)
+			return exynos_pmu_update_bits(offset, mask, val);
+
+		exynos_pmu_set_bit_atomic(offset, val, mask);
+
+	} else {
+		return exynos_pmu_update_bits(offset, mask, val);
+	}
+
+	return ret;
 }
+EXPORT_SYMBOL(exynos_pmu_update);
 
 void exynos_sys_powerdown_conf(enum sys_powerdown mode)
 {
@@ -75,11 +251,18 @@  void exynos_sys_powerdown_conf(enum sys_powerdown mode)
 #define exynos_pmu_data_arm_ptr(data)	NULL
 #endif
 
+static const struct exynos_pmu_data gs101_pmu_data = {
+	.quirks = QUIRK_HAS_ATOMIC_BITSETHW | QUIRK_PMU_ALIVE_WRITE_SEC,
+};
+
 /*
  * PMU platform driver and devicetree bindings.
  */
 static const struct of_device_id exynos_pmu_of_device_ids[] = {
 	{
+		.compatible = "google,gs101-pmu",
+		.data = &gs101_pmu_data,
+	}, {
 		.compatible = "samsung,exynos3250-pmu",
 		.data = exynos_pmu_data_arm_ptr(exynos3250_pmu_data),
 	}, {
@@ -125,18 +308,30 @@  EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap);
 
 static int exynos_pmu_probe(struct platform_device *pdev)
 {
+	struct resource *res;
 	struct device *dev = &pdev->dev;
 	int ret;
 
-	pmu_base_addr = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(pmu_base_addr))
-		return PTR_ERR(pmu_base_addr);
-
 	pmu_context = devm_kzalloc(&pdev->dev,
 			sizeof(struct exynos_pmu_context),
 			GFP_KERNEL);
 	if (!pmu_context)
 		return -ENOMEM;
+
+	pmu_context->pmu_base_addr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pmu_context->pmu_base_addr))
+		return PTR_ERR(pmu_context->pmu_base_addr);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	pmu_context->pmu_base_pa = res->start;
+	pmu_context->pmureg = exynos_get_pmu_regmap();
+	if (IS_ERR(pmu_context->pmureg))
+		return PTR_ERR(pmu_context->pmureg);
+
+	spin_lock_init(&pmu_context->update_lock);
 	pmu_context->dev = dev;
 	pmu_context->pmu_data = of_device_get_match_data(dev);
 
diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
index 1c652ffd79b4..570c6e4dc8c3 100644
--- a/drivers/soc/samsung/exynos-pmu.h
+++ b/drivers/soc/samsung/exynos-pmu.h
@@ -25,8 +25,12 @@  struct exynos_pmu_data {
 	void (*pmu_init)(void);
 	void (*powerdown_conf)(enum sys_powerdown);
 	void (*powerdown_conf_extra)(enum sys_powerdown);
+	u32 quirks;
 };
 
+int set_priv_reg(phys_addr_t reg, u32 val);
+int rmw_priv_reg(phys_addr_t reg, u32 mask, u32 val);
+
 extern void __iomem *pmu_base_addr;
 
 #ifdef CONFIG_EXYNOS_PMU_ARM_DRIVERS
diff --git a/include/linux/soc/samsung/exynos-pmu.h b/include/linux/soc/samsung/exynos-pmu.h
index a4f5516cc956..2c5ce21fb00b 100644
--- a/include/linux/soc/samsung/exynos-pmu.h
+++ b/include/linux/soc/samsung/exynos-pmu.h
@@ -21,11 +21,39 @@  enum sys_powerdown {
 extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
 #ifdef CONFIG_EXYNOS_PMU
 extern struct regmap *exynos_get_pmu_regmap(void);
+extern int exynos_pmu_update_bits(unsigned int offset, unsigned int mask,
+				  unsigned int val);
+extern int exynos_pmu_update(unsigned int offset, unsigned int mask,
+			     unsigned int val);
+extern int exynos_pmu_write(unsigned int offset, unsigned int val);
+extern int exynos_pmu_read(unsigned int offset, unsigned int *val);
 #else
 static inline struct regmap *exynos_get_pmu_regmap(void)
 {
 	return ERR_PTR(-ENODEV);
 }
+
+static inline int exynos_pmu_update_bits(unsigned int offset, unsigned int mask,
+					 unsigned int val);
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline int exynos_pmu_update(unsigned int offset, unsigned int mask,
+				    unsigned int val);
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline int exynos_pmu_write(unsigned int offset, unsigned int val)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline int exynos_pmu_read(unsigned int offset, unsigned int *val)
+{
+	return ERR_PTR(-ENODEV);
+}
 #endif
 
 #endif /* __LINUX_SOC_EXYNOS_PMU_H */