linux-kernel - Re: [PATCH V1 1/1] usb/host: Let usb phy shutdown later

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Apr 2022 14:59:01 +0300
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     surong pang <surong.pang@...il.com>
Cc:     mathias.nyman@...el.com, Greg KH <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Orson.Zhai@...soc.com, yunguo.wu@...soc.com
Subject: Re: [PATCH V1 1/1] usb/host: Let usb phy shutdown later

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