linux-kernel - RE: [PATCH] USB: xhci-plat: fix legacy PHY double inity

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 6 Nov 2023 10:08:24 +0000
From:   Stanley Chang[昌育德] 
        <stanley_chang@...ltek.com>
To:     Johan Hovold <johan@...nel.org>
CC:     Stefan Eichenberger <eichest@...il.com>,
        Johan Hovold <johan+linaro@...nel.org>,
        Mathias Nyman <mathias.nyman@...el.com>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Maxime Ripard <mripard@...nel.org>
Subject: RE: [PATCH] USB: xhci-plat: fix legacy PHY double inity

Hi Johan,

> 
> On Mon, Nov 06, 2023 at 06:53:23AM +0000, Stanley Chang[昌育德] wrote:
> > > > On Fri, Nov 03, 2023 at 05:43:23PM +0100, Johan Hovold wrote:
> > > > > Commits 7b8ef22ea547 ("usb: xhci: plat: Add USB phy support")
> > > > > and
> > > > > 9134c1fd0503 ("usb: xhci: plat: Add USB 3.0 phy support") added
> > > > > support for looking up legacy PHYs from the sysdev devicetree
> > > > > node and initialising them.
> > > > >
> > > > > This broke drivers such as dwc3 which manages PHYs themself as
> > > > > the PHYs would now be initialised twice, something which
> > > > > specifically can lead to resources being left enabled during
> > > > > suspend (e.g. with the usb_phy_generic PHY driver).
> > > > >
> > > > > As the dwc3 driver uses driver-name matching for the xhci
> > > > > platform device, fix this by only looking up and initialising
> > > > > PHYs for devices that have been matched using OF.
> 
> > > Tested-by: Stanley Chang <stanley_chang@...ltek.com>
> 
> > I am sorry to notify you this patch is tested fail.
> 
> Hmm. Thanks for testing.
> 
> > I test the Realtek phy driver at drivers/phy/Realtek/phy-rtk-usb2.c again.
> > But I can't get the phy in xhci.
> 
> > It is a dwc3 generic phy driver, and it is also a usb phy driver.
> 
> That sounds broken (i.e. to be relying on both frameworks), but indeed that
> seems to be the current state of the generic and legacy USB PHY
> implementations.
> 
> What a mess.

> > Base on you modified, I can't run on callback
> > rtk_phy->phy.notify_port_status = rtk_phy_notify_port_status;
> 
> Which dwc3 driver are you using? Unless I'm missing something this would not
> be an issue unless you are doing something crazy like describing the same PHY
> twice in the devicetree (i.e. both as a generic and legacy PHY).

I use drivers/usb/dwc3/core.c and drivers/usb/dwc3/dwc3-rtk.c
I describe the PHY as generic and legacy PHY in device tree.
Our driver needs the API base on a08799cf17c2 ("usb: phy: add usb phy notify port status API").
But generic PHY driver is not support this.

Thanks,
Stanley

> Apparently, there are no in-tree users of this particular realtek PHY so I can't
> check the devicetree, but we do have other instances of such abuse since at
> least a decade:
> 
>         6747caa76cab ("usb: phy: twl4030: use the new generic PHY
> framework")
> 
> And, yes, then this is sort of expected. The dwc3 driver has always managed its
> own PHYs, but functionality has now been bolted on top so that people may
> have started relying on it being managed *also* by xhci, well at least for
> notifications like the one you just added:
> 
>         a08799cf17c2 ("usb: phy: add usb phy notify port status API")
> 
> Johan