[PATCH 0/2] Add syscon of_syscon_register_regmap api

[PATCH 0/2] Add syscon of_syscon_register_regmap api

[Peter Griffin]

Hi Lee, Arnd, Krzysztof, all,

This series adds support to syscon driver for a new of_syscon_register_regmap()
api.

Platforms such as gs101 require a special regmap to access PMU registers, which
in the existing upstream client drivers are accessed via syscon regmap. This
issue was partly solved in [1] whereby a custom regmap is created in exynos-pmu
and a new API exynos_get_pmu_regmap_by_phandle() created.

One issue with the approach in [1] is that it required client drivers to be
updated from syscon_regmap_lookup_by_phandle() to
exynos_get_pmu_regmap_by_phandle() when obtaining the regmap.

Whilst updating to exynos_get_pmu_regmap_by_phandle() was OK for exynos
specific drivers, it meant other drivers like syscon-reboot and syscon-poweroff
which span multiple SoC architectures could not be easily re-used.

In previous review feedback for USB phy and gs101 poweroff driver Krzysztof
requested [2] that we take a more generic approach that other SoCs can also
leverage.

The new of_syscon_register_regmap() api overcomes this limitation by allowing
a SoC driver like exynos-pmu to register it's SoC specific regmap with the
syscon driver. This keeps the SoC complexity out of syscon driver, and allows
client drivers to continue using syscon_regmap_lookup_by_phandle() as before.
The solution allows more code re-use and can be used by other SoC archs.

Notes on probe ordering

exynos-pmu runs at postcore_initcall, so all but one of the client drivers
(ufs phy, usb phy, watchdog) run after the regmap is created and registered.

The one exception to this is pinctrl-samsung driver which is also
postcore_initcall level. The exynos_get_pmu_regmap() and
exynos_get_pmu_regmap_by_phandle() have been temporarily left to support
-EPROBE_DEFER for pinctrl-samsung driver.

The longer term plan to solve that probe ordering issue is to enable
fw_devlink for syscon dt properties so they are correctly listed as
suppliers in /sys/class/devlink. I tested a PoC patch (see below) for
fw_devlink and that seemed to work fine. Once fw_devlink supports syscon I
believe exynos_get_pmu_regmap_by_phandle() api could be removed. The main issue
currently with fw_devlink syscon support is the wide diversity of dt property
naming currently in use. That was discussed previously here [3]

1248a1256,1257
> DEFINE_SUFFIX_PROP(syscon_phandle, "syscon-phandle", NULL)
> DEFINE_SUFFIX_PROP(pmu_syscon, "pmu-syscon", NULL)
1358a1368,1369
>     { .parse_prop = parse_syscon_phandle, },
>     { .parse_prop = parse_pmu_syscon, },


Note one previous concern from Saravana about syscon potentially probing
before exynos-pmu driver and it relying on drivers/Makefile ordering. I tested
this and even if mfd is listed before soc in drivers/Makefile exynos-pmu
always probes first due to syscon driver not setting a .of_match_table entry.

Once the syscon and exynos-pmu patchs are queued I will send patches for
watchdog and ufs phy driver to switch back to syscon_regmap_lookup_by_phandle()

Many thanks,

Peter.

[1] https://lore.kernel.org/linux-arm-kernel/20240219204238.356942-1-peter.griffin@linaro.org/T/
[2] https://lore.kernel.org/lkml/06383015-51b2-4f4c-9fd8-e4f7ce12f44e@kernel.org/
[3] https://lore.kernel.org/all/CAGETcx-CCpaV7R0O0HpDpoX6KxQBuJiMmKdWA8nDE-5Qj2Sa7g@mail.gmail.com/


Peter Griffin (2):
  mfd: syscon: add of_syscon_register_regmap() API
  soc: samsung: exynos-pmu: update to use of_syscon_register_regmap()

 drivers/mfd/syscon.c             | 48 ++++++++++++++++++++++++++++++++
 drivers/soc/samsung/exynos-pmu.c | 38 ++++++++++---------------
 include/linux/mfd/syscon.h       |  8 ++++++
 3 files changed, 70 insertions(+), 24 deletions(-)

