All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Yunus Bas <Y.Bas@phytec.de>,
	"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: Wed, 30 Jun 2021 13:33:58 +0100	[thread overview]
Message-ID: <YNxktsFmlzLcn4+Y@dell> (raw)
In-Reply-To: <20210630105557.eaktwdz5p6yzuron@maple.lan>

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:
> > > On Tue, 29 Jun 2021, Yunus Bas wrote:
> > > > Interestingly, all subdevices defined in the driver are registered
> > > > as platform devices from the MFD framework, regardless of a
> > > > devicetree entry or not. The preceding code checks the subdevice
> > > > cells with an additional compatible. In case a device has no
> > > > devicetree entry, an irritating failed-message is printed on the
> > > > display. I'm not sure if this was the intention but the framework
> > > > somehow forces the users to describe all subdevices of an MFD. I
> > > > think the info print is not needed. It makes more sense to set it
> > > > as a debug print.
> > > 
> > > Actually, this has served to highlight that your DTS is not correct.
> > > 
> > > Why are some devices represented in DT and some aren't?
> > > 
> > > If anything, I'm tempted to upgrade the info() print to warn().
> > 
> > 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?
> 
> There is almost certainly a problem here since there is a mismatch
> between mfd-core and the DA9062 DT bindings. mfd-core warns when the
> hardware description is incomplete and the DA9062 (and generic mfd) DT
> bindings are ambiguous about whether sub-nodes are mandatory and include
> an example that contains missing compatibles rather than disabled nodes
> like the above.
> 
> However it is not entirely clear to me at this point whether this should
> be fixed in mfd-core or by improving the bindings documentation.

Right.  This is a potential solution.

NB: The suggestion above is usually the default for devices (at least
this was the case back when I was neck deep in DT).  You usually have
the a device specified in a DTSI file with the generic properties
defined from within a top-level node which is usually disabled.  Then
you link back to that node (usually with a &) from within your DTS
file where you provide platform specific properties and override the
status to 'okay' or what have you.

However before I provide any further assistance, I really want to get
an idea of the H/W you're working with.  Is this a reduced function
DA9062?  Or is the functionality actually present, you just don't want
to make use of it?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

  reply	other threads:[~2021-06-30 12:34 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 [this message]
2021-07-01 15:34                 ` Yunus Bas
2021-07-01 16:45                   ` Lee Jones
2021-07-02 12:59                   ` Daniel Thompson
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=YNxktsFmlzLcn4+Y@dell \
    --to=lee.jones@linaro.org \
    --cc=Y.Bas@phytec.de \
    --cc=daniel.thompson@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.