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>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
- Keep syscon lock held between checking and adding entry (Krzysztof)
- Link to v1 https://lore.kernel.org/linux-arm-kernel/20240614140421.3172674-2-peter.griffin@linaro.org/
---
 drivers/mfd/syscon.c       | 54 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/syscon.h |  8 ++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 7d0e91164cba..75379e089b6b 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -192,6 +192,60 @@ 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;
+	int ret;
+
+	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;
+		}
+
+	if (syscon) {
+		ret = -EEXIST;
+		goto err_unlock;
+	}
+
+	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
+	if (!syscon) {
+		ret = -ENOMEM;
+		goto err_unlock;
+	}
+
+	syscon->regmap = regmap;
+	syscon->np = np;
+
+	/* register the regmap in syscon list */
+	list_add_tail(&syscon->list, &syscon_list);
+	spin_unlock(&syscon_list_slock);
+
+	return 0;
+
+err_unlock:
+	spin_unlock(&syscon_list_slock);
+	return ret;
+}
+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

On Thu, Jun 20, 2024, at 13:24, Peter Griffin wrote:
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v2:
> - Keep syscon lock held between checking and adding entry (Krzysztof)

Unfortunately you now have a different bug:

> +	/* 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;
> +		}
> +
> +	if (syscon) {
> +		ret = -EEXIST;
> +		goto err_unlock;
> +	}
> +
> +	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> +	if (!syscon) {
> +		ret = -ENOMEM;
> +		goto err_unlock;
> +	}

You can't use GFP_KERNEL while holding a spinlock.

I think the correct way to do this is to move the allocation
before the locked section, and then free it in the failure case.

     Arnd

Hi Arnd,

Thanks for the review feedback.

On Thu, 20 Jun 2024 at 12:40, Arnd Bergmann <arnd@arndb.de> wrote:
>
>
> On Thu, Jun 20, 2024, at 13:24, Peter Griffin wrote:
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> > Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> > Changes in v2:
> > - Keep syscon lock held between checking and adding entry (Krzysztof)
>
> Unfortunately you now have a different bug:
>
> > +     /* 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;
> > +             }
> > +
> > +     if (syscon) {
> > +             ret = -EEXIST;
> > +             goto err_unlock;
> > +     }
> > +
> > +     syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
> > +     if (!syscon) {
> > +             ret = -ENOMEM;
> > +             goto err_unlock;
> > +     }
>
> You can't use GFP_KERNEL while holding a spinlock.
>
> I think the correct way to do this is to move the allocation
> before the locked section, and then free it in the failure case.

Thanks for spotting this! I'll update as you suggest in v3.

Peter.