On Wed, 16 Nov 2022, Liu Ying wrote:

On Tue, 2022-11-15 at 07:33 -0600, Rob Herring wrote:

quoted

On Mon, Nov 14, 2022 at 11:22 PM Liu Ying [off-list ref] wrote:

quoted

On Mon, 2022-11-14 at 14:54 -0600, Rob Herring wrote:

quoted

On Mon, Nov 7, 2022 at 9:58 PM Liu Ying [off-list ref] wrote:

quoted

Hi Lee,

On Mon, 2022-11-07 at 09:05 +0000, Lee Jones wrote:

quoted

On Wed, 02 Nov 2022, Liu Ying wrote:

quoted

Hi Lee,

On Tue, 2022-11-01 at 13:53 +0800, Liu Ying wrote:

quoted

Hi Lee,

On Mon, 2022-10-31 at 15:40 +0000, Lee Jones wrote:

quoted

On Mon, 17 Oct 2022, Liu Ying wrote:

[...]

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+     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.

quoted

  /*
   * 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.

quoted

quoted

quoted

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?

Adding "fsl,imx8qxp-mipi-lvds-csr" and "fsl,imx8qm-lvds-csr" compatible
strings to the simple-pm-bus match table does work, but it tends to
make people think the CSR module is a bus instead of a MFD.

MFDs don't exist.  They're a figment of our imagination.

If you want PM enabled and wish for your child devices to be
auto-magically registered for you, simple-pm-bus is what you need.

It's kinka
straightforward if a MFD device driver lives in drivers/mfd directory.
However, since "simple-mfd" compatible is also in that match table, I
would add the CSR module's compatible strings to that match table if no
objections.

Fine by me if it saves authoring a pointless driver.

-- 
Lee Jones [李琼斯]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel