Message-Id: <DEHVZPZVWVTR.314J3P86OLG6X@bootlin.com>
Date: Tue, 25 Nov 2025 16:39:01 +0100
From: Théo Lebrun <theo.lebrun@...tlin.com>
To: "Luca Ceresoli" <luca.ceresoli@...tlin.com>, "Vinod Koul"
 <vkoul@...nel.org>, "Kishon Vijay Abraham I" <kishon@...nel.org>, "Heiko
 Stuebner" <heiko@...ech.de>, "William Wu" <wulf@...k-chips.com>
Cc: "Kever Yang" <kever.yang@...k-chips.com>, "Minas Harutyunyan"
 <Minas.Harutyunyan@...opsys.com>, "Alan Stern" <stern@...land.harvard.edu>,
 "Louis Chauvet" <louis.chauvet@...tlin.com>, Hervé Codina
 <herve.codina@...tlin.com>, "Thomas Petazzoni"
 <thomas.petazzoni@...tlin.com>, <linux-phy@...ts.infradead.org>,
 <linux-rockchip@...ts.infradead.org>, <linux-usb@...r.kernel.org>,
 <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] phy: rockchip: inno-usb2: fix communication
 disruption in gadget mode

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@...tlin.com>

Thanks,

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

Powered by blists - more mailing lists