linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: Yunus Bas <Y.Bas@phytec.de>
Cc: "lee.jones@linaro.org" <lee.jones@linaro.org>,
	"stwiss.opensource@diasemi.com" <stwiss.opensource@diasemi.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mfd: mfd-core: Change "Failed to locate of_node" warning to debug
Date: Fri, 2 Jul 2021 13:59:20 +0100	[thread overview]
Message-ID: <20210702125920.fydyfhwqe7tyr7oi@maple.lan> (raw)
In-Reply-To: <9b5d0003cce92cad57e7712d1e46c78c10f1a0ab.camel@phytec.de>

On Thu, Jul 01, 2021 at 03:34:43PM +0000, Yunus Bas wrote:
> Am Mittwoch, dem 30.06.2021 um 13:33 +0100 schrieb Lee Jones:
> > On Wed, 30 Jun 2021, Daniel Thompson wrote:
> > 
> > > On Wed, Jun 30, 2021 at 07:27:32AM +0000, Yunus Bas wrote:
> > > > Am Dienstag, dem 29.06.2021 um 14:39 +0100 schrieb Lee Jones:
> > > > Imagine only required parts of the MFD is connected to the
> > > > designed
> > > > system and unrequired parts are not. In that case, fully
> > > > describing the
> > > > MFD in the devicetree wouldn't represent the system at all.
> > > 
> > > To describe hardware that is present but unused we would normally
> > > use
> > > status = "disabled".
> > > 
> > > So if, for example, your board cannot use the RTC for some reason
> > > (perhaps the board has no 32KHz oscillator?) then the DA9062 still
> > > contains the hardware but it is useless. Such hardware could be
> > > described as:
> > > 
> > > da9062_rtc: rtc {
> > >     compatible = "dlg,da9062-rtc";
> > >     status = "disabled";
> > > }
> > > 
> > > Is this sufficient to suppress the warnings when the hardware is
> > > not fully described?
<snip>
> > 
> > Right.  This is a potential solution.
> 
> @Daniel, you hit the nail on the head :). Thank you for that.
> 
> This solution would indeed surpress the warnings, but what is the
> benefit of this? We would define never used device nodes just to
> satisfy the driver.

I would say that doing so resolves an awkward ambiguity of
interpretation w.r.t. the bindings.

1. The MFD device compatible "dlg,da9062" tells the OS that we
   have an DA9062. An DA9062 contains six functions and this can be
   inferred *entirely* from the MFD compatible string. We do not
   need any subnodes to tell us that a DA9062 contains an RTC. The OS
   can (and in this case, does) already know that there is an RTC
   because we have a DA9062 (and a datasheet).

2. The default behaviour when a node has no status field is to
   assume that is is *enabled*.

Based on #1 and #2 above then assuming that a DT that omits the
sub-nodes actually means "disable the RTC" is risky. #2 might
actually make it more natural to assume that the device is present and
functional because there is no status field to tell MFD *not* to
initialize it.

That leaves us in a situation where there is no way to correctly guess
the authors intent when sub-nodes are omitted from the DT. Given this is
something of a corner case and the documentation is ambiguous then a
warning of the author does not clearly resolve the ambiguity seems
reasonable.


Daniel.

  parent reply	other threads:[~2021-07-02 12:59 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16  8:19 [PATCH] mfd: mfd-core: Change "Failed to locate of_node" warning to debug Yunus Bas
2021-06-16  9:03 ` Lee Jones
2021-06-17  7:46   ` Yunus Bas
2021-06-17  8:27     ` Lee Jones
2021-06-29  7:25       ` Yunus Bas
2021-06-29  9:07         ` Lee Jones
2021-06-29  9:41           ` Yunus Bas
2021-06-29 13:39         ` Lee Jones
2021-06-30  7:27           ` Yunus Bas
2021-06-30  8:42             ` Lee Jones
2021-06-30 10:55             ` Daniel Thompson
2021-06-30 12:33               ` Lee Jones
2021-07-01 15:34                 ` Yunus Bas
2021-07-01 16:45                   ` Lee Jones
2021-07-02 12:59                   ` Daniel Thompson [this message]
2021-07-02 18:36                     ` Lee Jones
2021-07-02 19:10                       ` Daniel Thompson
2021-07-05  7:24                         ` Yunus Bas
2021-07-05  7:31                           ` Lee Jones
2021-07-05  7:50                             ` Yunus Bas
2021-07-05  8:05                               ` Lee Jones

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=20210702125920.fydyfhwqe7tyr7oi@maple.lan \
    --to=daniel.thompson@linaro.org \
    --cc=Y.Bas@phytec.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stwiss.opensource@diasemi.com \
    /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;
as well as URLs for NNTP newsgroup(s).