Accessing non-existent PMU registers causes an SError, halting the
system.

regmap can help us with that by allowing to pass the list of valid
registers as part of the config during creation. When this driver
creates a new regmap itself rather than relying on
syscon_node_to_regmap(), it's therefore easily possible to hook in
custom access tables for valid read and write registers.

Specifying access tables avoids SErrors for invalid registers and
instead the regmap core can just return an error. Outside drivers, this
is also helpful when using debugfs to access the regmap.

Make it possible for drivers to specify read and write tables to be
used on creation of the secure regmap.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c | 3 +++
 drivers/soc/samsung/exynos-pmu.h | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 22c50ca2aa79bf1945255ee6cc7443d7309b2573..9f416de03610b1727d8cc77616e5c87e2525cc69 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -635,6 +635,9 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 		pmu_regmcfg = regmap_smccfg;
 		pmu_regmcfg.max_register = resource_size(res) -
 					   pmu_regmcfg.reg_stride;
+		pmu_regmcfg.wr_table = pmu_context->pmu_data->wr_table;
+		pmu_regmcfg.rd_table = pmu_context->pmu_data->rd_table;
+
 		/* Need physical address for SMC call */
 		regmap = devm_regmap_init(dev, NULL,
 					  (void *)(uintptr_t)res->start,
diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
index 0938bb4fe15f439e2d8bddeec51b6077e79a7e84..113149ed32c88a09b075be82050c26970e4c0620 100644
--- a/drivers/soc/samsung/exynos-pmu.h
+++ b/drivers/soc/samsung/exynos-pmu.h
@@ -27,6 +27,10 @@ struct exynos_pmu_data {
 	void (*pmu_init)(void);
 	void (*powerdown_conf)(enum sys_powerdown);
 	void (*powerdown_conf_extra)(enum sys_powerdown);
+
+	/* for the pmu_secure case */
+	const struct regmap_access_table *rd_table;
+	const struct regmap_access_table *wr_table;
 };
 
 extern void __iomem *pmu_base_addr;

-- 
2.51.0.618.g983fd99d29-goog

On Thu, Oct 2, 2025 at 5:33 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> Accessing non-existent PMU registers causes an SError, halting the
> system.
>
> regmap can help us with that by allowing to pass the list of valid
> registers as part of the config during creation. When this driver
> creates a new regmap itself rather than relying on
> syscon_node_to_regmap(), it's therefore easily possible to hook in
> custom access tables for valid read and write registers.
>
> Specifying access tables avoids SErrors for invalid registers and
> instead the regmap core can just return an error. Outside drivers, this
> is also helpful when using debugfs to access the regmap.
>
> Make it possible for drivers to specify read and write tables to be
> used on creation of the secure regmap.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  drivers/soc/samsung/exynos-pmu.c | 3 +++
>  drivers/soc/samsung/exynos-pmu.h | 4 ++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> index 22c50ca2aa79bf1945255ee6cc7443d7309b2573..9f416de03610b1727d8cc77616e5c87e2525cc69 100644
> --- a/drivers/soc/samsung/exynos-pmu.c
> +++ b/drivers/soc/samsung/exynos-pmu.c
> @@ -635,6 +635,9 @@ static int exynos_pmu_probe(struct platform_device *pdev)
>                 pmu_regmcfg = regmap_smccfg;
>                 pmu_regmcfg.max_register = resource_size(res) -
>                                            pmu_regmcfg.reg_stride;
> +               pmu_regmcfg.wr_table = pmu_context->pmu_data->wr_table;
> +               pmu_regmcfg.rd_table = pmu_context->pmu_data->rd_table;
> +

Seems like pmu_regmcfg declaration can be pulled under this "if" scope
-- just a thought for future.

>                 /* Need physical address for SMC call */
>                 regmap = devm_regmap_init(dev, NULL,
>                                           (void *)(uintptr_t)res->start,
> diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
> index 0938bb4fe15f439e2d8bddeec51b6077e79a7e84..113149ed32c88a09b075be82050c26970e4c0620 100644
> --- a/drivers/soc/samsung/exynos-pmu.h
> +++ b/drivers/soc/samsung/exynos-pmu.h
> @@ -27,6 +27,10 @@ struct exynos_pmu_data {
>         void (*pmu_init)(void);
>         void (*powerdown_conf)(enum sys_powerdown);
>         void (*powerdown_conf_extra)(enum sys_powerdown);
> +
> +       /* for the pmu_secure case */
> +       const struct regmap_access_table *rd_table;
> +       const struct regmap_access_table *wr_table;

Maybe it's worth to add #include <linux/regmap.h> in this header, or
at least forward declaration struct regmap_access_table?

Also, would be nice to have kernel-doc comment for struct
exynos_pmu_data at this point, but it might be out of scope for this
patch.

Other than those minor nitpicks -- LGTM:

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

>  };
>
>  extern void __iomem *pmu_base_addr;
>
> --
> 2.51.0.618.g983fd99d29-goog
>
>

Hi Sam,

On Fri, 2025-10-03 at 12:22 -0500, Sam Protsenko wrote:
> On Thu, Oct 2, 2025 at 5:33 AM André Draszik <andre.draszik@linaro.org> wrote:
> 

[...]

> > 
> > diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
> > index 22c50ca2aa79bf1945255ee6cc7443d7309b2573..9f416de03610b1727d8cc77616e5c87e2525cc69 100644
> > --- a/drivers/soc/samsung/exynos-pmu.c
> > +++ b/drivers/soc/samsung/exynos-pmu.c
> > @@ -635,6 +635,9 @@ static int exynos_pmu_probe(struct platform_device *pdev)
> >                 pmu_regmcfg = regmap_smccfg;
> >                 pmu_regmcfg.max_register = resource_size(res) -
> >                                            pmu_regmcfg.reg_stride;
> > +               pmu_regmcfg.wr_table = pmu_context->pmu_data->wr_table;
> > +               pmu_regmcfg.rd_table = pmu_context->pmu_data->rd_table;
> > +
> 
> Seems like pmu_regmcfg declaration can be pulled under this "if" scope
> -- just a thought for future.

Yes, there is room for improvement. It might be even better if a platform could
provide its own regmap, rather than just a flag and the code here copying and
updating pmu_regmcfg.
I wanted to keep changes minimal at this stage, as there might be other considerations.

> 
> >                 /* Need physical address for SMC call */
> >                 regmap = devm_regmap_init(dev, NULL,
> >                                           (void *)(uintptr_t)res->start,
> > diff --git a/drivers/soc/samsung/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
> > index 0938bb4fe15f439e2d8bddeec51b6077e79a7e84..113149ed32c88a09b075be82050c26970e4c0620 100644
> > --- a/drivers/soc/samsung/exynos-pmu.h
> > +++ b/drivers/soc/samsung/exynos-pmu.h
> > @@ -27,6 +27,10 @@ struct exynos_pmu_data {
> >         void (*pmu_init)(void);
> >         void (*powerdown_conf)(enum sys_powerdown);
> >         void (*powerdown_conf_extra)(enum sys_powerdown);
> > +
> > +       /* for the pmu_secure case */
> > +       const struct regmap_access_table *rd_table;
> > +       const struct regmap_access_table *wr_table;
> 
> Maybe it's worth to add #include <linux/regmap.h> in this header, or
> at least forward declaration struct regmap_access_table?

Thanks! I'll add the forward declaration, as not all users of this header need
regmap.

> Also, would be nice to have kernel-doc comment for struct
> exynos_pmu_data at this point, but it might be out of scope for this
> patch.
> 
> Other than those minor nitpicks -- LGTM:

I'll add something for the next version.

Cheers,
Andre'