-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH 1/2] mfd: syscon: add of_syscon_register_regmap() API

[Peter Griffin]

The of_syscon_register_regmap() API allows an externally created regmap
to be registered with syscon. This regmap can then be returned to client
drivers using the syscon_regmap_lookup_by_phandle() APIs.

The API is used by platforms where mmio access to the syscon registers is
not possible, and a underlying soc driver like exynos-pmu provides a SoC
specific regmap that can issue a SMC or hypervisor call to write the
register.

This approach keeps the SoC complexities out of syscon, but allows common
drivers such as  syscon-poweroff, syscon-reboot and friends that are used
by many SoCs already to be re-used.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/mfd/syscon.c       | 48 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/syscon.h |  8 +++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 7d0e91164cba..44991da3ea23 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -192,6 +192,54 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
 	return syscon->regmap;
 }
 
+/**
+ * of_syscon_register_regmap() - Register regmap for specified device node
+ * @np: Device tree node
+ * @regmap: Pointer to regmap object
+ *
+ * Register an externally created regmap object with syscon for the specified
+ * device tree node. This regmap can then be returned to client drivers using
+ * the syscon_regmap_lookup_by_phandle() API.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int of_syscon_register_regmap(struct device_node *np, struct regmap *regmap)
+{
+	struct syscon  *entry, *syscon = NULL;
+
+	if (!np || !regmap)
+		return -EINVAL;
+
+	/* check if syscon entry already exists */
+	spin_lock(&syscon_list_slock);
+
+	list_for_each_entry(entry, &syscon_list, list)
+		if (entry->np == np) {
+			syscon = entry;
+			break;
+		}
+
+	spin_unlock(&syscon_list_slock);
+
+	if (syscon)
+		return -EEXIST;
+
+	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
+	if (!syscon)
+		return -ENOMEM;
+
+	syscon->regmap = regmap;
+	syscon->np = np;
+
+	/* register the regmap in syscon list */
+	spin_lock(&syscon_list_slock);
+	list_add_tail(&syscon->list, &syscon_list);
+	spin_unlock(&syscon_list_slock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_syscon_register_regmap);
+
 struct regmap *device_node_to_regmap(struct device_node *np)
 {
 	return device_node_get_regmap(np, false);
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index c315903f6dab..aad9c6b50463 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -28,6 +28,8 @@ struct regmap *syscon_regmap_lookup_by_phandle_args(struct device_node *np,
 						    unsigned int *out_args);
 struct regmap *syscon_regmap_lookup_by_phandle_optional(struct device_node *np,
 							const char *property);
+int of_syscon_register_regmap(struct device_node *np,
+			      struct regmap *regmap);
 #else
 static inline struct regmap *device_node_to_regmap(struct device_node *np)
 {
@@ -67,6 +69,12 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle_optional(
 	return NULL;
 }
 
+static inline int of_syscon_register_regmap(struct device_node *np,
+					struct regmap *regmap)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif
 
 #endif /* __LINUX_MFD_SYSCON_H__ */
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH 2/2] soc: samsung: exynos-pmu: update to use of_syscon_register_regmap()

[Peter Griffin]

For SoCs like gs101 that need a special regmap, register this with
of_syscon_register_regmap api, so it can be returned by
syscon_regmap_lookup_by_phandle() and friends.

For SoCs that don't require a custom regmap, revert back to syscon
creating the mmio regmap rather than duplicating the logic here.

exynos_get_pmu_regmap_by_phandle() api is also updated to retrieve
the regmap via syscon. The exynos_get_pmu_regmap_by_phandle() api
is kept around until fw_devlink support for syscon property is added
for the pinctrl-samsung driver that also runs at postcore_initcall
level.

All other exynos client drivers can revert back to
syscon_regmap_lookup_by_phandle().

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/soc/samsung/exynos-pmu.c | 38 ++++++++++++--------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index fd8b6ac06656..91d7f4121a72 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -204,16 +204,6 @@ static const struct regmap_config regmap_smccfg = {
 	.reg_update_bits = tensor_sec_update_bits,
 };
 
-static const struct regmap_config regmap_mmiocfg = {
-	.name = "pmu_regs",
-	.reg_bits = 32,
-	.reg_stride = 4,
-	.val_bits = 32,
-	.fast_io = true,
-	.use_single_read = true,
-	.use_single_write = true,
-};
-
 static const struct exynos_pmu_data gs101_pmu_data = {
 	.pmu_secure = true
 };
@@ -290,7 +280,6 @@ EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap);
 struct regmap *exynos_get_pmu_regmap_by_phandle(struct device_node *np,
 						const char *propname)
 {
-	struct exynos_pmu_context *ctx;
 	struct device_node *pmu_np;
 	struct device *dev;
 
@@ -316,9 +305,7 @@ struct regmap *exynos_get_pmu_regmap_by_phandle(struct device_node *np,
 	if (!dev)
 		return ERR_PTR(-EPROBE_DEFER);
 
-	ctx = dev_get_drvdata(dev);
-
-	return ctx->pmureg;
+	return syscon_node_to_regmap(np);
 }
 EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap_by_phandle);
 
