linux-kernel - Re: [PATCH] regulator: core: add of_match_full

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200819182245.GE5441@sirena.org.uk>
Date:   Wed, 19 Aug 2020 19:22:45 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Cristian Marussi <cristian.marussi@....com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regulator: core: add of_match_full_name boolean flag

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)