Re: [PATCH v3 1/3] drivers: bus: simple-pm-bus: Populate simple MFD child devices

From: Saravana Kannan <saravanak@google.com>
To: Rob Herring <robh+dt@kernel.org>
Cc: Liu Ying <victor.liu@nxp.com>,
	devicetree@vger.kernel.org,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Sascha Hauer <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Subject: Re: [PATCH v3 1/3] drivers: bus: simple-pm-bus: Populate simple MFD child devices
Date: Thu, 4 Aug 2022 11:26:58 -0700	[thread overview]
Message-ID: <CAGETcx94De-wofRjtPgNxa+YQoU3+j+we+4K9Evm=vtzhopX8g@mail.gmail.com> (raw)
In-Reply-To: <CAL_Jsq+B5PMOmZO4hz5DyEsA4V=UkrNn-6b58h8VbcPa2iaQ1g@mail.gmail.com>

On Thu, Aug 4, 2022 at 5:18 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Aug 4, 2022 at 12:10 AM Liu Ying <victor.liu@nxp.com> wrote:
> >
> > There could be simple MFD device(s) connected to a simple PM bus as child
> > node(s), like Freescale i.MX8qxp pixel link MSI bus. Add a child match
> > table as an argument to of_platform_populate() function call to specify
> > the simple MFD devices so that they can be populated.
>
> There could be a simple-bus under it as well. You should just use
> of_platform_default_populate() instead.

I'm confused why we even need this patch. Wouldn't this driver
automatically probe simple-mfd buses and populate its child devices?
We already have it in simple_pm_bus_of_match.

I'm wondering if you are trying to workaround the behavior of having
"ONLY_BUS" set in simple_pm_bus_of_match for "simple-mfd". Have you
tried deleting that field and see if it does what you want?

And we wouldn't need to use of_platform_default_populate() because
this driver would take care of doing that recursively. Especially when
you need the clocks and power domain to be able to access the child
devices, you want the driver to probe and do that at each level before
automatically recursively adding all the grand-children devices.

-Saravana

>
> >
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> > v1->v3:
> > * No change.
> >
> >  drivers/bus/simple-pm-bus.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
> > index 6b8d6257ed8a..ff5f8ca5c024 100644
> > --- a/drivers/bus/simple-pm-bus.c
> > +++ b/drivers/bus/simple-pm-bus.c
> > @@ -13,6 +13,11 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> >
> > +static const struct of_device_id simple_pm_bus_child_matches[] = {
> > +       { .compatible = "simple-mfd", },
> > +       {}
> > +};
> > +
> >  static int simple_pm_bus_probe(struct platform_device *pdev)
> >  {
> >         const struct device *dev = &pdev->dev;
> > @@ -49,7 +54,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
> >         pm_runtime_enable(&pdev->dev);
> >
> >         if (np)
> > -               of_platform_populate(np, NULL, lookup, &pdev->dev);
> > +               of_platform_populate(np, simple_pm_bus_child_matches, lookup, &pdev->dev);
> >
> >         return 0;
> >  }
> > --
> > 2.25.1
> >