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

Sebastian Reichel posted 38 patches 1 month, 2 weeks ago
.../bindings/phy/phy-rockchip-usbdp.yaml           |  24 +
drivers/phy/phy-core.c                             |  65 +++
drivers/phy/rockchip/Kconfig                       |   2 +
drivers/phy/rockchip/phy-rockchip-usbdp.c          | 590 ++++++++++-----------
drivers/usb/dwc3/Kconfig                           |  11 +
drivers/usb/dwc3/Makefile                          |   1 +
drivers/usb/dwc3/core.c                            |  17 +-
drivers/usb/dwc3/core.h                            |   9 +
drivers/usb/dwc3/dwc3-rockchip.c                   | 263 +++++++++
include/linux/phy/phy.h                            |  40 ++
10 files changed, 725 insertions(+), 297 deletions(-)
[PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Sebastian Reichel 1 month, 2 weeks ago
This series does a major overhaul of the Rockchip USBDP driver. The
initial main goal was to add USB-C DP AltMode support to the RK3576
and RK3588 and this series still prepares the PHY driver for exactly
that. But in addition to that I uncovered a huge amount of issues,
that are fixed along the way. Some of the more interesting ones are:

 * Currently the driver might trigger a fatal SError on USB-C hotplug,
   since re-initializing the PHY stops the clocks going to DWC3. If
   the DWC3 driver tries to access its registers at the same time the
   system will crash.
 * The DWC3 hardware can get into a buggy state when the PHY is
   disabled, which results in the PHY not coming up properly again.
 * Swithcing the USB-C connector orientation during hotplug breaks
   USB3 speed, as the PHY is not being re-initialized.
 * The code always enables DP mode when USB-C is involved.
 * The driver has some locking issues uncovered by Sashiko.

In addition to these bigger ones, Sashiko also found a bunch of
minor problems, which are mostly harmless, but were fixed while
going at it.

I've tested the code changes with dozens of replugs of different devices
(2 different USB-C hubs with USB3 + HDMI via DP AltMode, 1 USB-C to HDMI
adapter [4 lanes DP AltMode], 1 USB-C to DP adapter [4 lanes DP
AltMode], 1 USB-C to NVMe adapter [no DP AltMode] as well as a direct
USB-C connection to a Dell display) on a Sige 5 board and haven't run
into any issues. I've also tested with DWC3 runtime PM being enabled
manually via sysfs in this round and did not notice any issues. Apart
from that the series is boot tested via CI on Rock 5B and Rock 4D.

Changes in v14:
- Link to v13: https://lore.kernel.org/r/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com
- move PHY reset handling into Rockchip glue driver (Thinh Nguyen)
  - new patch: introduce Rockchip glue driver for dwc3
  - new patch: introduce dwc3 post PHY registration hook for platform glue drivers
  - register the PHY reset notify handlers via the new hook in the
    Rockchip glue driver instead of directly in the dwc3 core
- new patch: recover USB gadget connection on cable replug
- Collect Tested-by: Igor Paunovic <royalnet026@gmail.com>

Changes in v13:
- Link to v12: https://lore.kernel.org/r/20260710-rockchip-usbdp-cleanup-v12-0-8b41a9a9bef0@collabora.com
- Drop "Clear USB status on PHY exit" patch and fully rely on
  "Fix power state handling", which also fixes this problem
  (Sashiko reporting further problems with this)
- Check for highspeed mode in "Avoid xHCI SErrors" (Sashiko)
- In "dwc3: core: support PHY reset notifications" ignore errors
  for pm_runtime_get_if_active() to support !CONFIG_PM and use
  per-port bitmask protected by lock instead of atomic counters
  (Sashiko)
- Update commit message of -EPROBE_DEFER patch to properly mention
  the reset happening in the PHY init routine (Sashiko)
- Fixed bisectability issue in "Rename mode to hw_mode", which I
  accidently introduced in v12 (Sashiko)
- All other pre-existing issues reported by Sashiko in v12 are fixed
  by later patches in the series.

Changes in v12:
- Link to v11: https://lore.kernel.org/r/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com
- Add missing U3 port re-enable in Avoid xHCI SErrors patch (Sashiko)
- Mention possible deadlock issues in phy_notify_state() function
  documentation (Sashiko)
- Avoid runtime resume in dwc3 reset handler, which would result in
  a deadlock, if dwc3 is suspended (Sashiko)
- In patch adding reset notifications to USBDP PHY, also send the
  POST reset notification if rk_udphy_setup fails (Sashiko)
- Rework 'Fix power state handling' patch to adapt to these changes and
  avoid calling rk_udphy_u3_port_disable() when the USB3 PHY side is not
  requested by software (which means the USB power-domain being enabled,
  which is needed for the USB GRF). Previously this was guranteed by the
  runtime PM in the reset handler. The new version is better anyways as
  the old version would run into an SError when DWC3 was not loaded.
  (myself)
- I've not fixed various "pre-existing issues" reported by Sashiko to
  avoid further exploding this series. Also most of them are already
  fixed by later patches in this series anyways.

Changes in v11:
- Link to v10: https://lore.kernel.org/r/20260703-rockchip-usbdp-cleanup-v10-0-a392711ca8a9@collabora.com
- Fix depreated -> deprecated typo in DT binding (Sashiko)
- dwc3 patch: (un)register PHY notifier in probe/remove instead of
  phy_init/exit to avoid AB-BA deadlock (Sashiko)
- dwc3 patch: replace pm_runtime_get_sync by pm_runtime_resume_and_get
  and add error handling (Sashiko)
- implement error handling for PRE_RESET in USBDP driver to match
  this (me)
- dwc3 patch: add reset counter to have balanced runtime PM count if
  dwc3 is removed during an active reset (Sashiko)
- Keep code to disable USB3 in highspeed-only mode in phy_init (Sashiko)
- Always set lane mux in last patch to make sure orientation
  changes are handled properly (Sashiko)
- Update commit message of last patch to mention USB reconnections
  happening during PD state negotiation (Sashiko)

Changes in v10:
- Link to v9: https://lore.kernel.org/r/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com
- Add 'deprecated: true' to port in DT binding, since ports replaces it (Sashiko)
- In 'Drop seamless DP takeover' simply remove any handling for
  pre-enabled PHY as there is no known bootloader doing that and
  Sashiko keeps finding things, which I cannot test. (Sashiko, myself)
- Use on/off instead of enabled/disabled in PHY reinit message,
  which is shorter (myself)
- Use notifier_to_errno() in "add notifier infrastructure" (Sashiko)
- Rework DWC3 PHY reset notifier patch, so that it works correctly
  for multiple ports (Rockchip is single-port) and keep a runtime
  reference while the PHY reset is going on to massively simplify
  the locking logic. (Sashiko)
- Drop patch renaming phy_needs_reinit keep the existing logic to
  set it whenever the lane configuration changes (Sashiko)
- Update "Simplify power state handling" patch, to mostly depend
  on the DT configured or TypeC negotiated modes to avoid
  data stream disconnections when DP is hotplugged in a dock or
  USB is used with runtime PM (Sashiko)
- Ensure sw_mode is not set when the PHY enablement function fails
  (Sashiko)
- Add new patch adding USB-only mode as USB-C state, which results
  in proper powering off the DP side when the remote hardware is
  not capable of DP AltMode. (myself)

Changes in v9:
- Link to v8: https://lore.kernel.org/r/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com
- Update DT binding to explicitly mention that port@3 is for the
  DP aux channel and not DP in general (Sashiko got this wrong)
- Add a 100ms cooldown sleep in "Drop seamless DP takeover" after HPD
  is force disabled (Sashiko)
- Update comment in "Register DP aux bridge" to explain why port@3 is
  checked, but port@0 is used (Sashiko)
- Check for high-speed only mode in "Support going from DP-only mode to
  USB mode" (Sashiko)
- Add new patch for rk_udphy_reset_deassert error handling (Sashiko)
- Add new patch to avoid enabling USB3 in high-speed only mode during
  PHY reinit (Sashiko)
- Add 3 more patches to handle the LCPLL lock issue mentioned in the v8
  cover letter after feedback from Rockchip. Apparently the DWC3 does
  not cope very well with the PHY disappearing resulting in the PIPE
  interface misbehaving, which in turn results in the LCPLL not locking.
  The new patches avoid this by asserting DWC3_GUSB3PIPECTL_PHYSOFTRST.
  As this assert needs to be done when the PHY wants to reset, a new
  notifier system has been implemented to support triggering this from
  the PHY driver. This also means, that this version now also involves
  the USB subsystem.
- Drop old patch trying to solve the DP-only -> USB mode switch in
  favour of 5 new patches completely rewriting and simplifying the
  power status handling. The new code ensures that the PHY always
  has the right modes enabled and also makes sure a re-init happens
  on an orientation change.
- rebased on v7.2-rc1

Changes in v8:
- Link to v7: https://lore.kernel.org/r/20260625-rockchip-usbdp-cleanup-v7-0-38eb3cf654fd@collabora.com
- Move patch "Limit DP lane count to muxed lanes" after single lane
  support, which introduces dp_lanes variable to make sure series
  is bi-sectable (Sashiko)
- Force disable HPD in "Drop seamless DP takeover" patch and update
  patch description to mention potential issues with SErrors for
  bootloaders really keeping the DW-DP on. As mentioned in the new
  commit message this is untested as I'm not aware of such a
  bootloader anyways; this also means we need to keep the HPD GRF
  register defines in the 'Drop DP HPD handling' patch (Sashiko)
- Fix mode logic in "Properly handle TYPEC_STATE_SAFE and
  TYPEC_STATE_USB" patch; I blame the heat (Sashiko)
- Improve "Support going from DP-only mode to USB mode" patch to
  better handle starting in DP only mode; due to TypeC logic
  starting delayed this does not really happen, though (Sashiko)
- Improve "Support going from DP-only mode to USB mode" to avoid
  checking previous state and instead power on USB state based
  on previous requested state to avoid effects from the flip
  callback (Sashiko)
- Update the debug message patch to include some more info
- Ad one more patch, which disables USB3 at startup and drops
  the -EPROBE_DEFER logic

Changes in v7:
- Link to v6: https://lore.kernel.org/r/20260619-rockchip-usbdp-cleanup-v6-0-3bb1f54b3f35@collabora.com
- Add new patch handling missing clock-names in DT gracefully (Sashiko)
- Add new patch handling rk_udphy_reset_deassert_all errors in init check (Sashiko)
- Add new patch to handle Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB (Sashiko)
- Add new patch to avoid xHCI SErrors

Changes in v6:
- Link to v5: https://lore.kernel.org/r/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com
- Add explicit <linux/string_choices.h> include in last patch (Sashiko)
- Add new patch moving mode_change update after error handling (Sashiko)
- Add new patch fixing error masking of devm_clk_bulk_get_all() (Sashiko)
- Add new patch dropping seamless DP takeover as it is non-functional and buggy (Sashiko) 
- Add new patch limiting DP lane count to muxed lanes (Sashiko)
- Add error handling in the patch that keeps clocks running on PHY re-init (Sashiko)
- Also look for DP being configured to second lane for the flip config
  in DP single-lane mode, which should at least keep USB working for
  this super unusual config (Sashiko)
- Drop useless ret variable in patch introducing guard() for the mutex
- Add error handling for PHY re-enablement in the patch fixing support for
  DP-only -> USB mode (Sashiko)

Changes in v5:
- Link to v4: https://lore.kernel.org/r/20260428-rockchip-usbdp-cleanup-v4-0-7775671ece22@collabora.com
- Picked up Acked-by from Rob Herring for DT binding
- Fix typos in commit messages/comments
- Add Fixes tag to "Do not looe USB3 PHY status" patch
- Collect Reviewed-by: Neil Armstrong for multiple patches
- Drop now unused code from "Drop DP HPD handling" patch (Sashiko)
- Ignore mux events not involving DP AltMode (Sashiko)
- Add new patch to support going back from DP only mode to USB combo
  mode; technically this is a fix, but DP mode does not yet work
  upstream, so it does not matter (Sashiko)
- Add new patch adding a few debug messages, which are useful
  to investigate potential hotplug issues in the future
- Sashiko comments about the DT binding and property usage
  are wrong as the first port is for the superspeed lanes
  used for DP and USB, while the last port is just about
  DP aux. I ignored them.
- There is a pre-existing bug, that can already be hit with the
  upstream kernel and that the series doesn't fix properly:
  Accessing the USB3 controller registers requires the USB PHY
  running, since it provides a clock. Re-initializing the PHY
  means there is a race-condition - if the system tries to access
  the USB3 controller in parallel to the re-init, the system will
  hang and/or fail with an SError. By keeping the clocks running
  and only asserting the resets this time is minimized by this
  series. A proper fix for this will be looked into independently
  from this series.
- I used v7.1-rc6 as base, but the driver has no changes since
  6.18 even in linux-next and there are no pending patches for
  it on the mailinglist either, so it applies to *any* recent
  kernel branch.

Changes in v4:
- Link to v3: https://lore.kernel.org/r/20260313-rockchip-usbdp-cleanup-v3-0-3e8fe89a35b5@collabora.com
- rebased to v7.1-rc1 (no changes)
- Update DRM bridge registration patch to avoid registration when DP aux
  port is not connected to anything, since this results in errors and some
  boards use USBDP instances for USB3 only.
- Add patch renaming mode_change into phy_needs_reinit
- Add patch to re-init PHY on orientation change
- Add patch to factor out lane_mux_sel setup
- Add patch to handle mutex via guard functions

Changes in v3:
- Link to v2: https://lore.kernel.org/r/20260213-rockchip-usbdp-cleanup-v2-0-b67ec225f96e@collabora.com
- Add patch to register the USBDP PHY as DRM bridge
- Add patch to describe ports in DT binding (used by the DRM bridge)
- Add patch to drop HPD handling from the PHY

Changes in v2:
- Link to v1: https://lore.kernel.org/r/20260203-rockchip-usbdp-cleanup-v1-0-16a6f92ed176@collabora.com
- Added new patches to fix USB3 SError

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Frank Wang (1):
      phy: rockchip: usbdp: Amend SSC modulation deviation

Sebastian Reichel (35):
      dt-bindings: phy: rockchip-usbdp: add improved ports scheme
      phy: rockchip: usbdp: Update mode_change after error handling
      phy: rockchip: usbdp: Do not lose USB3 PHY status
      phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check
      phy: rockchip: usbdp: Handle missing clock-names DT property gracefully
      phy: rockchip: usbdp: Drop seamless DP takeover
      phy: rockchip: usbdp: Keep clocks running on PHY re-init
      phy: rockchip: usbdp: Add missing mode_change update
      phy: rockchip: usbdp: Limit DP lane count to muxed lanes
      phy: rockchip: usbdp: Rename DP lane functions
      phy: rockchip: usbdp: Use FIELD_PREP_WM16_CONST
      phy: rockchip: usbdp: Cleanup DP lane selection function
      phy: rockchip: usbdp: Register DP aux bridge
      phy: rockchip: usbdp: Drop DP HPD handling
      phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit
      phy: rockchip: usbdp: Re-init the PHY on orientation change
      phy: rockchip: usbdp: Factor out lane_mux_sel setup
      phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
      phy: rockchip: usbdp: Use guard functions for mutex
      phy: rockchip: usbdp: Hold mutex in DP PHY configure
      phy: rockchip: usbdp: Add some extra debug messages
      phy: rockchip: usbdp: Avoid xHCI SErrors
      phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors
      phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode
      phy: core: add notifier infrastructure
      usb: dwc3: rockchip: introduce glue driver
      usb: dwc3: core: add post PHY registration hook for platform glue
      usb: dwc3: rockchip: support PHY reset notifications
      usb: dwc3: rockchip: fix USB-C reconnect in gadget mode
      phy: rockchip: usbdp: Add phy reset notification support
      phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
      phy: rockchip: usbdp: Rename mode to hw_mode
      phy: rockchip: usbdp: Fix power state handling
      phy: rockchip: usbdp: Re-init PHY on mux change
      phy: rockchip: usbdp: Add USB-C state without DP enabled

William Wu (1):
      phy: rockchip: usbdp: Fix LFPS detect threshold control

Zhang Yubing (1):
      phy: rockchip: usbdp: Support single-lane DP

 .../bindings/phy/phy-rockchip-usbdp.yaml           |  24 +
 drivers/phy/phy-core.c                             |  65 +++
 drivers/phy/rockchip/Kconfig                       |   2 +
 drivers/phy/rockchip/phy-rockchip-usbdp.c          | 590 ++++++++++-----------
 drivers/usb/dwc3/Kconfig                           |  11 +
 drivers/usb/dwc3/Makefile                          |   1 +
 drivers/usb/dwc3/core.c                            |  17 +-
 drivers/usb/dwc3/core.h                            |   9 +
 drivers/usb/dwc3/dwc3-rockchip.c                   | 263 +++++++++
 include/linux/phy/phy.h                            |  40 ++
 10 files changed, 725 insertions(+), 297 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260203-rockchip-usbdp-cleanup-5b59dfb561a3

Best regards,
--  
Sebastian Reichel <sebastian.reichel@collabora.com>
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Manivannan Sadhasivam 1 month, 1 week ago
On Thu, Aug 13, 2026 at 10:51:43PM +0200, Sebastian Reichel wrote:
> This series does a major overhaul of the Rockchip USBDP driver. The
> initial main goal was to add USB-C DP AltMode support to the RK3576
> and RK3588 and this series still prepares the PHY driver for exactly
> that. But in addition to that I uncovered a huge amount of issues,
> that are fixed along the way. Some of the more interesting ones are:
> 
>  * Currently the driver might trigger a fatal SError on USB-C hotplug,
>    since re-initializing the PHY stops the clocks going to DWC3. If
>    the DWC3 driver tries to access its registers at the same time the
>    system will crash.
>  * The DWC3 hardware can get into a buggy state when the PHY is
>    disabled, which results in the PHY not coming up properly again.
>  * Swithcing the USB-C connector orientation during hotplug breaks
>    USB3 speed, as the PHY is not being re-initialized.
>  * The code always enables DP mode when USB-C is involved.
>  * The driver has some locking issues uncovered by Sashiko.
> 
> In addition to these bigger ones, Sashiko also found a bunch of
> minor problems, which are mostly harmless, but were fixed while
> going at it.
> 

38 patches for a single series is too much to review. Please consider splitting
it up into multiple series not exceeding ~10 patches per series. Thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Sebastian Reichel 1 month, 1 week ago
Hello Mani,

On Tue, Aug 18, 2026 at 11:27:07AM +0200, Manivannan Sadhasivam wrote:
> On Thu, Aug 13, 2026 at 10:51:43PM +0200, Sebastian Reichel wrote:
> > This series does a major overhaul of the Rockchip USBDP driver. The
> > initial main goal was to add USB-C DP AltMode support to the RK3576
> > and RK3588 and this series still prepares the PHY driver for exactly
> > that. But in addition to that I uncovered a huge amount of issues,
> > that are fixed along the way. Some of the more interesting ones are:
> > 
> >  * Currently the driver might trigger a fatal SError on USB-C hotplug,
> >    since re-initializing the PHY stops the clocks going to DWC3. If
> >    the DWC3 driver tries to access its registers at the same time the
> >    system will crash.
> >  * The DWC3 hardware can get into a buggy state when the PHY is
> >    disabled, which results in the PHY not coming up properly again.
> >  * Swithcing the USB-C connector orientation during hotplug breaks
> >    USB3 speed, as the PHY is not being re-initialized.
> >  * The code always enables DP mode when USB-C is involved.
> >  * The driver has some locking issues uncovered by Sashiko.
> > 
> > In addition to these bigger ones, Sashiko also found a bunch of
> > minor problems, which are mostly harmless, but were fixed while
> > going at it.
> 
> 38 patches for a single series is too much to review. Please
> consider splitting it up into multiple series not exceeding ~10
> patches per series. Thanks!

So let me summarize:

 - Vinod wants Sashiko feedback to be acted upon
 - linux-phy does not want series with dependencies ( [0] )
 - You don't want series with more than ~10 patches
 - Generally fixes should be at the start of a series

This tremendously slows down any real work. The first series would
just be Sashiko fixes without *any* of the work I originally wanted
to do (DP AltMode). I would have to wait for it to land before
working on the next bits as there are massive code conflicts with
the same file being heavily modified. This simply does not scale,
especially when considering that there are more dependencies on this
series.

[0] https://lore.kernel.org/linux-phy/20260330233532.ulnqiqrogezswht2@skbuf/

Greetings,

-- Sebastian
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Manivannan Sadhasivam 1 month, 1 week ago
On Tue, Aug 18, 2026 at 11:42:50PM +0200, Sebastian Reichel wrote:
> Hello Mani,
> 
> On Tue, Aug 18, 2026 at 11:27:07AM +0200, Manivannan Sadhasivam wrote:
> > On Thu, Aug 13, 2026 at 10:51:43PM +0200, Sebastian Reichel wrote:
> > > This series does a major overhaul of the Rockchip USBDP driver. The
> > > initial main goal was to add USB-C DP AltMode support to the RK3576
> > > and RK3588 and this series still prepares the PHY driver for exactly
> > > that. But in addition to that I uncovered a huge amount of issues,
> > > that are fixed along the way. Some of the more interesting ones are:
> > > 
> > >  * Currently the driver might trigger a fatal SError on USB-C hotplug,
> > >    since re-initializing the PHY stops the clocks going to DWC3. If
> > >    the DWC3 driver tries to access its registers at the same time the
> > >    system will crash.
> > >  * The DWC3 hardware can get into a buggy state when the PHY is
> > >    disabled, which results in the PHY not coming up properly again.
> > >  * Swithcing the USB-C connector orientation during hotplug breaks
> > >    USB3 speed, as the PHY is not being re-initialized.
> > >  * The code always enables DP mode when USB-C is involved.
> > >  * The driver has some locking issues uncovered by Sashiko.
> > > 
> > > In addition to these bigger ones, Sashiko also found a bunch of
> > > minor problems, which are mostly harmless, but were fixed while
> > > going at it.
> > 
> > 38 patches for a single series is too much to review. Please
> > consider splitting it up into multiple series not exceeding ~10
> > patches per series. Thanks!
> 
> So let me summarize:
> 
>  - Vinod wants Sashiko feedback to be acted upon

This is fine.

>  - linux-phy does not want series with dependencies ( [0] )

This is not something I expected. I read the reply from Vladimir in [0], and
he seems to be sharing the limitation of the build tool of linux-phy. But I
don't think that's a big deal. Every subsystem allows sending dependent series
as long as the dependency is clearly described in the cover letter and now with
b4. So I don't see why linux-phy should be different. If you combine all patches
in one series, it makes it impossible for a human reviewer to review it
thoroughly.

@vinod: Can you share your view on splitting the series with dependency?

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Vinod Koul 1 month ago
On 19-08-26, 05:31, Manivannan Sadhasivam wrote:
> On Tue, Aug 18, 2026 at 11:42:50PM +0200, Sebastian Reichel wrote:
> > Hello Mani,
> > 
> > On Tue, Aug 18, 2026 at 11:27:07AM +0200, Manivannan Sadhasivam wrote:
> > > On Thu, Aug 13, 2026 at 10:51:43PM +0200, Sebastian Reichel wrote:
> > > > This series does a major overhaul of the Rockchip USBDP driver. The
> > > > initial main goal was to add USB-C DP AltMode support to the RK3576
> > > > and RK3588 and this series still prepares the PHY driver for exactly
> > > > that. But in addition to that I uncovered a huge amount of issues,
> > > > that are fixed along the way. Some of the more interesting ones are:
> > > > 
> > > >  * Currently the driver might trigger a fatal SError on USB-C hotplug,
> > > >    since re-initializing the PHY stops the clocks going to DWC3. If
> > > >    the DWC3 driver tries to access its registers at the same time the
> > > >    system will crash.
> > > >  * The DWC3 hardware can get into a buggy state when the PHY is
> > > >    disabled, which results in the PHY not coming up properly again.
> > > >  * Swithcing the USB-C connector orientation during hotplug breaks
> > > >    USB3 speed, as the PHY is not being re-initialized.
> > > >  * The code always enables DP mode when USB-C is involved.
> > > >  * The driver has some locking issues uncovered by Sashiko.
> > > > 
> > > > In addition to these bigger ones, Sashiko also found a bunch of
> > > > minor problems, which are mostly harmless, but were fixed while
> > > > going at it.
> > > 
> > > 38 patches for a single series is too much to review. Please
> > > consider splitting it up into multiple series not exceeding ~10
> > > patches per series. Thanks!
> > 
> > So let me summarize:
> > 
> >  - Vinod wants Sashiko feedback to be acted upon
> 
> This is fine.
> 
> >  - linux-phy does not want series with dependencies ( [0] )
> 
> This is not something I expected. I read the reply from Vladimir in [0], and
> he seems to be sharing the limitation of the build tool of linux-phy. But I
> don't think that's a big deal. Every subsystem allows sending dependent series
> as long as the dependency is clearly described in the cover letter and now with
> b4. So I don't see why linux-phy should be different. If you combine all patches
> in one series, it makes it impossible for a human reviewer to review it
> thoroughly.
> 
> @vinod: Can you share your view on splitting the series with dependency?

I build locally when I apply and if there is a dependency on fixes,
cover letter mentioning, I do merge fixes.
So not sure from a process pov where the gap might be.

Builders are helpful for review and checking sanity :-)

I would say it would be helpful, if it was split. I can pick fixes now
and get that in for rc. While the rest can go into next

-- 
~Vinod
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Igor Paunovic 1 month, 2 weeks ago
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.

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

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.

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.

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

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

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.

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

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.

Best regards,
Igor
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Sebastian Reichel 1 month, 1 week ago
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
Re: [PATCH v14 00/38] phy: rockchip: usbdp: Clean up the mess
Posted by Igor Paunovic 1 month, 1 week ago
Hi Sebastian,

> 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?

I have now, and I owe you a correction: the 30 s was my measurement
artifact, not negotiation time.

The journal does not record the replug instant, so the figure I quoted
was deregistration to re-registration - including however long the
cable simply sat unplugged. Today, with the TCPM debugfs log and a
controlled replug:

  [12954.114919] CC1: 0 -> 2, CC2: 0 -> 1   <- replug
  [12955.837527] PD RX, header: 0x2e8f      <- PD exchange running

DRM reports the connector connected 2.1 s after the replug and the
compositor lights the output about 4 s after. The adapter negotiates
at normal speed; nothing to chase. Sorry for the noise, and thanks
for the tcpm devres fix.

Regards,
Igor