Regression: PM: domains: Delete usage of driver_deferred_probe_check_state

Peng Fan peng.fan at nxp.com
Fri Aug 19 02:00:14 PDT 2022


> driver_deferred_probe_check_state
> 
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fall%2F20220727185012.3255200-1-
> saravanak%40google.com%2F
> >
> &data=05%7C01%7Cpeng.fan%40nxp.com%7C4bab1e117c1e46b8cba9
> 08da8144b
> >
> e22%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6379644294901
> 88104%7C
> >
> Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> TiI6Ik1h
> >
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ZHriJc2t88Dyjt79
> DomOMvDN5
> > 2JJAcgJUK6wfOVtS6I%3D&reserved=0
> >
> > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re.kernel.org%2Fr%2F20220601070707.3946847-2-
> &data=05%7C01%7Cpen
> > >
> g.fan%40nxp.com%7C4bab1e117c1e46b8cba908da8144be22%7C686ea1d3b
> c2b4c6
> > >
> fa92cd99c5c301635%7C0%7C0%7C637964429490188104%7CUnknown%7CT
> WFpbGZsb
> > >
> 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3
> > >
> D%7C3000%7C%7C%7C&sdata=k98c9AaiMDTpYkx1353btm%2BB2DA4y
> Q8PPjV0zI
> > > RPMsE%3D&reserved=0
> > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re.kernel.org%2Fr%2F20220601070707.3946847-10-
> &data=05%7C01%7Cpe
> > >
> ng.fan%40nxp.com%7C4bab1e117c1e46b8cba908da8144be22%7C686ea1d3
> bc2b4c
> > >
> 6fa92cd99c5c301635%7C0%7C0%7C637964429490188104%7CUnknown%7C
> TWFpbGZs
> > >
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
> n0%
> > >
> 3D%7C3000%7C%7C%7C&sdata=pG64f9hJO9pAhCQ4v8txcD5LA3VXQu
> 8IsVYr4Sb
> > > 5Id8%3D&reserved=0
> > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Flkml%2F20220810060040.321697-1-
> saravanak%40google.com%2F&data=05%7C01%7Cpeng.fan%40nxp.co
> m%7C4bab1e117c1e46b8cba908da8144be22%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C637964429490344320%7CUnknown%7CTWFpbG
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%3D%7C3000%7C%7C%7C&sdata=ZIJqjOJb80tBKhEvCa8ZilQQY0OF
> LZBuon6H5A7%2FeRU%3D&reserved=0
> 
> 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);

The issue is 
really_probe->device_links_check_suppliers
Because we use scmi protocol node as supplier, the supplier
is changed to firmware:scmi. So the supplier check pass.

However the protocol device is created later, before the protocol device
ready, the mmc device is probed and dev_pm_domain_attach
return error.

BTW, your patchset  plus your new diff still not work.

[    1.583526] arm-scmi firmware:scmi: Enabled polling mode TX channel - prot_id:16
[    1.590775] device: 'firmware:scmi': device_add
[    1.595256] arm-scmi firmware:scmi: SCMI Notifications - Core Enabled.
[    1.601805] arm-scmi firmware:scmi: Malformed reply - real_sz:8  calc_sz:4  (loop_num_ret:2)
[    1.610098] arm-scmi firmware:scmi: SCMI Protocol v2.0 'NXP:' Firmware version 0x0
[    1.617731] mmc at 298d0000 Linked as a fwnode consumer to scmi
[    1.623279] mmc at 298d0000 Dropping the fwnode link to protocol at 11
[    1.629270] fw_devlink_create_devlink 298d0000.mmc firmware:scmi
[    1.635257] device: 'platform:firmware:scmi--platform:298d0000.mmc': device_add
[    1.642590] devices_kset: Moving 298d0000.mmc to end of list
[    1.648166] platform 298d0000.mmc: Linked as a consumer to firmware:scmi
[    1.654840] mmc at 298d0000 Dropping the fwnode link to scmi

Per log, mmc not take the protocol at 11 as supplier in the end.

Regards,
Peng.



More information about the linux-arm-kernel mailing list