Re: [PATCH] regulator: core: add of_match_full_name boolean flag

From: Mark Brown
Date: Wed Aug 19 2020 - 14:23:23 EST


On Wed, Aug 19, 2020 at 03:04:48PM +0100, Cristian Marussi wrote:

> Property 'regulator-compatible' is now deprecated (even if still widely
> used in the code base), and the node-name fallback works fine only as long

I'm seeing a very small number of DTs using it, the majority of which
are pretty old - the arm64 ones are just mistakes on the part of
reviewers.

> as the nodes are named in an unique way; if it makes sense to use a common
> name and identifying them using an index through a 'reg' property the
> standard advices to use a naming in the form <common-name>@<unit>.

> In this case the above matching mechanism based on the simple (common) name
> will fail and the only viable alternative would be to properly define the
> deprecrated 'regulator-compatible' property equal to the full name
> <common-name>@<unit>.

This seems like a massive jump. You appear to be saying that the reg
property is unusable which doesn't seem right to me?

> In order to address this case without using such deprecated property,
> define a new boolean flag .of_match_full_name in struct regulator_desc to
> force the core to match against the node full-name instead.

I can't tell from this description what this change is intended to do,
and I suspect it'd be difficult for anyone trying to figure out if they
should use this or not. What is a full name and what should people put
in there? What would one look like for example? I have to look at the
code to see that this is changing to compare against the full_name field
in the node and there's no kerneldoc for struct device_node.

I'm also wondering why we can't just add this to the list of fallbacks
rather than requiring some custom per driver thing?

> - name = child->name;
> + name = !desc->of_match_full_name ?
> + child->name : child->full_name;

Please write normal conditional statements for the benefits of people
who have to read this code, the extra ! in there isn't adding anything
here either.

Attachment: signature.asc
Description: PGP signature