Re: [PATCH 2/2] phy: rockchip: usbdp: re-init the phy on orientation-change

["Heiko Stübner" <heiko@sntech.de>]

Hey Ondřej,

Am Mittwoch, 26. Februar 2025, 15:46:11 MEZ schrieb Ondřej Jirman:
> On Tue, Feb 25, 2025 at 07:45:19PM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@cherry.de>
> > 
> > Until now the usbdp in the orientation-handler set the new lane setup in
> > its internal state variables and adapted the sbu gpios as needed.
> > It never actually updated the phy itself though, but relied on the
> > controlling usb-controller to disable and re-enable the phy.
> > 
> > And while on the vendor-kernel, I could see that on every unplug the dwc3
> > did go to its suspend and woke up on the next device plug-in event,
> > thus toggling the phy as needed, this does not happen in all cases and we
> > should not rely on that behaviour.
> 
> On RK3399 there's a similar issue with the equivalent type-c PHY driver.
> The TRM (part 2) states that:
> 
> 4.6.1 Some Special Settings before Initialization
> 
> - Set USB3.0 OTG controller AXI master setting.
> - Clear USB2.0 only mode setting (bit 3 of register GRF_USB3PHY0/1_CON0 in Chapter GRF)
> - USB3.0 OTG controller should be hold in reset during the initialization of the corresponding
>   TypeC PHY until TypeC PHY is ready for USB operation.
> - Set PHYIF to 1 to use 16-bit UTMI+ interface (see register GUSB2PHYCFG0)
> - Clear ENBLSLPM to 0 to disable sleep and l1 suspend (see register GUSB2PHYCFG0)
>   ...
> 
> The PHY for Superspeed signals is expected to be set up while the USB
> controller is held in reset, which makes sense HW wise, and it's what downstream
> kernel efectivelly does, via its RPM based hack.
> 
> RK3588 TRM doesn't have very detailed notes on this, but I expect it will be
> similar.
> 
> So reconfiguring the phy here, while it's actively linked to the USB controller
> without the controller driver driving the process so it reliably happens while
> it's in reset, or at least so that USB controller reset happens afterwards, may
> not be correct way to approach this.

the function here, is using infrastructure from the type-c framework.

The function in question tcpm_mux_set(), which then ends up in the
usbdp_phy only gets called from tcpm_reset_port() .

Which I think will do the right already - judging by its name ;-) .
[and also by the fact that the referenced qcom phy behaves similarly
 when talking to the type-c framework]


For the rk3399 I would think converting the old typec-phy-driver over to
use the actual kernel type-c framework, might just magically solve the
issue you have on it.

Rockchip actually has converted the rk3399 typec-phy to use the type-c
framework in their vendor kernel.


Heiko