Hi Peter, On Thu, Jan 14, 2021 at 08:54:54AM +0800, Peter Chen wrote: > On 21-01-13 12:18:35, Stephan Gerhold wrote: > > > > Also, on a completely different note I looked again at the chipidea USB > > driver that produces this situation. To request the PHY (which ends up > > in the circular device link) it does: > > > > /* Look for a generic PHY first */ > > ci->phy = devm_phy_get(dev->parent, "usb-phy"); > > > > To me it doesn't really seem great to use the devm_* helpers on the > > parent device either, so I will check if I can refactor this somehow. > > Perhaps this situation can be prevented entirely. > > > > You could try to get the PHY at parent driver > (drivers/usb/chipidea/ci_hdrc_msm.c) to see the difference. > Unfortunately, I don't think this works in my case because I have an ULPI PHY. It's not available until ret = ci_ulpi_init(ci); is called from the ci_hdrc.0 platform device. I tried the following diff yesterday. It prevents the circular device link, therefore also the crash and even devm_* on the parent device: diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index aa40e510b806..79f556d0c93e 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -847,6 +847,8 @@ struct platform_device *ci_hdrc_add_device(struct device *dev, } pdev->dev.parent = dev; + device_set_of_node_from_dev(&pdev->dev, dev); + pdev->driver_override = kstrdup("ci_hdrc", GFP_KERNEL); ret = platform_device_add_resources(pdev, res, nres); if (ret) @@ -1027,7 +1029,7 @@ static int ci_hdrc_probe(struct platform_device *pdev) ci->usb_phy = ci->platdata->usb_phy; } else { /* Look for a generic PHY first */ - ci->phy = devm_phy_get(dev->parent, "usb-phy"); + ci->phy = devm_phy_get(dev, "usb-phy"); if (PTR_ERR(ci->phy) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; Basically my idea was to share the of_node with the ci_hdrc.0 platform device, so that it can request the PHY itself instead of going through the parent. It seems to work fine but I had to add pdev->driver_override to prevent the ci_hdrc.0 device from probing using ci_hdrc_msm (since it considers the "compatible" value on the of_node otherwise). This is a bit weird (I think driver_override is mainly intended to be written through sysfs, not from kernel code directly). That is why I did not post this as a proper patch yet... Thanks, Stephan