| Message ID | 20220804061133.4110734-2-victor.liu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drivers: bus: Add Freescale i.MX8qxp pixel link MSI bus support | expand |
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. > > 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 >
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 > >
On Thu, 2022-08-04 at 11:26 -0700, Saravana Kannan wrote: > 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. First of all, this driver doesn't populate simple-mfd bus's child devices because "ONLY_BUS" is set in simple_pm_bus_of_match[] for simple-mfd. The device tree I'm working with is something like this: bus@560000000 { compatible = "fsl,aips-bus", "simple-bus"; ... bus@562000000 { compatible = "fsl,imx8qm-display-pixel-link-msi-bus", "simple- pm-bus"; ... syscon@56241000 { compatible = "fsl,imx8qm-lvds-csr", "syscon", "simple-mfd"; ... syscon_child {}; }; /* more regular mmap devices */ }; }; IIUC, default buses listed in of_default_bus_match_table[], including simple-bus and simple-mfd, are populated by of_platform_default_populate() in a recursive fashion, when of_platform_default_populate_init() is called. However, simple-pm-bus is not listed in that table. So, bus@562000000 (simple-pm-bus) is the last one to be populated successfully and syscon@56241000 (simple-mfd) is not populated (recursion stops). Then, this patch adds a match table to populate syscon@56241000 (simple -mfd) _and_ it's child nodes when bus@562000000 (simple-pm-bus) is probed. of_platform_populate() will populate syscon@56241000 (simple- mfd) and it's child devices (sycon_child) together. Hence, sycon_child devices will be probed ok. The problem is that syscon@56241000 (simple-mfd) fails to be probed with return code -ENODEV as "ONLY_BUS" is set and "simple-mfd" is the 3rd compatible string. Even if it's probed ok, syscon@56241000 (simple- mfd) is not power managed, which means syscon_child devices' PM operations won't be propagated to bus@562000000 (simple-pm-bus) (?). Anyway, somehow, syscon_child devices do work, based on my test. With regard to PM, simple-bus is the same if it sits at simple-mfd's place. So, maybe, simple-mfd and simple-bus should be power managed as well? Or, simple-pm-bus should have no simple-mfd and simple-bus child nodes at all? > > 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? Without this patch, deleting "ONLY_BUS" works for me, as syscon_child devices are populated when syscon@56241000 (simple-mfd) is probed. Deleting "ONLY_BUS" may make simple-mfd a power managed device. Is it a right thing to do? Regards, Liu Ying > > 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 > > >
On Fri, Aug 5, 2022 at 3:07 AM Liu Ying <victor.liu@nxp.com> wrote: > > On Thu, 2022-08-04 at 11:26 -0700, Saravana Kannan wrote: > > 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. > > First of all, this driver doesn't populate simple-mfd bus's child > devices because "ONLY_BUS" is set in simple_pm_bus_of_match[] for > simple-mfd. > > The device tree I'm working with is something like this: > > bus@560000000 { > compatible = "fsl,aips-bus", "simple-bus"; > ... > > bus@562000000 { > compatible = "fsl,imx8qm-display-pixel-link-msi-bus", "simple- > pm-bus"; > ... > > syscon@56241000 { > compatible = "fsl,imx8qm-lvds-csr", "syscon", "simple-mfd"; > ... > > syscon_child {}; > }; > > /* more regular mmap devices */ > }; > }; > > IIUC, default buses listed in of_default_bus_match_table[], including > simple-bus and simple-mfd, are populated by > of_platform_default_populate() in a recursive fashion, when > of_platform_default_populate_init() is called. However, simple-pm-bus > is not listed in that table. So, bus@562000000 (simple-pm-bus) is the > last one to be populated successfully and syscon@56241000 (simple-mfd) > is not populated (recursion stops). Ok, it's working as intended so far. > Then, this patch adds a match table to populate syscon@56241000 (simple > -mfd) _and_ it's child nodes when bus@562000000 (simple-pm-bus) is > probed. of_platform_populate() will populate syscon@56241000 (simple- > mfd) and it's child devices (sycon_child) together. Hence, sycon_child > devices will be probed ok. I think of_platform_default_populate() is the right solution here instead of spinning up a new table. Because a tree of simple-bus children of simple-pm-bus would have the same problem you are facing with simple-mfd's children not being populated. > The problem is that syscon@56241000 (simple-mfd) fails to be probed > with return code -ENODEV as "ONLY_BUS" is set and "simple-mfd" is the > 3rd compatible string. Ah, thanks for the example of your DT. My bad, I had forgotten the "simple-mfd" is one of the default populate busses and can be a 2nd or later entry in the compatible string and still have its children populated by default by OF platform code. > Even if it's probed ok, syscon@56241000 (simple- > mfd) is not power managed, which means syscon_child devices' PM > operations won't be propagated to bus@562000000 (simple-pm-bus) (?). > Anyway, somehow, syscon_child devices do work, based on my test. Aren't you seeing this propagation issue even with your current patches? > With regard to PM, simple-bus is the same if it sits at simple-mfd's > place. So, maybe, simple-mfd and simple-bus should be power managed as > well? Or, simple-pm-bus should have no simple-mfd and simple-bus child > nodes at all? The problem is that there are cases of devices with real drivers that also list simple-bus as their secondary compatible string. So we can't really remove any of the existing ONLY_BUS as that could cause simple-pm-bus driver to probe them instead of the real driver. In your case, why even list this as "fsl,imx8qm-lvds-csr"? Can't you just change your compatible string from: "fsl,imx8qm-lvds-csr", "syscon", "simple-mfd"; To: "simple-pm-bus", "syscon", "simple-mfd"; You are treating it exactly as a simple-pm-bus. So I don't see what this extra "fsl,imx8qm-lvds-csr" distinction brings. Or make it if you really want the "fsl,imx8qm-lvds-csr" in there: "fsl,imx8qm-lvds-csr", "simple-pm-bus", "syscon", "simple-mfd"; If you are actually going to write a driver for "fsl,imx8qm-lvds-csr" then you need to have that driver bind to this device of yours and do PM management and populate the child devices if they aren't already. Long story short, with what I understand so far, I think what you need to do are: 1. Patch to manage clock. 2. Patch to use of_platform_default_populate() 3. Fix up the compatible string to list simple-pm-bus in it. > > > > > 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? > > Without this patch, deleting "ONLY_BUS" works for me, as syscon_child > devices are populated when syscon@56241000 (simple-mfd) is probed. > Deleting "ONLY_BUS" may make simple-mfd a power managed device. Is it a > right thing to do? Ignore my point about deleting ONLY_BUS. That's wrong because then the simple-pm-bus driver can end up probing any device that lists simple-mfd even if there's another driver that could (like "fsl,imx8qm-lvds-csr") and we don't want that. -Saravana > > Regards, > Liu Ying > > > > > 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 > > > > >
On Fri, 2022-08-05 at 10:55 -0700, Saravana Kannan wrote: > On Fri, Aug 5, 2022 at 3:07 AM Liu Ying <victor.liu@nxp.com> wrote: > > > > On Thu, 2022-08-04 at 11:26 -0700, Saravana Kannan wrote: > > > 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. > > > > First of all, this driver doesn't populate simple-mfd bus's child > > devices because "ONLY_BUS" is set in simple_pm_bus_of_match[] for > > simple-mfd. > > > > The device tree I'm working with is something like this: > > > > bus@560000000 { > > compatible = "fsl,aips-bus", "simple-bus"; > > ... > > > > bus@562000000 { > > compatible = "fsl,imx8qm-display-pixel-link-msi- > > bus", "simple- > > pm-bus"; > > ... > > > > syscon@56241000 { > > compatible = "fsl,imx8qm-lvds-csr", > > "syscon", "simple-mfd"; > > ... > > > > syscon_child {}; > > }; > > > > /* more regular mmap devices */ > > }; > > }; > > > > IIUC, default buses listed in of_default_bus_match_table[], > > including > > simple-bus and simple-mfd, are populated by > > of_platform_default_populate() in a recursive fashion, when > > of_platform_default_populate_init() is called. However, simple-pm- > > bus > > is not listed in that table. So, bus@562000000 (simple-pm-bus) is > > the > > last one to be populated successfully and syscon@56241000 (simple- > > mfd) > > is not populated (recursion stops). > > Ok, it's working as intended so far. > > > Then, this patch adds a match table to populate syscon@56241000 > > (simple > > -mfd) _and_ it's child nodes when bus@562000000 (simple-pm-bus) is > > probed. of_platform_populate() will populate syscon@56241000 > > (simple- > > mfd) and it's child devices (sycon_child) together. Hence, > > sycon_child > > devices will be probed ok. > > I think of_platform_default_populate() is the right solution here > instead of spinning up a new table. Because a tree of simple-bus > children of simple-pm-bus would have the same problem you are facing > with simple-mfd's children not being populated. It seems that of_platform_default_populate() is _not_ the right solution, because simple-bus/simple-mfd devices probed by this driver, as child nodes of simple-pm-bus, are not power managed buses, which means simple-bus/simple-mfd's child devices PM operations cannot be propagated to simple-pm-bus. I'm assuming that simple-bus/simple-mfd devices probed by this driver should not be child nodes of simple-pm- bus at all. > > > The problem is that syscon@56241000 (simple-mfd) fails to be probed > > with return code -ENODEV as "ONLY_BUS" is set and "simple-mfd" is > > the > > 3rd compatible string. > > Ah, thanks for the example of your DT. My bad, I had forgotten the > "simple-mfd" is one of the default populate busses and can be a 2nd > or > later entry in the compatible string and still have its children > populated by default by OF platform code. > > > Even if it's probed ok, syscon@56241000 (simple- > > mfd) is not power managed, which means syscon_child devices' PM > > operations won't be propagated to bus@562000000 (simple-pm-bus) > > (?). > > Anyway, somehow, syscon_child devices do work, based on my test. > > Aren't you seeing this propagation issue even with your current > patches? I realized this propagation issue with this patch of mine when I looked at Rob's comment - to use of_platform_default_populate() to cover simple-bus. This propagation issue is the reason why I'm assuming simple-bus/simple-mfd devices probed by this driver should not be child nodes of simple-pm-bus at all. > > > With regard to PM, simple-bus is the same if it sits at simple- > > mfd's > > place. So, maybe, simple-mfd and simple-bus should be power > > managed as > > well? Or, simple-pm-bus should have no simple-mfd and simple-bus > > child > > nodes at all? > > The problem is that there are cases of devices with real drivers that > also list simple-bus as their secondary compatible string. So we > can't > really remove any of the existing ONLY_BUS as that could cause > simple-pm-bus driver to probe them instead of the real driver. I don't attempt to remove any of the existing ONLY_BUS. > > In your case, why even list this as "fsl,imx8qm-lvds-csr"? Can't you > just change your compatible string from: > "fsl,imx8qm-lvds-csr", "syscon", "simple-mfd"; > To: > "simple-pm-bus", "syscon", "simple-mfd"; fsl,imx8qxp-csr.yaml[1](already upstreamed) mentions "fsl,imx8qm-lvds- csr" and "fsl,imx8qxp-mipi-lvds-csr" compatible string entries. It follows the SoC name(e.g., imx8qm) + subsytem name(e.g., lvds) + IP name(csr) fashion, which exactly tells what the IP is. Although no real device tree is using the IP yet, I tend not to change the compatible string (DT maintainers usually don't like the change I'm assuming). [1] Documentation/devicetree/bindings/mfd/fsl,imx8qxp-csr.yaml > > You are treating it exactly as a simple-pm-bus. So I don't see what > this extra "fsl,imx8qm-lvds-csr" distinction brings. Or make it if > you > really want the "fsl,imx8qm-lvds-csr" in there: > "fsl,imx8qm-lvds-csr", "simple-pm-bus", "syscon", "simple-mfd"; If you take a look at the examples in fsl,imx8qxp-csr.yaml[1], you'll find that a "ipg" clock is syscon@56221000's property. Drivers[2][3] for child nodes pxl2dpi/ldb call syscon_node_to_regmap() to get regmaps from the syscon. The problem is that syscon_node_to_regmap() attaches the "ipg" clock to the regmaps by calling device_node_get_regmap() with "check_clk = true". Then, the clock will be managed by both the regmap driver and the simple-pm-bus driver, thus unreasonable clock enable count. I know drivers[2][3] may call device_node_to_regmap() instead with "check_clk = false", but the drivers will be changed and they don't really know which funtion should be called. [2] drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c [3] drivers/gpu/drm/bridge/imx/imx-ldb-helper.c And, like I mentioned, "fsl,imx8qm-lvds-csr" tells the exact IP ... > > If you are actually going to write a driver for "fsl,imx8qm-lvds-csr" > then you need to have that driver bind to this device of yours and do > PM management and populate the child devices if they aren't already. ... and can be used by a dedicated driver - yes, I'm going to write a driver for "fsl,imx8qm-lvds-csr". The driver probe function just essentially calls pm_runtime_enable() and devm_of_platform_populate(), that's it. Leave the "ipg" clock managed by the regmap driver. Make sense? > > Long story short, with what I understand so far, I think what you > need > to do are: > 1. Patch to manage clock. I'll do this, just like patch 2 does. > 2. Patch to use of_platform_default_populate() Nope for this one. Based on my understanding, I won't use of_platform_default_populate() in this driver to populate child nodes of simple-bus/simple-mfd devices . > 3. Fix up the compatible string to list simple-pm-bus in it. Nope, I'm afraid I'm not willing to change the compatible string. So, it looks like what I need to do are: 1. Drop this patch (patch 1). 2. Patch to manage clocks (patch 2). 3. Add a dedicated driver for "fsl,imx8qm-lvds-csr"/"fsl,imx8qxp-mipi- lvds-csr". Regards, Liu Ying > > > > > > > > > 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? > > > > Without this patch, deleting "ONLY_BUS" works for me, as > > syscon_child > > devices are populated when syscon@56241000 (simple-mfd) is probed. > > Deleting "ONLY_BUS" may make simple-mfd a power managed device. Is > > it a > > right thing to do? > > Ignore my point about deleting ONLY_BUS. That's wrong because then > the > simple-pm-bus driver can end up probing any device that lists > simple-mfd even if there's another driver that could (like > "fsl,imx8qm-lvds-csr") and we don't want that. > > -Saravana > > > > > > > Regards, > > Liu Ying > > > > > > > > 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 > > > > >
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; }
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. 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(-)