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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 6 Nov 2023 10:53:10 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Stanley Chang[昌育德] 
        <stanley_chang@...ltek.com>
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

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).

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