When the OTG USB port is used to power to SoC, configured as peripheral and
used in gadget mode, communication stops without notice about 6 seconds
after the gadget is configured and enumerated.

The problem was observed on a Radxa Rock Pi S board, which can only be
powered by the only USB-C connector. That connector is the only one usable
in gadget mode. This implies the USB cable is connected from before boot
and never disconnects while the kernel runs.

The related code flow in the PHY driver code can be summarized as:

 * UDC start code (triggered via configfs at any time after boot)
   -> phy_init
       -> rockchip_usb2phy_init
           -> schedule_delayed_work(otg_sm_work [A], 6 sec)
   -> phy_power_on
       -> rockchip_usb2phy_power_on
           -> enable clock
           -> rockchip_usb2phy_reset

 * Now the gadget interface is up and running.

 * 6 seconds later otg_sm_work starts [A]
   -> rockchip_usb2phy_otg_sm_work():
       if (B_IDLE state && VBUS present && ...):
           schedule_delayed_work(&rport->chg_work [B], 0);

 * immediately the chg_detect_work starts [B]
   -> rockchip_chg_detect_work():
       if chg_state is UNDEFINED:
            property_enable(base, &rphy->phy_cfg->chg_det.opmode, false); [Y]

 * rockchip_chg_detect_work() changes state and re-triggers itself a few
   times until it reached the DETECTED state:
   -> rockchip_chg_detect_work():
       if chg_state is DETECTED:
            property_enable(base, &rphy->phy_cfg->chg_det.opmode, true); [Z]

At [Y] there is no disconnection and the USB device appears still present
to userspace, but all existing communications stop. E.g. using a CDC serial
gadget, the /dev/tty* devices are still present on both host and device,
but no data is transferred anymore. The later call with a 'true' argument
at [Z] does not restore it.

Due to the lack of documentation, what chg_det.opmode does exactly is not
clear, however by code inspection it seems reasonable that is disables
something needed to keep the communication working, and testing proves that
disabling these lines lefs gadget mode keep working. So prevent changes to
chg_det.opmode when there is a cable connected (VBUS present).

Fixes: 98898f3bc83c ("phy: rockchip-inno-usb2: support otg-port for rk3399")
Closes: https://lore.kernel.org/lkml/20250414185458.7767aabc@booty/
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 0106d7b7ae24ead91d9c996daaa56671de02a39a..e5efae7b013590f5b0bf65654008cdc167f52e3f 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -833,7 +833,8 @@ static void rockchip_chg_detect_work(struct work_struct *work)
 		if (!rport->suspended && !vbus_attach)
 			rockchip_usb2phy_power_off(rport->phy);
 		/* put the controller in non-driving mode */
-		property_enable(base, &rphy->phy_cfg->chg_det.opmode, false);
+		if (!vbus_attach)
+			property_enable(base, &rphy->phy_cfg->chg_det.opmode, false);
 		/* Start DCD processing stage 1 */
 		rockchip_chg_enable_dcd(rphy, true);
 		rphy->chg_state = USB_CHG_STATE_WAIT_FOR_DCD;
@@ -896,7 +897,8 @@ static void rockchip_chg_detect_work(struct work_struct *work)
 		fallthrough;
 	case USB_CHG_STATE_DETECTED:
 		/* put the controller in normal mode */
-		property_enable(base, &rphy->phy_cfg->chg_det.opmode, true);
+		if (!vbus_attach)
+			property_enable(base, &rphy->phy_cfg->chg_det.opmode, true);
 		rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
 		dev_dbg(&rport->phy->dev, "charger = %s\n",
 			 chg_to_string(rphy->chg_type));

-- 
2.50.1

Hello Luca,

On Tue Jul 22, 2025 at 10:43 AM CEST, Luca Ceresoli wrote:
> When the OTG USB port is used to power to SoC, configured as peripheral and
> used in gadget mode, communication stops without notice about 6 seconds
> after the gadget is configured and enumerated.
>
> The problem was observed on a Radxa Rock Pi S board, which can only be
> powered by the only USB-C connector. That connector is the only one usable
> in gadget mode. This implies the USB cable is connected from before boot
> and never disconnects while the kernel runs.
>
> The related code flow in the PHY driver code can be summarized as:
>
>  * UDC start code (triggered via configfs at any time after boot)
>    -> phy_init
>        -> rockchip_usb2phy_init
>            -> schedule_delayed_work(otg_sm_work [A], 6 sec)
>    -> phy_power_on
>        -> rockchip_usb2phy_power_on
>            -> enable clock
>            -> rockchip_usb2phy_reset

The above code flow summary was important for [PATCH 1/2] but it feels
like not as important for [PATCH 2/2], could you drop or summarise it?
The key point is that the below DCD sequence has invalid assumptions
and does side effects that don't fit if VBUS is already present. I feel
like it distracted me from the main point that is chg_det.opmode
writes.

>  * Now the gadget interface is up and running.
>
>  * 6 seconds later otg_sm_work starts [A]
>    -> rockchip_usb2phy_otg_sm_work():
>        if (B_IDLE state && VBUS present && ...):
>            schedule_delayed_work(&rport->chg_work [B], 0);
>
>  * immediately the chg_detect_work starts [B]
>    -> rockchip_chg_detect_work():
>        if chg_state is UNDEFINED:
>             property_enable(base, &rphy->phy_cfg->chg_det.opmode, false); [Y]
>
>  * rockchip_chg_detect_work() changes state and re-triggers itself a few
>    times until it reached the DETECTED state:
>    -> rockchip_chg_detect_work():
>        if chg_state is DETECTED:
>             property_enable(base, &rphy->phy_cfg->chg_det.opmode, true); [Z]
>
> At [Y] there is no disconnection and the USB device appears still present
> to userspace, but all existing communications stop. E.g. using a CDC serial
> gadget, the /dev/tty* devices are still present on both host and device,
> but no data is transferred anymore. The later call with a 'true' argument
> at [Z] does not restore it.

You mention "there is no disconnection" but that sounds irrelevant to
this precise commit. The issue at hand is a communication halt.

> Due to the lack of documentation, what chg_det.opmode does exactly is not
> clear, however by code inspection it seems reasonable that is disables
> something needed to keep the communication working, and testing proves that
> disabling these lines lefs gadget mode keep working. So prevent changes to
> chg_det.opmode when there is a cable connected (VBUS present).

"lefs" -> "let's", I think

With those nits

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com