On 22.4.2022 13.43, surong pang wrote: >>>> @@ -398,6 +397,7 @@ static int xhci_plat_remove(struct platform_device *dev) >>>> clk_disable_unprepare(clk); >>>> clk_disable_unprepare(reg_clk); >>>> + usb_phy_shutdown(hcd->usb_phy); >>>> usb_put_hcd(hcd); > > Is it ok to put usb_phy_shutdown before usb_put_hcd(hcd)? hcd is > released at usb_put_hcd. yes, above looks good. > > UNISOC DWC3 phy is not divided USB 2.0/3.0 phy clearly. Yes, it's > UNISOC's issue. > It UNISOC's dtsi: phys = <&ssphy>, <&ssphy>; > If to shutdown phy too earlier, it will cost 10s timeout to do xhci_reset. > usb_remmove_hcd --> usb_stop_hcd --> xhci_stop --> xhci_reset --> > xhci_handshake(&xhci->op_regs->command, CMD_RESET, 0, 10 * 1000 *1000) > > I want to know this change is acceptable or not? > > hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0); > Why in xhci_plat_remove, just to shutdown "usb-phy"[0], not to > shutdown "usb-phy"[1] ? xhci-plat.c only takes one phy at index 0, so we only shutdowns that one. Looks like usb core hcd code has better phy handling when adding and removing hcds. It supports multiple phys. If possible use that instead. See drivers/usb/core/hcd.c usb_add_hcd() Thanks -Mathias
