linux-kernel - Re: [PATCH RESEND] mfd: Add Freescale i.MX8qxp Control and Status Registers (CSR) module driver

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: <CAL_JsqKiKgQ31gjihWs5HR_Bc7j=D7-QDn=jhoy6E5mnMSk-kA@mail.gmail.com>
Date:   Tue, 15 Nov 2022 07:33:37 -0600
From:   Rob Herring <robh@...nel.org>
To:     Liu Ying <victor.liu@....com>
Cc:     Lee Jones <lee@...nel.org>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, shawnguo@...nel.org,
        s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
        linux-imx@....com
Subject: Re: [PATCH RESEND] mfd: Add Freescale i.MX8qxp Control and Status
 Registers (CSR) module driver

On Mon, Nov 14, 2022 at 11:22 PM Liu Ying <victor.liu@....com> wrote:
> On Mon, 2022-11-14 at 14:54 -0600, Rob Herring wrote:
> > On Mon, Nov 7, 2022 at 9:58 PM Liu Ying <victor.liu@....com> wrote:
> > > Hi Lee,
> > >
> > > On Mon, 2022-11-07 at 09:05 +0000, Lee Jones wrote:
> > > > On Wed, 02 Nov 2022, Liu Ying wrote:
> > > >
> > > > > Hi Lee,
> > > > >
> > > > > On Tue, 2022-11-01 at 13:53 +0800, Liu Ying wrote:
> > > > > > Hi Lee,
> > > > > >
> > > > > > On Mon, 2022-10-31 at 15:40 +0000, Lee Jones wrote:
> > > > > > > On Mon, 17 Oct 2022, Liu Ying wrote:
> > > > > > >

[...]

> > > > > > > > +     ret = devm_of_platform_populate(&pdev->dev);
> > > > > > >
> > > > > > > The use of this API does not constitute a MFD.
> > > > > > >
> > > > > > > Please use "simple-mfd" instead.
> > > > > >
> > > > > > simple-mfd devices have "ONLY_BUS" set in simple-pm-bus.c, so the
> > > > > > simple-pm-bus driver would not populate child devices of simple-mfd
> > > > > > devices.
> > > >
> > > > Right, simple-pm-bus will not populate child devices, because:
> > >
> > > simple-pm-bus.c may populate child devices of simple-pm-bus devices
> > > because "ONLY_BUS" is _not_ set for simple-pm-bus devices.
> > >
> > > simple-pm-bus.c would _not_ populate child devices of simple-mfd
> > > devices because "ONLY_BUS" is set for simple-mfd devices.
> > >
> > > >   /*
> > > >    * These are transparent bus devices (not simple-pm-bus matches) that
> > > >    * have their child nodes populated automatically.  So, don't need to
> > > >    * do anything more. We only match with the device if this driver is
> > > >    * the most specific match because we don't want to incorrectly bind to
> > > >    * a device that has a more specific driver.
> > > >    */
> > > >
> > > > So "simple-mfd" must be populated elsewhere i.e. drivers/of/platform.c.
> > >
> > > If simple-mfd device is a child device of one device listed in
> > > of_default_bus_match_table[], then it may be populated by
> > > drivers/of/platform.c.  But, in my case, simple-mfd device is a child
> > > device of simple-pm-bus device(not listed in that table), so it will
> > > not be populated by drivers/of/platform.c.  Instead,
> > > drivers/bus/simple-pm-bus.c would populate the simple-mfd device.
> > >
> > > > > > Also, the simple-pm-bus driver would not enable runtime
> > > > > > power management for simple-mfd devices due to "ONLY_BUS", which
> > > > > > means it would not propagate power management operations from child
> > > > > > devices of simple-mfd devices to parent devices of simple-mfd
> > > > > > devices.  That's why a dedicated fsl-imx8qxp-csr driver is needed.
> > > >
> > > > This is more of an issue.
> > > >
> > > > Why can't this device use "simple-pm-bus" to have support for both
> > > > auto-registration AND Runtime PM?
> > >
> > > If I change the compatible string of the CSR module from
> > > "fsl,imx8qxp-mipi-lvds-csr", "syscon", "simple-mfd"
> > > to
> > > "fsl,imx8qxp-mipi-lvds-csr", "syscon", "simple-pm-bus",
> > > all devices I'm interested in are populated and all relevant drivers
> > > can probe.  But, this change makes 'make dt_binding_check' for the
> > > existing fsl,imx8qxp-csr.yaml fail:
> >
> > As 'simple-bus' is for MMIO devices, so is 'simple-pm-bus' with the
> > addition of PM capabilities. That means you have registers defined
> > (reg), but you don't.
> >
> > Either you have a block with mixed functions or you have separate
> > blocks. If the register space is all mixed together, then it is the
> > former and an MFD. Don't be designing your binding based on Linux
> > behavior.
>
> Thanks for clarifying how to differetiate MFD and 'simple-bus'/'simple-
> pm-bus'.  I would say the register space of the CSR module is mixed
> together, e.g., LVDS PHY child device has a register offset 0x00,
> PXL2DPI child device has a register offset 0x40 and LDB child device
> has register offsets 0x20 and 0xe0 in i.MX8qxp MIPI DSI/LVDS combo
> subsystem CSR module register space.  So, it appears to be a MFD.
>
> Lee, what do you think?  If it is indeed an MFD, a new MFD driver for
> the CSR module is needed then.

There already exists a driver which does what you need, so why create
a 2nd one? Just add the "fsl,imx8qxp-mipi-lvds-csr" compatible to the
simple-pm-bus match table and don't set ONLY_BUS. Isn't that enough?

Rob