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
| ||
|
Date: Fri, 05 Aug 2022 18:06:46 +0800 From: Liu Ying <victor.liu@....com> To: Saravana Kannan <saravanak@...gle.com>, Rob Herring <robh+dt@...nel.org> Cc: devicetree@...r.kernel.org, linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Sascha Hauer <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, NXP Linux Team <linux-imx@....com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Geert Uytterhoeven <geert+renesas@...der.be>, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> Subject: Re: [PATCH v3 1/3] drivers: bus: simple-pm-bus: Populate simple MFD child devices 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@...nel.org> > wrote: > > > > On Thu, Aug 4, 2022 at 12:10 AM Liu Ying <victor.liu@....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@...000000 { compatible = "fsl,aips-bus", "simple-bus"; ... bus@...000000 { compatible = "fsl,imx8qm-display-pixel-link-msi-bus", "simple- pm-bus"; ... syscon@...41000 { 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@...000000 (simple-pm-bus) is the last one to be populated successfully and syscon@...41000 (simple-mfd) is not populated (recursion stops). Then, this patch adds a match table to populate syscon@...41000 (simple -mfd) _and_ it's child nodes when bus@...000000 (simple-pm-bus) is probed. of_platform_populate() will populate syscon@...41000 (simple- mfd) and it's child devices (sycon_child) together. Hence, sycon_child devices will be probed ok. The problem is that syscon@...41000 (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@...41000 (simple- mfd) is not power managed, which means syscon_child devices' PM operations won't be propagated to bus@...000000 (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@...41000 (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@....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 > > >
Powered by blists - more mailing lists