| 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
| ||
|
Message-ID: <YMsHXEP36Vxr7lAb@dell>
Date: Thu, 17 Jun 2021 09:27:08 +0100
From: Lee Jones <lee.jones@...aro.org>
To: Yunus Bas <Y.Bas@...tec.de>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Steve Twiss <stwiss.opensource@...semi.com>
Subject: Re: [PATCH] mfd: mfd-core: Change "Failed to locate of_node" warning
to debug
On Thu, 17 Jun 2021, Yunus Bas wrote:
> Hi Lee,
>
> Am Mittwoch, dem 16.06.2021 um 10:03 +0100 schrieb Lee Jones:
> > On Wed, 16 Jun 2021, Yunus Bas wrote:
> >
> > > The MFD-core iterates through all subdevices of the corresponding
> > > MFD-device and checks, if the devicetree subnode has a fitting
> > > compatible.
> > > When nothing is found, a warning is thrown. This can be the case,
> > > when it
> > > is the intention to not use the MFD-device to it's full content.
> > > Therefore, change the warning to a debug print instead, to also avoid
> > > irritations.
> > >
> > > Signed-off-by: Yunus Bas <y.bas@...tec.de>
> > > ---
> > > drivers/mfd/mfd-core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> > > index 6f02b8022c6d..e34c97088943 100644
> > > --- a/drivers/mfd/mfd-core.c
> > > +++ b/drivers/mfd/mfd-core.c
> > > @@ -213,7 +213,7 @@ static int mfd_add_device(struct device *parent,
> > > int id,
> > > }
> > >
> > > if (!pdev->dev.of_node)
> > > - pr_warn("%s: Failed to locate of_node [id:
> > > %d]\n",
> > > + pr_debug("%s: Failed to locate of_node [id:
> > > %d]\n",
> > > cell->name, platform_id);
> > > }
> >
> > Can you provide an example of a device tree where this is a problem?
>
> Of course, sorry for the poor description.
>
> Here is an example of the imx6qdl-phytec-phycore-som.dtsi which uses
> the DA9062 multi-functional device. The DA9062 has five mfd-cell
> devices with compatibles defined as subfunctions. The devicetree needs
> and uses just three of them:
>
> ...
> pmic: pmic@58 {
> compatible = "dlg,da9062";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_pmic>;
> reg = <0x58>;
> interrupt-parent = <&gpio1>;
> interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> interrupt-controller;
> gpio-controller;
> #gpio-cells = <2>;
>
> da9062_rtc: rtc {
> compatible = "dlg,da9062-rtc";
> };
>
> da9062_onkey: onkey {
> compatible = "dlg,da9062-onkey";
> };
>
> watchdog {
> compatible = "dlg,da9062-watchdog";
> dlg,use-sw-pm;
> }
> ...
So, looking at the mfd_cells table, I see:
static const struct mfd_cell da9061_devs[] = {
{
.name = "da9061-core",
.num_resources = ARRAY_SIZE(da9061_core_resources),
.resources = da9061_core_resources,
},
{
.name = "da9062-regulators",
.num_resources = ARRAY_SIZE(da9061_regulators_resources),
.resources = da9061_regulators_resources,
},
{
.name = "da9061-watchdog",
.num_resources = ARRAY_SIZE(da9061_wdt_resources),
.resources = da9061_wdt_resources,
.of_compatible = "dlg,da9061-watchdog",
},
{
.name = "da9061-thermal",
.num_resources = ARRAY_SIZE(da9061_thermal_resources),
.resources = da9061_thermal_resources,
.of_compatible = "dlg,da9061-thermal",
},
{
.name = "da9061-onkey",
.num_resources = ARRAY_SIZE(da9061_onkey_resources),
.resources = da9061_onkey_resources,
.of_compatible = "dlg,da9061-onkey",
},
};
Not sure why "da9061-core" is even in there. It looks like this would
be referencing itself (if this driver's name contained the "-core"
element). So what from I can tell, I think this entry should actually
just be removed.
With regards to "da9062-regulators", this looks like an oversight at
best or a Linux hack at worst. Device Tree is designed to be OS
agnostic. It should describe the H/W as-is, which would include the
Regulator functionality. Choosing to opt-out and instead use Linux
specific systems (i.e. MFD) for device registration is a hack.
I've always said we should not mix DT and MFD in this way.
> Since the driver iterates through the mfd_cells-struct tries matching
> compatibles in the devicetree MFD node, it throws a warning when there
> is no counterpart in the devicetree.
>
> In fact, I could also evalutate oder devicetree's using MFD-devices not
> to it's full content.
>
> >
> > Probably worth popping that in the commit message too.
>
> Yes, I will send a v2 ASAP. Thank you for the advice.
I think the current code is fine as it is.
It's the implementation that needs to change.
Maybe Steve would like to comment?
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Powered by blists - more mailing lists