[RFC PATCH v1 0/2] phy: rockchip: inno-usb2: fix USB gadget hot-plug on peripheral-only OTG ports

[RFC PATCH v1 0/2] phy: rockchip: inno-usb2: fix USB gadget hot-plug on peripheral-only OTG ports

[Andreas Zdziarstek]

Hi All,

I had an interesting time trying out the USB OTG micro-USB port on the
Odroid-M1S (Rockchip RK3566) on the mainline Kernel. My intention was to
set up a cdc_ncm+cdc_acm peripheral gadget as a hot-pluggable debug
port.

For the most part that went swimmingly. Just not quite with the
hot-plugging. The apparent first problem was that peripheral mode only
works if the host is connected at boot time of the RK3566 Kernel,
regardless if the dr_mode settings is "otg" or "peripheral". When no
connection is present at boot, gadget setup fails with a
"failed to enable ep0out" error and the port seems to end up in an
unrecoverable state afterwards.

I investigated a little as to why. With my very limited understanding
of the subsystems involved, I think I have identified some problems.
All seem largely related to each other.

* The M1S references its 5V OTG VBUS switch as phy-supply of the OTG
  port in its DT. The phy core unconditionally enables that on phy
  power_on. From then on it powers its own VBUSDET input, making further
  cable VBUS detection impossible. Also, apparently, after reading the
  schematic, that means even with a cable connected at boot (and
  peripheral mode therefore working), the board will try to back-power
  the host. Seems my host port is pretty robust, luckily.
* The inno-usb2 driver seems to generally lack DRP power supply VBUS
  switching support? At least PHY and VBUS supply are semantically
  treated the same.
* The inno-usb2 driver's state machine powers off the Phy in several
  places: on an initial no-VBUS sample, on DCP detection and on
  disconnect. That seems to happen unrelated to controller-initiated
  phy_power_on/off calls and looks like the main cause the gadget setup
  fails and then borks the controller state.

My proposed fixes are in two places:

* set the micro-USB M1S port to "peripheral" in DTS and remove the
  supply node. The board has two USB-A host ports, so device mode is the
  most likely usage scenario for the microUSB and real compliant DRP
  does not seem to be in the cards in the current state. Also, this
  fixes the potentially harmful back-powering issue. (2nd patch)
* keep the inno-usb2 phy driver from autonomously powering off when in
  peripheral mode. This makes hot-plugging and gadget-setup without a
  connected cable work. (1st patch)

This "works great for me". No issues whatsoever with the M1S in device
mode. I can plug/unplug/replug at will at any time, rock solid.

However, I am also doubting myself if I have completely misunderstood
the whole Dual-Role status quo and should have just done something
differently.

Also even if I am right about the problems, I would agree with anyone
saying that this isn't the "proper" fix for the whole situation. It
seems the rockchip vendor kernel is doing DRP related stuff differently,
e.g. an additional vbus-supply setting in DT with apparent support for
VBUS role-switching. Fully automatic OTG with gadget support *should*
be possible to do. Possibly a lot of work, though.

Still, I would say having a solid peripheral-only option including
hot-plugging is an improvement to the status-quo.

Host-only configs in other DTs should be unaffected as the inno-usb2
changes won't matter there. The autonomous power-off will still happen,
for better or worse.

Would love to get some insights on this from the experts.

Cheers,
Andy

Andreas Zdziarstek (2):
  phy: rockchip: inno-usb2: keep peripheral-only OTG port active
  arm64: dts: rockchip: fix gadget hot-plug on ODROID-M1S micro-USB

 .../boot/dts/rockchip/rk3566-odroid-m1s.dts   |  2 +-
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 27 +++++++++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.53.0

[RFC PATCH v1 1/2] phy: rockchip: inno-usb2: keep peripheral-only OTG port active

[Andreas Zdziarstek]

The OTG state machine autonomously suspends the OTG port whenever it
decides no usable VBUS session is present. On the initial sample after
init, when a dedicated charger is detected, on peripheral disconnect
and on host-session end. It calls rockchip_usb2phy_power_off()
directly, underneath the USB controller.

For a dr_mode="peripheral" port this kills the USB gadget on any boot
without an attached cable. The state machine suspends the port shortly
after init, resulting in "failed to enable ep0out", ep0 start transfer
returns -EINVAL). The gadget remains unrecoverable in user space
afterwards.

Skip the state machine's autonomous power-offs when the port is
peripheral-only. VBUS detection, extcon signalling and charger
detection are unaffected, and controller-initiated power management
via phy_power_on()/phy_power_off() keeps working.