@@ -355,19 +342,22 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 		regmap = devm_regmap_init(dev, NULL,
 					  (void *)(uintptr_t)res->start,
 					  &pmu_regmcfg);
+
+		if (IS_ERR(regmap))
+			return dev_err_probe(&pdev->dev, PTR_ERR(regmap),
+					     "regmap init failed\n");
+
+		ret = of_syscon_register_regmap(dev->of_node, regmap);
+		if (ret)
+			return ret;
 	} else {
-		/* All other SoCs use a MMIO regmap */
-		pmu_regmcfg = regmap_mmiocfg;
-		pmu_regmcfg.max_register = resource_size(res) -
-					   pmu_regmcfg.reg_stride;
-		regmap = devm_regmap_init_mmio(dev, pmu_base_addr,
-					       &pmu_regmcfg);
+		/* let syscon create mmio regmap */
+		regmap = syscon_node_to_regmap(dev->of_node);
+		if (IS_ERR(regmap))
+			return dev_err_probe(&pdev->dev, PTR_ERR(regmap),
+					     "syscon_node_to_regmap failed\n");
 	}
 
-	if (IS_ERR(regmap))
-		return dev_err_probe(&pdev->dev, PTR_ERR(regmap),
-				     "regmap init failed\n");
-
 	pmu_context->pmureg = regmap;
 	pmu_context->dev = dev;
 
-- 
2.45.2.627.g7a2c4fd464-goog

Re: [PATCH 1/2] mfd: syscon: add of_syscon_register_regmap() API

[André Draszik]

On Fri, 2024-06-14 at 15:04 +0100, Peter Griffin wrote:
> The of_syscon_register_regmap() API allows an externally created regmap
> to be registered with syscon. This regmap can then be returned to client
> drivers using the syscon_regmap_lookup_by_phandle() APIs.
> 
> The API is used by platforms where mmio access to the syscon registers is
> not possible, and a underlying soc driver like exynos-pmu provides a SoC
> specific regmap that can issue a SMC or hypervisor call to write the
> register.
> 
> This approach keeps the SoC complexities out of syscon, but allows common
> drivers such as  syscon-poweroff, syscon-reboot and friends that are used
> by many SoCs already to be re-used.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

With this series, the exynos5 usb phy driver works on gs101 without
having to patch it to use exynos_get_pmu_regmap_by_phandle() instead
of the standard syscon_regmap_lookup_by_phandle():

Tested-by: André Draszik <andre.draszik@linaro.org>

Re: [PATCH 2/2] soc: samsung: exynos-pmu: update to use of_syscon_register_regmap()

[André Draszik]

On Fri, 2024-06-14 at 15:04 +0100, Peter Griffin wrote:
> For SoCs like gs101 that need a special regmap, register this with
> of_syscon_register_regmap api, so it can be returned by
> syscon_regmap_lookup_by_phandle() and friends.
> 
> For SoCs that don't require a custom regmap, revert back to syscon
> creating the mmio regmap rather than duplicating the logic here.
> 
> exynos_get_pmu_regmap_by_phandle() api is also updated to retrieve
> the regmap via syscon. The exynos_get_pmu_regmap_by_phandle() api
> is kept around until fw_devlink support for syscon property is added
> for the pinctrl-samsung driver that also runs at postcore_initcall
> level.
> 
> All other exynos client drivers can revert back to
> syscon_regmap_lookup_by_phandle().
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

