mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Cristian Marussi <cristian.marussi@arm.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: core: add of_match_full_name boolean flag
Date: Wed, 19 Aug 2020 19:22:45 +0100	[thread overview]
Message-ID: <20200819182245.GE5441@sirena.org.uk> (raw)
In-Reply-To: <20200819140448.51373-1-cristian.marussi@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1975 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-08-19 18:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19 14:04 Cristian Marussi
2020-08-19 18:22 ` Mark Brown [this message]
2020-08-20 16:38   ` Cristian Marussi
2020-08-20 17:11     ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200819182245.GE5441@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=cristian.marussi@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®