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

[Krzysztof Kozlowski <krzk@kernel.org>]

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