Regression: PM: domains: Delete usage of driver_deferred_probe_check_state
Bough Chen
haibo.chen at nxp.com
Thu Aug 18 21:04:23 PDT 2022
> -----Original Message-----
> From: Saravana Kannan <saravanak at google.com>
> Sent: 2022年8月19日 2:08
> To: Peng Fan <peng.fan at nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; dl-linux-imx <linux-imx at nxp.com>;
> sudeep.holla at arm.com; linux-pm at vger.kernel.org; Alice Guo
> <alice.guo at nxp.com>; Bough Chen <haibo.chen at nxp.com>
> Subject: Re: Regression: PM: domains: Delete usage of
> 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%7Chaibo.chen%40nxp.com%7C360d3e523a9441882e59
> 08da814
> >
> 4be16%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637964429489
> 059385%
> >
> 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> I6Ik
> >
> 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ks6LI%2FRywgIK
> HrMJmXtzG
> > mdz%2B2M1j5M7BvfsNiXOxVc%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%7Ch
> ai
> > >
> bo.chen%40nxp.com%7C360d3e523a9441882e5908da8144be16%7C686ea1d3
> bc2b4
> > >
> c6fa92cd99c5c301635%7C0%7C0%7C637964429489059385%7CUnknown%7CT
> WFpbGZ
> > >
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> > > %3D%7C3000%7C%7C%7C&sdata=iEFIifTVfvCnrrrJ8F19nIacGdF8VQ6Y
> wnN9BJ
> > > edaPk%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%7C
> ha
> > >
> ibo.chen%40nxp.com%7C360d3e523a9441882e5908da8144be16%7C686ea1d3
> bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C637964429489059385%7CUnknown%7C
> TWFpbG
> > >
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> > >
> 0%3D%7C3000%7C%7C%7C&sdata=4odCYW1lE17PZLBlvYFM4%2B0b0pm
> chu3vs8W
> > > iSfEuHI8%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.
HI Saravana,
I notice that the three reverted patch do not in upstream 6.0-RC1, so on the latest 6.0-RC1,
I apply your 9 patch set and the two diff, but unfortunately still meet the same issue. MMC still not probe.
You can refer to arch/arm64/boot/dts/freescale/imx8ulp.dtsi for more detail.
And for your second diff, will meet following build error, I just add a little change to fix it:
1, add onel line : #include <linux/of.h>
2, add a blank before "=" in line 195.
drivers/firmware/arm_scmi/bus.c: In function ‘scmi_device_create’:
drivers/firmware/arm_scmi/bus.c:195:31: error: implicit declaration of function ‘of_fwnode_handle’ [-Werror=implicit-function-declaration]
195 | scmi_dev->dev.fwnode= of_fwnode_handle(np);
| ^~~~~~~~~~~~~~~~
drivers/firmware/arm_scmi/bus.c:195:29: warning: assignment to ‘struct fwnode_handle *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
195 | scmi_dev->dev.fwnode= of_fwnode_handle(np);
| ^
cc1: some warnings being treated as errors
make[3]: *** [scripts/Makefile.build:250: drivers/firmware/arm_scmi/bus.o] Error 1
make[3]: *** Waiting for unfinished jobs....
Best Regards
Haibo Chen
>
> Thanks,
> Saravana
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> el.org%2Flkml%2F20220810060040.321697-1-saravanak%40google.com%2F&a
> mp;data=05%7C01%7Chaibo.chen%40nxp.com%7C360d3e523a9441882e5908
> da8144be16%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6379644
> 29489059385%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo
> iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdat
> a=o7%2B5HStu82S3dj1J5SfEO3PrG9fo2SSsR6PT5t0v0x0%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);
More information about the linux-arm-kernel
mailing list