phy: Fix db410c crash issue during usb_stop

JianfengA.Zhu at sony.com JianfengA.Zhu at sony.com
Mon Oct 14 08:24:41 CEST 2024


Is there any update on this issue?
I found that this issue still exists on the latest v2024.10 u-boot.

-----Original Message-----
From: Zhu, Jianfeng 
Sent: Friday, August 23, 2024 1:00 PM
To: caleb.connolly at linaro.org; neil.armstrong at linaro.org; marex at denx.de; trini at konsulko.com
Cc: u-boot-qcom at groups.io; Cao, Jacky <Jacky.Cao at sony.com>; u-boot at lists.denx.de
Subject: phy: Fix db410c crash issue during usb_stop

usb stop crash log
====================================================================
dragonboard410c => usb start
starting USB...
Bus usb at 78d9000: USB EHCI 1.00
scanning bus usb at 78d9000 for devices... 4 USB Device(s) found
       scanning usb for storage devices... 0 Storage Device(s) found dragonboard410c => usb stop stopping USB..
"Synchronous Abort" handler, esr 0x96000007, far 0x0
elr: 000000008f636a28 lr : 000000008f636a24 (reloc)
elr: 00000000bd97ea28 lr : 00000000bd97ea24
x0 : 0000000000000000 x1 : 00000000bd1494d0
x2 : 0000000000000000 x3 : 0000000000001f40
x4 : 00000000bd131990 x5 : 00000000bd157b70
x6 : 0000000000000041 x7 : 00000000bd163b80
x8 : 00000000bd131db0 x9 : 000000000000b65c
x10: 00000000bd13183c x11: 0000000000000006
x12: 000000000001869f x13: 0000000000000061
x14: 00000000ffffffff x15: 00000000bd1317b8
x16: 00000000bd9b78ac x17: 0000000000000000
x18: 00000000bd143d90 x19: 00000000bd1575d8
x20: 00000000bd1575d8 x21: 00000000bd157440
x22: 00000000bd1493b0 x23: 0000000000000002
x24: 00000000bd9eb910 x25: 0000000000000000
x26: 0000000000000000 x27: 0000000000000000
x28: 00000000bd158540 x29: 00000000bd131980

Code: f9400000 b4000120 97ffdd1e aa0003e2 (f9400001) Resetting CPU ...

resetting ...
====================================================================

After commit ed8fbd2889fc ("dts: msm8916: replace with upstream DTS") msm8916_usbphy will be defined as a child device of usb at 78d9000. When calling usb stop, usb at 78d9000 will be removed. During the removal process, the sub-device msm8916_usbphy will be removed first. In the subsequent remove process, the uclass_priv_ of msm8916_usbphy will be accessed again.
Because uclass_priv_ is NULL at this time, it will cause a crash.

Detailed calling process
====================================================================
usb_stop (drivers/usb/host/usb-uclass.c)
|-> device_remove(bus, DM_REMOVE_NORMAL); <== remove ehci_msm
.|-> device_chld_remove <== remove msm8916_usbphy . |-> device_remove .  |-> device_free(dev);
.   |-> dev_set_uclass_priv(dev, NULL);
.    |-> dev->uclass_priv_ = uclass_priv; <== uclass_priv is NULL
|-> drv->remove(dev);
 |-> ehci_usb_remove
  |-> generic_shutdown_phy
   |-> generic_shutdown_phy
    |-> phy_get_counts
     |-> dev_get_uclass_priv
      |-> dm_priv_to_rw(dev->uclass_priv_); <== NULL pointer access ====================================================================

Fix: move usb phy power off into msm8916_usbphy remove.

Signed-off-by: Jianfeng Zhu <JianfengA.Zhu at sony.com>
Reviewed-by: Jacky Cao <Jacky.Cao at sony.com>
Reviewed-by: Toyama, Yoshihiro <Yoshihiro.Toyama at sony.com>
---
 drivers/phy/qcom/msm8916-usbh-phy.c | 20 ++++++++++++++++++++
 drivers/usb/host/ehci-msm.c         |  4 ----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qcom/msm8916-usbh-phy.c b/drivers/phy/qcom/msm8916-usbh-phy.c
index 4b435aa2a6..0ba520c93d 100644
--- a/drivers/phy/qcom/msm8916-usbh-phy.c
+++ b/drivers/phy/qcom/msm8916-usbh-phy.c
@@ -46,6 +46,25 @@ static int msm_phy_power_off(struct phy *phy)
 	return 0;
 }
 
+static int msm_phy_remove(struct udevice *dev) {
+	struct phy phy;
+	int ret;
+
+	ret = generic_phy_get_by_index(dev->parent, 0, &phy);
+	if (ret)
+		return ret;
+
+	/* To avoid NULL pointer access exception issue, move
+	 * generic_shutdown_phy from ehci_usb_remove to here
+	 */
+	ret = generic_shutdown_phy(&phy);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int msm_phy_reset(struct phy *phy)  {
 	struct msm_phy_priv *p = dev_get_priv(phy->dev); @@ -105,5 +124,6 @@ U_BOOT_DRIVER(msm8916_usbphy) = {
 	.of_match	= msm_phy_ids,
 	.ops		= &msm_phy_ops,
 	.probe		= msm_phy_probe,
+	.remove		= msm_phy_remove,
 	.priv_auto	= sizeof(struct msm_phy_priv),
 };
diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c index ff336082e3..565859e806 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -114,10 +114,6 @@ static int ehci_usb_remove(struct udevice *dev)
 	clk_disable_unprepare(&p->iface_clk);
 	clk_disable_unprepare(&p->core_clk);
 
-	ret = generic_shutdown_phy(&p->phy);
-	if (ret)
-		return ret;
-
 	ret = board_usb_init(0, USB_INIT_DEVICE); /* Board specific hook */
 	if (ret < 0)
 		return ret;
--
2.25.1


More information about the U-Boot mailing list