With this series, the exynos5 usb phy driver works on gs101 without
having to patch it to use exynos_get_pmu_regmap_by_phandle() instead
of the standard syscon_regmap_lookup_by_phandle():

Tested-by: André Draszik <andre.draszik@linaro.org>

Re: [PATCH 2/2] soc: samsung: exynos-pmu: update to use of_syscon_register_regmap()

[Arnd Bergmann]

On Fri, Jun 14, 2024, at 16:04, Peter Griffin wrote:
> For SoCs like gs101 that need a special regmap, register this with
> of_syscon_register_regmap api, so it can be returned by
> syscon_regmap_lookup_by_phandle() and friends.
>
> For SoCs that don't require a custom regmap, revert back to syscon
> creating the mmio regmap rather than duplicating the logic here.
>
> exynos_get_pmu_regmap_by_phandle() api is also updated to retrieve
> the regmap via syscon. The exynos_get_pmu_regmap_by_phandle() api
> is kept around until fw_devlink support for syscon property is added
> for the pinctrl-samsung driver that also runs at postcore_initcall
> level.
>
> All other exynos client drivers can revert back to
> syscon_regmap_lookup_by_phandle().
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/2] mfd: syscon: add of_syscon_register_regmap() API

[Arnd Bergmann]

On Fri, Jun 14, 2024, at 16:04, Peter Griffin wrote:
> The of_syscon_register_regmap() API allows an externally created regmap
> to be registered with syscon. This regmap can then be returned to client
> drivers using the syscon_regmap_lookup_by_phandle() APIs.
>
> The API is used by platforms where mmio access to the syscon registers is
> not possible, and a underlying soc driver like exynos-pmu provides a SoC
> specific regmap that can issue a SMC or hypervisor call to write the
> register.
>
> This approach keeps the SoC complexities out of syscon, but allows common
> drivers such as  syscon-poweroff, syscon-reboot and friends that are used
> by many SoCs already to be re-used.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 2/2] soc: samsung: exynos-pmu: update to use of_syscon_register_regmap()

[Sam Protsenko]

On Fri, Jun 14, 2024 at 9:04 AM Peter Griffin <peter.griffin@linaro.org> wrote:
>
> For SoCs like gs101 that need a special regmap, register this with
> of_syscon_register_regmap api, so it can be returned by
> syscon_regmap_lookup_by_phandle() and friends.
>
> For SoCs that don't require a custom regmap, revert back to syscon
> creating the mmio regmap rather than duplicating the logic here.
>
> exynos_get_pmu_regmap_by_phandle() api is also updated to retrieve
> the regmap via syscon. The exynos_get_pmu_regmap_by_phandle() api
> is kept around until fw_devlink support for syscon property is added
> for the pinctrl-samsung driver that also runs at postcore_initcall
> level.
>
> All other exynos client drivers can revert back to
> syscon_regmap_lookup_by_phandle().
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---

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

[snip]

Re: [PATCH 1/2] mfd: syscon: add of_syscon_register_regmap() API

[Sam Protsenko]

On Fri, Jun 14, 2024 at 9:04 AM Peter Griffin <peter.griffin@linaro.org> wrote:
>
> The of_syscon_register_regmap() API allows an externally created regmap
> to be registered with syscon. This regmap can then be returned to client
> drivers using the syscon_regmap_lookup_by_phandle() APIs.
>
> The API is used by platforms where mmio access to the syscon registers is
> not possible, and a underlying soc driver like exynos-pmu provides a SoC
> specific regmap that can issue a SMC or hypervisor call to write the
> register.
>
> This approach keeps the SoC complexities out of syscon, but allows common
> drivers such as  syscon-poweroff, syscon-reboot and friends that are used
> by many SoCs already to be re-used.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---

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

