Re: [PATCH v2 1/2] soc: samsung: exynos-pmu: Add regmap support for SoCs that protect PMU regs

Hi Krzysztof,

Thanks for your review feedback.

On Mon, 5 Feb 2024 at 13:13, Krzysztof Kozlowski
[off-list ref] wrote:

On 01/02/2024 13:51, Peter Griffin wrote:

quoted

Hi Krzysztof,

On Tue, 30 Jan 2024 at 16:01, Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 29/01/2024 22:19, Peter Griffin wrote:

quoted

Some Exynos based SoCs like Tensor gs101 protect the PMU registers for
security hardening reasons so that they are only accessible in el3 via an
SMC call.

As most Exynos drivers that need to write PMU registers currently obtain a
regmap via syscon (phys, pinctrl, watchdog). Support for the above usecase
is implemented in this driver using a custom regmap similar to syscon to
handle the SMC call. Platforms that don't secure PMU registers, get a mmio
regmap like before. As regmaps abstract out the underlying register access
changes to the leaf drivers are minimal.

A new API exynos_get_pmu_regmap_by_phandle() is provided for leaf drivers
that currently use syscon_regmap_lookup_by_phandle(). This also handles
deferred probing.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c       | 227 ++++++++++++++++++++++++-
 include/linux/soc/samsung/exynos-pmu.h |  10 ++
 2 files changed, 236 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 250537d7cfd6..7bcc144e53a2 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,20 +13,159 @@
 #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"

+static struct platform_driver exynos_pmu_driver;

I don't understand why do you need it. You can have only one
pmu_context. The moment you probe second one, previous becomes invalid.

I guess you want to parse phandle and check if just in case if it points
to the right device, but still the original code is not ready for two
PMU devices. I say either this problem should be solved entirely,
allowing two devices, or just compare device node from phandle with
device node of exynos_pmu_context->dev and return -EINVAL on mismatches.

Apologies I didn't answer your original question. This wasn't about
having partial support for multiple pmu devices. It is being used by
driver_find_device_by_of_node() in exynos_get_pmu_regmap_by_phandle()
to determine that the exynos-pmu device has probed and therefore a
pmu_context exists and a regmap has been created and can be returned
to the caller (as opposed to doing a -EPROBE_DEFER).

Is there some better/other API you recommend for this purpose? Just
checking pmu_context directly seems racy, so I don't think we should
do that.

Hm, I don't quite get why you cannot use of_find_device_by_node()?

of_find_device_by_node() returns a platform_device, even if the driver
hasn't probed. Whereas driver_find_device_by_of_node() iterates
devices bound to a driver.

If using of_find_device_by_node() API I could check the result of
platform_get_drvdata(), and -EPROBE_DEFER if NULL (that pattern seems
to be used by a few drivers). But that AFAIK only guarantees you
reached the platform_set_drvdata() call in your driver probe()
function, not that it has completed.

IMHO the drivers using driver_find_device_by_of_node() for probe
deferral are doing it more robustly than those using
of_find_device_by_node()  and checking if platform_get_drvdata() is
NULL.

Or is there some other way you had in mind of using
of_find_device_by_node() I've not thought of to implement this?

Thanks,

Peter.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel