Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1.1: Type: text/plain, Size: 9637 bytes --]

Hello Igor,

On Sat, Aug 15, 2026 at 02:16:01PM +0200, Igor Paunovic wrote:
> Hi Sebastian,
> 
> I retested v14 on my Orange Pi 5 Plus, this time covering the phy-core
> notifier patch and the new dwc3 glue patches (28/38-31/38), which do not
> carry my tag yet.
> 
> Test kernel: 7.2.0-rc7, built from your rockchip-devel branch at
> ea51774c5c42 ("usb: typec: mux: initialize mux switch array"), which
> already contains the whole v14 series (I checked that 1/38 and 28/38-32/38
> are in that history, plus all 32 "phy: rockchip: usbdp:" commits are
> ancestors of it).
> 
> On top of that base I carry 50 local commits - display, GPU, media and
> platform work. None of them touch drivers/usb/, drivers/usb/typec/ or
> phy-rockchip-usbdp.c, but two are worth naming so you know exactly what
> was under test:
> 
>   - drivers/phy/rockchip/phy-rockchip-snps-pcie3.c: a one-line fix to the
>     PCIe combo PHY SRAM-init check. Unrelated to this series.

That's a completley unrelated :)

>   - arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts: in mainline
>     this board still has the old single-"port" graph for usbdp_phy0 and no
>     altmodes node on the usb-c-connector, so there is no DP alt mode to
>     exercise at all. I carry a local DT patch that converts usbdp_phy0 to
>     the four-port "ports" graph and usb_host0_xhci to the two-port graph
>     your bindings describe, and adds an altmodes/displayport node
>     (svid 0xff01) to the connector. That DT change is mine, it is not in
>     mainline and not part of v14 - so everything below is your v14 code,
>     unmodified, driven by that DT.

Sure, Otherwise you cannot test.

> The USB-C link runs through an Epico EC65 "UltraLink 8K/60Hz HDMI to
> USB-C" cable - an active DP alt-mode to HDMI converter (GsCooLink chip,
> USB VID 0x3679) - into the HDMI input of a Samsung Odyssey G70B. So the
> DP link partner your code negotiates with is that converter, not a
> monitor directly.

Correct, the most important part is the DP to HDMI adapter in your
case.

> What I actually exercised, running this kernel as my daily driver since it
> booted on 2026-08-14 at 20:09 local time (17+ hours of continuous uptime
> when I ran these checks):
> 
> - The new glue driver is bound to both controllers:
> 
>     /sys/bus/platform/drivers/dwc3-rockchip/fc000000.usb
>     /sys/bus/platform/drivers/dwc3-rockchip/fc400000.usb
> 
> - DisplayPort alt mode is active at the Type-C layer:
> 
>     /sys/class/typec/port0-partner/port0-partner.0/svid   = ff01
>     /sys/class/typec/port0-partner/port0-partner.0/mode   = 1
>     /sys/class/typec/port0-partner/port0-partner.0/active = yes
> 
>   driving DP-1 at 3840x2160@120 with HDR, simultaneously with two HDMI
>   outputs: HDMI-A-1 to a Sony TV at 3840x2160@60 and HDMI-A-2 to a second
>   Odyssey G70B at 3840x2160@120, both with HDR.
> 
>   Two caveats about DP-1, both pre-existing on my setup and unrelated to
>   your series: the converter emits a broken Y420VDB block, so I feed the
>   connector a corrected EDID through edid_override, and after the EDK2
>   firmware hand-off DP-1 comes up "disconnected" with the cable present,
>   so a boot script does one unbind/bind of the fusb302 i2c device to kick
>   it. Neither workaround was touched during the hotplug test below.

obviously completley unrelated to USBDP PHY.

> - Hot unplug/replug of the USB-C cable. The xHCI host controller was
>   re-registered and the DisplayPort output came back on its own, with no
>   manual intervention:
> 
>     13:00:56  kernel: xhci-hcd xhci-hcd.6.auto: USB bus 4 deregistered
>     13:00:57  kwin_wayland_drm: Removing output
>     13:01:26  kernel: xhci-hcd xhci-hcd.6.auto: xHCI Host Controller
>     13:01:26  kernel: xhci-hcd xhci-hcd.6.auto: Host supports USB 3.0 SuperSpeed
>     13:01:29  kwin_wayland_drm: New output on GPU /dev/dri/card0: Odyssey G70B
> 
>   (kernel and compositor lines interleaved, abridged.) Note the ~30 s
>   between disconnect and re-registration - PD/alt-mode negotiation with
>   this converter is slow. After the replug the connector is back at
>   3840x2160@120 with HDR.

30 seconds for PD negotiation is indeed very slow. Usually it's at
least 10x faster. Have you checked for the root cause via TCPM log?
Also you might want to check if there are firmware updates available
for your adapter.

> - No messages at all from dwc3, dwc3-rockchip or the usbdp PHY for the
>   whole uptime, before or after the replug.
> 
>   For completeness, the DP controller does print a burst of 32
>   "dw-dp fde50000.dp: timeout waiting for AUX reply" right after the cable
>   is pulled - that is the DRM side probing a dead link, it clears on
>   replug and is not from your code. The converter's billboard device also
>   logs one "cdc_acm 3-1:1.1: probe with driver cdc_acm failed with error
>   -22" per connect - a converter quirk, seen daily on this setup since
>   long before this kernel.

