Re: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional
From: sashiko-bot@kernel.org
Date: 2026-07-11 20:54:54
Also in:
linux-phy
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The new charger detection logic introduces an infinite connect/disconnect flapping loop for active USB peripheral connections and an infinite polling loop for proprietary chargers. - [High] Transitioning from idle mode to USB mode leaves the PHY transceiver improperly suspended because CPCAP_BIT_USBSUSPEND is not cleared. --
--- Patch [4]: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional ---commit a10ef479eac91f75ee1175abda606fb7eaa3fb70 Author: Ivaylo Dimitrov [off-list ref] phy: cpcap-usb: add DCP detection and make UART idle mode optional Handle DCP separately from USB host connections using CPCAP charger detection status. Make the existing idle UART mode optional via the "enable_uart" module parameter. Initialize the PHY into the baseline configuration required for reliable charger detection during probe.
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c index 741145c89e5bb..2d770ff19e934 100644 --- a/drivers/phy/motorola/phy-cpcap-usb.c +++ b/drivers/phy/motorola/phy-cpcap-usb.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -292,38 +301,74 @@ static void cpcap_usb_detect(struct work_struct *work)
[ ... ]
- /* Otherwise assume we're connected to a USB host */
- if (vbus) {
- dev_dbg(ddata->dev, "connected to USB host\n");
- error = cpcap_usb_set_usb_mode(ddata);
+ /* Check if we're connected to a USB host or charger */
+ if (vbus && ddata->mode != CPCAP_USB_HOST) {
+ enum cpcap_mode new_mode;
+
+ if (s.se1)
+ new_mode = CPCAP_CHARGER;
+ else if (!s.dm && !s.dp)
+ new_mode = CPCAP_USB;
+ else {
+ error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC2,
+ CPCAP_BIT_USBXCVREN |
+ CPCAP_BIT_UARTMUX0 |
+ CPCAP_BIT_EMUMODE0, 0);
+ if (error)
+ goto out_err;
+
+ schedule_delayed_work(&ddata->detect_work,
+ msecs_to_jiffies(100));
+ return;
+ }[Severity: High] Does this else block create a regression with an infinite connect/disconnect flapping loop for active USB peripheral connections? When the device successfully enters CPCAP_USB mode, normal USB pull-ups or traffic will assert D+ high (s.dp == 1). Any subsequent interrupt triggers detect_work(), and if s.se1 is false, it falls into this else block. It then disables the transceiver (USBXCVREN = 0) and reschedules itself in 100ms, repeatedly dropping standard PC connections or proprietary chargers. [ ... ]
quoted hunk ↗ jump to hunk
@@ -424,43 +469,84 @@ static int cpcap_usb_gpio_set_mode(struct cpcap_phy_ddata *ddata,
[ ... ]
+static int cpcap_usb_set_idle_mode(struct cpcap_phy_ddata *ddata)
+{[ ... ]
+ val = CPCAP_BIT_USBSUSPEND; + + if (enable_uart) + val |= (CPCAP_BIT_UARTMUX0 | CPCAP_BIT_EMUMODE0); + + error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC2, 0xffff, val); + if (error) + goto out_err;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -503,11 +578,23 @@ static int cpcap_usb_set_usb_mode(struct cpcap_phy_ddata *ddata) goto out_err; error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC2, - CPCAP_BIT_USBXCVREN, + CPCAP_BIT_USBXCVREN | + CPCAP_BIT_UARTMUX0 | + CPCAP_BIT_EMUMODE0, CPCAP_BIT_USBXCVREN);
[Severity: High] When transitioning back to USB mode, does this cause a regression where the PHY transceiver is left improperly suspended? In cpcap_usb_set_idle_mode(), CPCAP_REG_USBC2 is updated with a 0xffff mask to explicitly set CPCAP_BIT_USBSUSPEND. When returning to USB mode here, the mask is limited to CPCAP_BIT_USBXCVREN | CPCAP_BIT_UARTMUX0 | CPCAP_BIT_EMUMODE0. Since this mask does not include CPCAP_BIT_USBSUSPEND, it looks like it fails to clear the suspension state, which could cause communication failures. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260711204210.197144-1-ivo.g.dimitrov.75@gmail.com?part=4