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

["Stanley Chang[昌育德]" <stanley_chang@realtek.com>]

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@realtek.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