Regression: PM: domains: Delete usage of driver_deferred_probe_check_state

[Saravana Kannan]

On Mon, Aug 15, 2022 at 11:43 PM Peng Fan <peng.fan at nxp.com> wrote:
>
> > Subject: Regression: PM: domains: Delete usage of
> > driver_deferred_probe_check_state
>
> Just see your patchset :)
> https://lore.kernel.org/all/20220727185012.3255200-1-saravanak@google.com/
>
> Thanks,
> Peng.
> >
> > Hi Saravana,
> >
> > The following two patches breaks NXP i.MX8ULP, but I think it may break
> > others use SCMI.
> >
> > commit 5a46079a96451cfb15e4f5f01f73f7ba24ef851a
> > Author: Saravana Kannan <mailto:saravanak at google.com>
> > Date:   Wed Jun 1 00:06:57 2022 -0700
> >
> >     PM: domains: Delete usage of driver_deferred_probe_check_state()
> >
> >     Now that fw_devlink=on by default and fw_devlink supports
> >     "power-domains" property, the execution will never get to the point
> >     where driver_deferred_probe_check_state() is called before the supplier
> >     has probed successfully or before deferred probe timeout has expired.
> >
> >     So, delete the call and replace it with -ENODEV.
> >
> >     Tested-by: Geert Uytterhoeven <mailto:geert+renesas at glider.be>
> >     Reviewed-by: Ulf Hansson <mailto:ulf.hansson at linaro.org>
> >     Signed-off-by: Saravana Kannan <mailto:saravanak at google.com>
> >     Link: https://lore.kernel.org/r/20220601070707.3946847-2-
> > saravanak at google.com
> >     Signed-off-by: Greg Kroah-Hartman <mailto:gregkh at linuxfoundation.org>
> >
> > commit 9cbffc7a59561be950ecc675d19a3d2b45202b2b
> > Author: Saravana Kannan <mailto:saravanak at google.com>
> > Date:   Wed Jun 1 00:07:05 2022 -0700
> >
> >     driver core: Delete driver_deferred_probe_check_state()
> >
> >     The function is no longer used. So delete it.
> >
> >     Tested-by: Geert Uytterhoeven <mailto:geert+renesas at glider.be>
> >     Signed-off-by: Saravana Kannan <mailto:saravanak at google.com>
> >     Link: https://lore.kernel.org/r/20220601070707.3946847-10-
> > saravanak at google.com
> >     Signed-off-by: Greg Kroah-Hartman <mailto:gregkh at linuxfoundation.org>
> >
> > The i.MX8ULP mmc device node use
> > "power-domains = <&scmi_devpd IMX8ULP_PD_USDHC0>;"
> >
> > The scmi firmware node as below:
> >         firmware {
> >                 scmi {
> >                         compatible = "arm,scmi-smc";
> >                         arm,smc-id = <0xc20000fe>;
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         shmem = <&scmi_buf>;
> >
> >                         scmi_devpd: protocol at 11 {
> >                                 reg = <0x11>;
> >                                 #power-domain-cells = <1>;
> >                         };
> >
> >                         scmi_sensor: protocol at 15 {
> >                                 reg = <0x15>;
> >                                 #thermal-sensor-cells = <1>;
> >                         };
> >                 };
> >         };
> >
> > When sdhc driver probe, the scmi power domain provider has not been
> > registered. So __genpd_dev_pm_attach directly return -ENODEV.
> >
> > device_links_check_suppliers should already check suppliers, but scmi
> > protocol device not have compatible, so of_link_to_phandle
> >       |-> of_get_compat_node
> > use the parent node of scmi protocol as supplier if I understand correct.
> >
> > I am not sure whether we need to revert the above two patches, or do you
> > have other suggestions?

Hi Peng,

Thanks for the report. If you try this series with the following diff,
I expect it to fix the issue for you. Can you please test it out and
let me know? The v1 of the series removes the dependency on
"compatible" strings. The first diff below is something I'm going to
roll into v2 of the series and the 2nd diff below is fixing up the
scmi bus to set the fwnode for devices it creates.

Thanks,
Saravana

https://lore.kernel.org/lkml/20220810060040.321697-1-saravanak@google.com/

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2f012e826986..866755d8ad95 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2068,7 +2068,11 @@ static int fw_devlink_create_devlink(struct device *con,
                device_links_write_unlock();
        }

-       sup_dev = get_dev_from_fwnode(sup_handle);
+       if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE)
+               sup_dev = fwnode_get_next_parent_dev(sup_handle);
+       else
+               sup_dev = get_dev_from_fwnode(sup_handle);
+
        if (sup_dev) {
                /*
                 * If it's one of those drivers that don't actually bind to


diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index d4e23101448a..0802bdd0ebfc 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -192,6 +192,7 @@ scmi_device_create(struct device_node *np, struct
device *parent, int protocol,
        scmi_dev->protocol_id = protocol;
        scmi_dev->dev.parent = parent;
        scmi_dev->dev.of_node = np;
+       scmi_dev->dev.fwnode= of_fwnode_handle(np);
        scmi_dev->dev.bus = &scmi_bus_type;
        scmi_dev->dev.release = scmi_device_release;
        dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id);