Hi Krzysztof,
Thanks for your review feedback.
On Wed, 19 Jun 2024 at 07:29, Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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.
[..]