>  drivers/mfd/syscon.c       | 48 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/syscon.h |  8 +++++++
>  2 files changed, 56 insertions(+)
>
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 7d0e91164cba..44991da3ea23 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -192,6 +192,54 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
>         return syscon->regmap;
>  }
>
> +/**
> + * of_syscon_register_regmap() - Register regmap for specified device node
> + * @np: Device tree node
> + * @regmap: Pointer to regmap object
> + *
> + * Register an externally created regmap object with syscon for the specified
> + * device tree node. This regmap can then be returned to client drivers using
> + * the syscon_regmap_lookup_by_phandle() API.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int of_syscon_register_regmap(struct device_node *np, struct regmap *regmap)
> +{
> +       struct syscon  *entry, *syscon = NULL;
> +
> +       if (!np || !regmap)
> +               return -EINVAL;
> +
> +       /* check if syscon entry already exists */
> +       spin_lock(&syscon_list_slock);
> +
> +       list_for_each_entry(entry, &syscon_list, list)
> +               if (entry->np == np) {
> +                       syscon = entry;
> +                       break;
> +               }
> +
> +       spin_unlock(&syscon_list_slock);
> +
> +       if (syscon)
> +               return -EEXIST;
> +
> +       syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> +       if (!syscon)
> +               return -ENOMEM;
> +
> +       syscon->regmap = regmap;
> +       syscon->np = np;
> +
> +       /* register the regmap in syscon list */
> +       spin_lock(&syscon_list_slock);
> +       list_add_tail(&syscon->list, &syscon_list);
> +       spin_unlock(&syscon_list_slock);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_syscon_register_regmap);
> +
>  struct regmap *device_node_to_regmap(struct device_node *np)
>  {
>         return device_node_get_regmap(np, false);
> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index c315903f6dab..aad9c6b50463 100644
> --- a/include/linux/mfd/syscon.h
> +++ b/include/linux/mfd/syscon.h
> @@ -28,6 +28,8 @@ struct regmap *syscon_regmap_lookup_by_phandle_args(struct device_node *np,
>                                                     unsigned int *out_args);
>  struct regmap *syscon_regmap_lookup_by_phandle_optional(struct device_node *np,
>                                                         const char *property);
> +int of_syscon_register_regmap(struct device_node *np,
> +                             struct regmap *regmap);
>  #else
>  static inline struct regmap *device_node_to_regmap(struct device_node *np)
>  {
> @@ -67,6 +69,12 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle_optional(
>         return NULL;
>  }
>
> +static inline int of_syscon_register_regmap(struct device_node *np,
> +                                       struct regmap *regmap)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
>  #endif
>
>  #endif /* __LINUX_MFD_SYSCON_H__ */
> --
> 2.45.2.627.g7a2c4fd464-goog
>

Re: [PATCH 1/2] mfd: syscon: add of_syscon_register_regmap() API

[Krzysztof Kozlowski]

On 14/06/2024 16:04, Peter Griffin wrote:
> The of_syscon_register_regmap() API allows an externally created regmap
> to be registered with syscon. This regmap can then be returned to client
> drivers using the syscon_regmap_lookup_by_phandle() APIs.
> 
> The API is used by platforms where mmio access to the syscon registers is
> not possible, and a underlying soc driver like exynos-pmu provides a SoC
> specific regmap that can issue a SMC or hypervisor call to write the
> register.
> 
> This approach keeps the SoC complexities out of syscon, but allows common
> drivers such as  syscon-poweroff, syscon-reboot and friends that are used
> by many SoCs already to be re-used.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  drivers/mfd/syscon.c       | 48 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/syscon.h |  8 +++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 7d0e91164cba..44991da3ea23 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -192,6 +192,54 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
>  	return syscon->regmap;
>  }
>  
> +/**
> + * of_syscon_register_regmap() - Register regmap for specified device node
> + * @np: Device tree node
> + * @regmap: Pointer to regmap object
> + *
> + * Register an externally created regmap object with syscon for the specified
> + * device tree node. This regmap can then be returned to client drivers using
> + * the syscon_regmap_lookup_by_phandle() API.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int of_syscon_register_regmap(struct device_node *np, struct regmap *regmap)
> +{
> +	struct syscon  *entry, *syscon = NULL;
> +
> +	if (!np || !regmap)
> +		return -EINVAL;
> +
> +	/* check if syscon entry already exists */
> +	spin_lock(&syscon_list_slock);
> +
> +	list_for_each_entry(entry, &syscon_list, list)
> +		if (entry->np == np) {
> +			syscon = entry;
> +			break;
> +		}
> +
> +	spin_unlock(&syscon_list_slock);
> +
> +	if (syscon)
> +		return -EEXIST;
> +
> +	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> +	if (!syscon)
> +		return -ENOMEM;
> +
> +	syscon->regmap = regmap;
> +	syscon->np = np;
> +
> +	/* register the regmap in syscon list */
> +	spin_lock(&syscon_list_slock);

You still have window between the check for existing syscon and adding
to the list. This likely is not an issue now, but it might if we have
more devices using same syscon and we enable asynchronous probing.

> +	list_add_tail(&syscon->list, &syscon_list);
> +	spin_unlock(&syscon_list_slock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_syscon_register_regmap);
> +


Best regards,
Krzysztof

Re: [PATCH 1/2] mfd: syscon: add of_syscon_register_regmap() API

[Peter Griffin]

Hi Krzysztof,

Thanks for your review feedback.

On Wed, 19 Jun 2024 at 07:29, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 14/06/2024 16:04, Peter Griffin wrote:
> > The of_syscon_register_regmap() API allows an externally created regmap
> > to be registered with syscon. This regmap can then be returned to client
> > drivers using the syscon_regmap_lookup_by_phandle() APIs.
> >
> > The API is used by platforms where mmio access to the syscon registers is
> > not possible, and a underlying soc driver like exynos-pmu provides a SoC
> > specific regmap that can issue a SMC or hypervisor call to write the
> > register.
> >
> > This approach keeps the SoC complexities out of syscon, but allows common
> > drivers such as  syscon-poweroff, syscon-reboot and friends that are used
> > by many SoCs already to be re-used.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/mfd/syscon.c       | 48 ++++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/syscon.h |  8 +++++++
> >  2 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> > index 7d0e91164cba..44991da3ea23 100644
> > --- a/drivers/mfd/syscon.c
> > +++ b/drivers/mfd/syscon.c
> > @@ -192,6 +192,54 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
> >       return syscon->regmap;
> >  }
> >
> > +/**
> > + * of_syscon_register_regmap() - Register regmap for specified device node
> > + * @np: Device tree node
> > + * @regmap: Pointer to regmap object
> > + *
> > + * Register an externally created regmap object with syscon for the specified
> > + * device tree node. This regmap can then be returned to client drivers using
> > + * the syscon_regmap_lookup_by_phandle() API.
> > + *
> > + * Return: 0 on success, negative error code on failure.
> > + */
> > +int of_syscon_register_regmap(struct device_node *np, struct regmap *regmap)
> > +{
> > +     struct syscon  *entry, *syscon = NULL;
> > +
> > +     if (!np || !regmap)
> > +             return -EINVAL;
> > +
> > +     /* check if syscon entry already exists */
> > +     spin_lock(&syscon_list_slock);
> > +
> > +     list_for_each_entry(entry, &syscon_list, list)
> > +             if (entry->np == np) {
> > +                     syscon = entry;
> > +                     break;
> > +             }
> > +
> > +     spin_unlock(&syscon_list_slock);
> > +
> > +     if (syscon)
> > +             return -EEXIST;
> > +
> > +     syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> > +     if (!syscon)
> > +             return -ENOMEM;
> > +
> > +     syscon->regmap = regmap;
> > +     syscon->np = np;
> > +
> > +     /* register the regmap in syscon list */
> > +     spin_lock(&syscon_list_slock);
>
> You still have window between the check for existing syscon and adding
> to the list. This likely is not an issue now, but it might if we have
> more devices using same syscon and we enable asynchronous probing.

Good point, I will update it so that the lock is held throughout for
the check, and also adding it to the list.

Thanks,

Peter.

[..]