Signed-off-by: Andreas Zdziarstek <andreas.zdziarstek@gmail.com>
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 27 +++++++++++++++----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 7d8a533f24ae..9b138b7aaeb8 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -670,6 +670,23 @@ static const struct phy_ops rockchip_usb2phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+/*
+ * Autonomous power-off from the OTG state machine or charger detection.
+ *
+ * A peripheral-only port shouldn't be power-gated behind the controller's
+ * back. If the gadget (re)binds while the PHY is suspended, the
+ * controller's ep0 setup fails and the gadget stays dead even when a VBUS
+ * session appears later. Keep the phy active on peripheral ports for
+ * correct connection detection.
+ */
+static void rockchip_usb2phy_sm_power_off(struct rockchip_usb2phy_port *rport)
+{
+	if (rport->mode == USB_DR_MODE_PERIPHERAL)
+		return;
+
+	rockchip_usb2phy_power_off(rport->phy);
+}
+
 static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 {
 	struct rockchip_usb2phy_port *rport =
@@ -693,7 +710,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 	case OTG_STATE_UNDEFINED:
 		rport->state = OTG_STATE_B_IDLE;
 		if (!vbus_attach)
-			rockchip_usb2phy_power_off(rport->phy);
+			rockchip_usb2phy_sm_power_off(rport);
 		fallthrough;
 	case OTG_STATE_B_IDLE:
 		if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) > 0) {
@@ -719,7 +736,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 					break;
 				case POWER_SUPPLY_TYPE_USB_DCP:
 					dev_dbg(&rport->phy->dev, "dcp cable is connected\n");
-					rockchip_usb2phy_power_off(rport->phy);
+					rockchip_usb2phy_sm_power_off(rport);
 					notify_charger = true;
 					sch_work = true;
 					cable = EXTCON_CHG_USB_DCP;
@@ -765,7 +782,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 			rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
 			rport->state = OTG_STATE_B_IDLE;
 			delay = 0;
-			rockchip_usb2phy_power_off(rport->phy);
+			rockchip_usb2phy_sm_power_off(rport);
 		}
 		sch_work = true;
 		break;
@@ -773,7 +790,7 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 		if (extcon_get_state(rphy->edev, EXTCON_USB_HOST) == 0) {
 			dev_dbg(&rport->phy->dev, "usb otg host disconnect\n");
 			rport->state = OTG_STATE_B_IDLE;
-			rockchip_usb2phy_power_off(rport->phy);
+			rockchip_usb2phy_sm_power_off(rport);
 		}
 		break;
 	default:
@@ -838,7 +855,7 @@ static void rockchip_chg_detect_work(struct work_struct *work)
 	switch (rphy->chg_state) {
 	case USB_CHG_STATE_UNDEFINED:
 		if (!rport->suspended && !vbus_attach)
-			rockchip_usb2phy_power_off(rport->phy);
+			rockchip_usb2phy_sm_power_off(rport);
 		/* put the controller in non-driving mode */
 		if (!vbus_attach)
 			property_enable(rphy->grf, &rphy->phy_cfg->chg_det.opmode, false);
-- 
2.53.0

[RFC PATCH v1 2/2] arm64: dts: rockchip: fix gadget hot-plug on ODROID-M1S micro-USB

[Andreas Zdziarstek]

Referencing the vcc5v0_usb2_otg switch as phy-supply of the OTG port
makes the phy core enable it as soon as the USB controller powers the
PHY, so the board permanently drives its own 5 V onto the micro-USB
connector's VBUS pin. That blinds the PHY's session detection: VBUSDET
(fed from connector VBUS through the on-board 10k/15k divider) is
always high, no bvalid edge can ever occur on plug or unplug, and the
OTG state machine never wakes from its initial state. A USB gadget
only enumerates if the cable is already attached at power-on. It also
back-drives 5 V against the host port.

The inno-usb2 driver has no role-managed VBUS supply support, and the
practical use of this port is as the board's USB device/download port.
Separate Type-A ports are available for host functions.

Drop the phy-supply so the port never sources VBUS and pin the
controller to dr_mode="peripheral" to match that.

Fixes: 10dc64fe0f98 ("arm64: dts: rockchip: Add Hardkernel ODROID-M1S")
Signed-off-by: Andreas Zdziarstek <andreas.zdziarstek@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3566-odroid-m1s.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-odroid-m1s.dts b/arch/arm64/boot/dts/rockchip/rk3566-odroid-m1s.dts
index 33bc5249d729..d9d4a31373a0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-odroid-m1s.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-odroid-m1s.dts
@@ -602,6 +602,7 @@ &usb_host0_ohci {
 };
 
 &usb_host0_xhci {
+	dr_mode = "peripheral";
 	status = "okay";
 };
 
@@ -627,7 +628,6 @@ &usb2phy0_host {
 };
 
 &usb2phy0_otg {
-	phy-supply = <&vcc5v0_usb2_otg>;
 	status = "okay";
 };
 
-- 
2.53.0