The billboard device should be on USB2, so this series does not
affect it.

> One thing I should not hide: I do get a WARN from tcpm, but I trigger it
> myself and it is not from this series:
> 
>   WARNING: drivers/base/devres.c:1184 at devm_kfree+0xb8/0xd0
>   Call trace:
>     devm_kfree
>     tcpm_port_unregister_pd [tcpm]
>     tcpm_unregister_port [tcpm]
>     devm_tcpm_unregister_port [tcpm]
>     devm_action_release / release_nodes / devres_release_group
>     i2c_device_remove / device_release_driver_internal / unbind_store
>
> (trace abridged.) It fires when the boot script mentioned above explicitly
> unbinds fusb302 (i2c 6-0022), i.e. on the devres teardown path introduced
> by 48bf0f5f9ec8
> ("usb: typec: tcpm: add device managed port registration") and fcdd23c1984e
> ("usb: typec: fusb302: Switch to device managed resources") in
> rockchip-devel. Those are not part of v14 and I have not root-caused this,
> but since they are in the branch I tested I thought you would rather know.
> It does not fire on a normal cable unplug/replug, only on an explicit
> driver unbind.

That's unrelated to this series, but I fixed it up in rockchip-devel.

> I am sending the Tested-by tags as separate replies to 28/38, 29/38, 30/38
> and 31/38, so that they land on exactly those patches.
> 
> I am deliberately not tagging 32/38 ("fix USB-C reconnect in gadget mode").
> I have never used this board in gadget mode, so I cannot claim to have
> tested that path.
> 
> One build issue to report, starting at 31/38
> ============================================
> 
> The new glue driver cannot be built as a module once 31/38 is applied.
> 29/38 alone is fine - the glue it introduces only pulls in module.h,
> platform_device.h, pm_runtime.h and glue.h. It is 31/38 that adds
> #include "io.h" and the dwc3_readl()/dwc3_writel() calls.
> 
> Reproducer: ea51774c5c42, arm64 defconfig plus CONFIG_USB_DWC3=y,
> CONFIG_USB_DWC3_ROCKCHIP=m, CONFIG_TRACEPOINTS=y. The build then fails at
> modpost:
> 
>   ERROR: modpost: "__tracepoint_dwc3_readl" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>   ERROR: modpost: "__traceiter_dwc3_readl" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>   ERROR: modpost: "__tracepoint_dwc3_writel" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>   ERROR: modpost: "__traceiter_dwc3_writel" [drivers/usb/dwc3/dwc3-rockchip.ko] undefined!
>   make[2]: *** [scripts/Makefile.modpost:147: Module.symvers] Error 1
>   [...]
> 
> dwc3_readl()/dwc3_writel() call trace_dwc3_readl()/trace_dwc3_writel(),
> and the dwc3 tracepoints are not exported - there is no
> EXPORT_TRACEPOINT_SYMBOL* anywhere in drivers/usb/dwc3/ - so a separate
> module cannot reference them. With CONFIG_TRACEPOINTS=n the trace_* calls
> become empty inlines and this should not trigger; I only tested
> CONFIG_TRACEPOINTS=y.
> 
> dwc3-rockchip.c is the only glue driver in drivers/usb/dwc3/ that calls
> the core's dwc3_readl()/dwc3_writel(). dwc3-st.c also includes io.h, but
> it reads through its own st_dwc3_readl()/st_dwc3_writel(), and e.g.
> dwc3-keystone.c defines kdwc3_readl()/kdwc3_writel() - though those all
> access their own glue registers, not the core register block.
> 
> Two ways to fix it, whichever you prefer:
> 
>   - EXPORT_TRACEPOINT_SYMBOL_GPL(dwc3_readl) and (dwc3_writel) in
>     drivers/usb/dwc3/trace.c, or
>   - open-code the access in the glue, e.g.
>     readl(dwc->regs + DWC3_GUSB3PIPECTL(port) - DWC3_GLOBALS_REGS_START),
>     at the cost of losing the dwc3_readl/dwc3_writel trace events.
> 
> I worked around it locally with CONFIG_USB_DWC3_ROCKCHIP=y, which is how
> the kernel I tested above was built. Note that "default USB_DWC3" means
> the glue follows the core: with CONFIG_USB_DWC3=y the default is =y and
> the problem stays hidden, but with CONFIG_USB_DWC3=m the default would be
> =m. I have not build-tested the CONFIG_USB_DWC3=m case. The Kconfig entry
> added by 29/38 does promise "Say 'Y' or 'M' if you have such device."

Thanks, I only tested non-modular and defconfig (which is modular,
but does not have CONFIG_TRACEPOINTS=y). I don't see a good reason
for duplicating dwc3 readl/writel. Exporting is the reasonable thing
to do and will be done in the next version.

> This reply was prepared with the help of Claude (Anthropic). The board,
> the tests and the measurements are mine, and I checked every claim above
> before sending.

Ideally you trim down its wall of text in future mails.

Greetings,

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy