Thread (18 messages) 18 messages, 4 authors, 2021-09-24

Re: [PATCH v3 1/2] drivers: bus: simple-pm-bus: Add support for probing simple bus only devices

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-09-06 07:54:04
Also in: linux-omap, linux-renesas-soc, linux-riscv, lkml

Hi Saravana,

Thanks for your patch!

CC linux-pm, Lee (mfd)

On Sat, Sep 4, 2021 at 2:05 AM Saravana Kannan [off-list ref] wrote:
fw_devlink could end up creating device links for bus only devices.
However, bus only devices don't get probed and can block probe() or
sync_state() [1] call backs of other devices. To avoid this, probe these
devices using the simple-pm-bus driver.

However, there are instances of devices that are not simple buses (they
get probed by their specific drivers) that also list the "simple-bus"
(or other bus only compatible strings) in their compatible property to
automatically populate their child devices. We still want these devices
to get probed by their specific drivers. So, we make sure this driver
only probes devices that are only buses.
Note that this can also be the case for buses declaring compatibility
with "simple-pm-bus".  However, at the moment, none of such device
nodes in upstream DTS files have device-specific drivers.
[1] - https://lore.kernel.org/lkml/CAPDyKFo9Bxremkb1dDrr4OcXSpE0keVze94Cm=zrkOVxHHxBmQ@mail.gmail.com/ (local)
Signed-off-by: Saravana Kannan <redacted>
Tested-by: Saravana Kannan <redacted>
quoted hunk ↗ jump to hunk
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -13,11 +13,26 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>

-
 static int simple_pm_bus_probe(struct platform_device *pdev)
 {
-       const struct of_dev_auxdata *lookup = dev_get_platdata(&pdev->dev);
-       struct device_node *np = pdev->dev.of_node;
+       const struct device *dev = &pdev->dev;
+       const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
+       struct device_node *np = dev->of_node;
+       const struct of_device_id *match;
+
+       match = of_match_device(dev->driver->of_match_table, dev);
+
+       /*
+        * 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.
+        */
+       if (match && match->data) {
+               if (of_property_match_string(np, "compatible", match->compatible) == 0)
Does this work as expected? Having multiple compatible values in a
device node does not guarantee there exist a separate driver for any
of the device-specific compatible values.
+                       return 0;
+               else
+                       return -ENODEV;
So if we get here, as both branches use "return", we skip the
pm_runtime_enable() and of_platform_populate() below:
  - of_platform_populate() is handled for these buses by
    of_platform_default_populate(), so that's OK,
  - I'm wondering if any of the simple-mfd sub-devices use Runtime
    PM, but currently fail to save power because pm_runtime_enable()
    is never called for the MFD container, just like with simple-bus...
quoted hunk ↗ jump to hunk
+       }

        dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -31,14 +46,25 @@ static int simple_pm_bus_probe(struct platform_device *pdev)

 static int simple_pm_bus_remove(struct platform_device *pdev)
 {
+       const void *data = of_device_get_match_data(&pdev->dev);
+
+       if (data)
+               return 0;
+
        dev_dbg(&pdev->dev, "%s\n", __func__);

        pm_runtime_disable(&pdev->dev);
        return 0;
 }

+#define ONLY_BUS       ((void *) 1) /* Match if the device is only a bus. */
+
 static const struct of_device_id simple_pm_bus_of_match[] = {
        { .compatible = "simple-pm-bus", },
+       { .compatible = "simple-bus",   .data = ONLY_BUS },
+       { .compatible = "simple-mfd",   .data = ONLY_BUS },
+       { .compatible = "isa",          .data = ONLY_BUS },
#ifdef CONFIG_ARM_AMBA ?
+       { .compatible = "arm,amba-bus", .data = ONLY_BUS },
        { /* sentinel */ }
This is now (almost[*]) the same as of_default_bus_match_table[]
in drivers/of/platform.c. Perhaps it can be shared?

[*] Especially if "simple-pm-bus" and "simple-bus" would be treated
    the same.
 };
 MODULE_DEVICE_TABLE(of, simple_pm_bus_of_match);
Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help