Re: [PATCH 3/3] mfd: syscon: Allow syscon nodes without a "syscon" compatible

John Madieu
2024-12-15T20:34:04Z

Hi Rob,

On Wed, 11 Dec 2024 14:57:14 -0600 Rob Herring wrote:
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index bfb1f69fcff1..e6df2825c14d 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -171,8 +171,10 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
> break;
> }
>
> - if (!syscon)
> + if (!syscon && of_device_is_compatible(np, "syscon"))
> syscon = of_syscon_register(np, check_res);
> + else
> + syscon = ERR_PTR(-EINVAL);

The current modification will make device_node_get_regmap() return -EINVAL even
for syscons that were already found in the syscon_list, which I believe is not
the intended behavior.

I suggest modifying it this way to maintain lookup functionality for registered
syscons while implementing your intended changes:

static struct regmap *device_node_get_regmap(struct device_node *np,
bool check_res)
{
struct syscon *entry, *syscon = NULL;
struct regmap *regmap;

mutex_lock(&syscon_list_lock);

list_for_each_entry(entry, &syscon_list, list)
if (entry->np == np) {
syscon = entry;
break;
}

if (syscon) {
regmap = syscon->regmap;
mutex_unlock(&syscon_list_lock);
return regmap;
}

if (of_device_is_compatible(np, "syscon")) {
syscon = of_syscon_register(np, check_res);
mutex_unlock(&syscon_list_lock);
if (IS_ERR(syscon))
return ERR_CAST(syscon);
return syscon->regmap;
}

mutex_unlock(&syscon_list_lock);
return ERR_PTR(-EINVAL);
}

The above is the resulting working function I've obtained while testing with
different scenarios. This ensures that:

1. Already registered syscons are found and returned correctly
2. New syscons with "syscon" compatible are registered as before
3. Nodes without "syscon" compatible return -EINVAL only if not found in the list

This has been tested with my v1 syscon work [1] and with v2, where I remove the
"syscon" compatible string for custom regmap initialization, aligning with your
goals for this series.

Let me know if I should add this series as a dependency of my v2 or how I should proceed.

Thanks,

John Madieu

[1] https://lore.kernel.org/all/20241206212559.192705-2-john.madieu.xa@bp.renesas.com