[PATCH 0/5] phy: cpcap-usb: improve charger detection and export cable state

[PATCH 0/5] phy: cpcap-usb: improve charger detection and export cable state

[Ivaylo Dimitrov]

The Motorola CPCAP USB PHY contains the hardware state machine used for
USB cable detection. Besides distinguishing USB peripheral and host
connections, it can also detect dedicated charging ports (DCP).

This series starts with a prerequisite fix from Tony Lindgren to prevent
spurious SysRq events when switching between USB and UART modes. It then
adds DCP detection support to the CPCAP USB PHY, updates the Device Tree
binding and corresponding mapphone Device Tree to use the charger
detection interrupt, and finally exports the detected cable state through
the Extcon framework.

The existing driver already interprets the CPCAP USB detection state
machine to determine the attached cable type. This series extends that
logic to distinguish DCP connections and exposes the detected cable state
through Extcon using a standard kernel interface. It also makes the idle
UART mode optional, allowing the PHY to remain in its default USB
detection configuration unless UART support is explicitly requested.

The series has been tested on Motorola Droid 4 hardware.

Ivaylo Dimitrov (4):
  phy: cpcap-usb: add DCP detection and make UART idle mode optional
  dt-bindings: phy: motorola,cpcap-usb: replace se1 interrupt with
    chrg_det
  ARM: dts: ti: cpcap-mapphone: use charger detection interrupt for
    CPCAP USB PHY
  phy: cpcap-usb: add extcon support

Tony Lindgren (1):
  phy: cpcap-usb: Prevent line glitches from triggering sysrq

 .../bindings/phy/motorola,cpcap-usb-phy.yaml  |   8 +-
 .../dts/ti/omap/motorola-cpcap-mapphone.dtsi  |   4 +-
 drivers/phy/motorola/phy-cpcap-usb.c          | 312 +++++++++++++++---
 3 files changed, 274 insertions(+), 50 deletions(-)

-- 
2.25.1

[PATCH 1/5] phy: cpcap-usb: Prevent line glitches from triggering sysrq

[Ivaylo Dimitrov]

From: Tony Lindgren <tony@atomide.com>

We can get random sysrq events on switching from USB to debug serial
port mode unless is temporarily disable sysrq for the mode changes.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/phy/motorola/phy-cpcap-usb.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
index 7cb020dd3423..c6cc8fce6940 100644
--- a/drivers/phy/motorola/phy-cpcap-usb.c
+++ b/drivers/phy/motorola/phy-cpcap-usb.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/sysrq.h>
 
 #include <linux/gpio/consumer.h>
 #include <linux/mfd/motorola-cpcap.h>
@@ -413,7 +414,12 @@ static int cpcap_usb_gpio_set_mode(struct cpcap_phy_ddata *ddata,
 
 static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
 {
-	int error;
+	int sysrq, error;
+
+	/* Disable sysrq to prevent random sysrq events on line glitches */
+	sysrq = sysrq_mask();
+	if (sysrq & 1)
+		sysrq_toggle_support(sysrq & ~1);
 
 	/* Disable lines to prevent glitches from waking up mdm6600 */
 	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_UNKNOWN_DISABLED);
@@ -448,6 +454,9 @@ static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
 	if (error)
 		goto out_err;
 
+	if (sysrq & 1)
+		sysrq_toggle_support(sysrq);
+
 	return 0;
 
 out_err:
-- 
2.25.1

Re: [PATCH 1/5] phy: cpcap-usb: Prevent line glitches from triggering sysrq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] The global SysRq support mask is permanently altered if an error occurs during UART mode configuration.
- [High] Flawed bitwise logic fails to effectively disable SysRq in system configurations that use specific feature masks, leaving the system vulnerable to glitches.
- [High] The driver modifies global system configuration to mask a local hardware glitch, introducing system-wide side effects and race conditions.

Pre-existing issues:
- [High] A Use-After-Free vulnerability exists because devres-managed IRQs can queue delayed work on freed memory after `remove()` completes.
- [High] Hardware teardown in `remove()` races with the background worker, leading to undefined hardware state.
--

commit d89e10225f70bc2771f7d996decb42920d32fe88
Author: Tony Lindgren <tony@atomide.com>

phy: cpcap-usb: Prevent line glitches from triggering sysrq

This commit attempts to prevent random sysrq events when switching from USB
to debug serial port mode. It temporarily disables sysrq during mode changes
to mask line glitches.

> diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
> --- a/drivers/phy/motorola/phy-cpcap-usb.c
> +++ b/drivers/phy/motorola/phy-cpcap-usb.c

[ ... ]

> @@ -413,7 +414,12 @@ static int cpcap_usb_gpio_set_mode(struct cpcap_phy_ddata *ddata,
>  
>  static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
>  {
> -	int error;
> +	int sysrq, error;
> +
> +	/* Disable sysrq to prevent random sysrq events on line glitches */
> +	sysrq = sysrq_mask();
> +	if (sysrq & 1)
> +		sysrq_toggle_support(sysrq & ~1);

[Severity: High]
Does modifying the global sysrq state here create a race condition?

If an administrator concurrently modifies the sysrq settings via
/proc/sys/kernel/sysrq, will the restoration call at the end of this
function blindly overwrite their new state?

[Severity: High]
Will this correctly disable sysrq if the system is configured with a
specific feature mask?

If the administrator clears the SYSRQ_ENABLE_ALL bit (bit 0) but leaves
other features enabled, sysrq & 1 evaluates to 0, bypassing the disablement.

If bit 0 and other bits are set, sysrq & ~1 disables the 'enable all' flag
but leaves the specific logging/dumping features actively enabled.

[ ... ]

> @@ -448,6 +454,9 @@ static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
>  	if (error)
>  		goto out_err;
>  
> +	if (sysrq & 1)
> +		sysrq_toggle_support(sysrq);
> +
>  	return 0;
>  
>  out_err:

[Severity: High]
Does this error path permanently alter the sysrq support mask?

If a failure occurs during the GPIO or pinctrl mode configuration, the
function jumps to out_err and bypasses the restoration of the sysrq state.
Could this permanently disable sysrq features system-wide?


[Severity: High]
This is a pre-existing issue, but is there a use-after-free vulnerability
during device teardown?

Looking at cpcap_phy_irq_thread():

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_phy_irq_thread() {
    if (!atomic_read(&ddata->active))
        return IRQ_NONE;
    
    schedule_delayed_work(&ddata->detect_work, msecs_to_jiffies(1));
}

And in cpcap_usb_phy_remove(), cancel_delayed_work_sync() is called, but
because devm_request_threaded_irq() is used, the IRQ is freed after
remove() finishes.

If the IRQ thread is preempted immediately after reading ddata->active,
can it resume during free_irq(), and queue detect_work on freed memory
after devres frees ddata?


[Severity: High]
This is a pre-existing issue, but does the hardware teardown in
cpcap_usb_phy_remove() race with the background worker?

Looking at cpcap_usb_phy_remove():

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_phy_remove() {
    cpcap_usb_set_idle_mode(ddata);
    cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_OFF);
    
    usb_remove_phy(&ddata->phy);
    cancel_delayed_work_sync(&ddata->detect_work);
}

The driver attempts to force an idle state before stopping the background
worker cpcap_usb_detect().

Since the worker thread doesn't check ddata->active, could it concurrently
modify the same regmap registers and pinctrl states, leaving the hardware
in an undefined state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704073843.1750458-1-ivo.g.dimitrov.75@gmail.com?part=1

[PATCH 2/5] phy: cpcap-usb: add DCP detection and make UART idle mode optional

[Ivaylo Dimitrov]

Handle DCP separately from USB host connections based on CPCAP charger
detection status.

Make the existing idle UART mode optional via the "enable_uart" module
parameter. When disabled, the PHY remains in its USB/charger detection
configuration while idle. This reduces power usage in idle by 25mW.

Also initialize the PHY into the baseline configuration required for
reliable charger detection during probe.

Note: Enabling UART idle mode increases idle power consumption.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/phy/motorola/phy-cpcap-usb.c | 235 ++++++++++++++++++++++-----
 1 file changed, 191 insertions(+), 44 deletions(-)

diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
index c6cc8fce6940..3be2b3afaaf4 100644
--- a/drivers/phy/motorola/phy-cpcap-usb.c
+++ b/drivers/phy/motorola/phy-cpcap-usb.c
@@ -111,6 +111,14 @@ enum cpcap_gpio_mode {
 	CPCAP_OTG_DM_DP,
 };
 
+enum cpcap_mode {
+	CPCAP_IDLE,
+	CPCAP_CHARGER,
+	CPCAP_USB,
+	CPCAP_USB_HOST,
+	CPCAP_DOCK,
+};
+
 struct cpcap_phy_ddata {
 	struct regmap *reg;
 	struct device *dev;
@@ -125,10 +133,13 @@ struct cpcap_phy_ddata {
 	struct iio_channel *id;
 	struct regulator *vusb;
 	atomic_t active;
-	unsigned int vbus_provider:1;
-	unsigned int docked:1;
+	enum cpcap_mode mode;
 };
 
+static bool cpcap_enable_uart;
+module_param_named(enable_uart, cpcap_enable_uart, bool, 0644);
+MODULE_PARM_DESC(enable_uart,
+		 "Enable UART on the USB connector while idle (increases power consumption)");
 static bool cpcap_usb_vbus_valid(struct cpcap_phy_ddata *ddata)
 {
 	int error, value = 0;
@@ -176,6 +187,7 @@ static int cpcap_phy_get_ints_state(struct cpcap_phy_ddata *ddata,
 
 	s->id_ground = val & BIT(15);
 	s->id_float = val & BIT(14);
+	s->chrg_det = val & BIT(13);
 	s->vbusov = val & BIT(11);
 
 	error = regmap_read(ddata->reg, CPCAP_REG_INTS2, &val);
@@ -197,8 +209,9 @@ static int cpcap_phy_get_ints_state(struct cpcap_phy_ddata *ddata,
 	return 0;
 }
 
-static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata);
+static int cpcap_usb_set_idle_mode(struct cpcap_phy_ddata *ddata);
 static int cpcap_usb_set_usb_mode(struct cpcap_phy_ddata *ddata);
+static int cpcap_usb_set_dcp_mode(struct cpcap_phy_ddata *ddata);
 
 static void cpcap_usb_try_musb_mailbox(struct cpcap_phy_ddata *ddata,
 				       enum musb_vbus_id_status status)
@@ -229,7 +242,7 @@ static void cpcap_usb_detect(struct work_struct *work)
 	vbus = cpcap_usb_vbus_valid(ddata);
 
 	/* We need to kick the VBUS as USB A-host */
-	if (s.id_ground && ddata->vbus_provider) {
+	if (s.id_ground && ddata->mode == CPCAP_USB_HOST) {
 		dev_dbg(ddata->dev, "still in USB A-host mode, kicking VBUS\n");
 
 		cpcap_usb_try_musb_mailbox(ddata, MUSB_ID_GROUND);
@@ -244,7 +257,7 @@ static void cpcap_usb_detect(struct work_struct *work)
 		return;
 	}
 
-	if (vbus && s.id_ground && ddata->docked) {
+	if (vbus && s.id_ground && ddata->mode == CPCAP_DOCK) {
 		dev_dbg(ddata->dev, "still docked as A-host, signal ID down\n");
 
 		cpcap_usb_try_musb_mailbox(ddata, MUSB_ID_GROUND);
@@ -253,10 +266,10 @@ static void cpcap_usb_detect(struct work_struct *work)
 	}
 
 	/* No VBUS needed with docks */
-	if (vbus && s.id_ground && !ddata->vbus_provider) {
+	if (vbus && s.id_ground && ddata->mode != CPCAP_USB_HOST) {
 		dev_dbg(ddata->dev, "connected to a dock\n");
 
-		ddata->docked = true;
+		ddata->mode = CPCAP_DOCK;
 
 		error = cpcap_usb_set_usb_mode(ddata);
 		if (error)
@@ -275,10 +288,10 @@ static void cpcap_usb_detect(struct work_struct *work)
 		return;
 	}
 
-	if (s.id_ground && !ddata->docked) {
+	if (s.id_ground && ddata->mode != CPCAP_DOCK) {
 		dev_dbg(ddata->dev, "id ground, USB host mode\n");
 
-		ddata->vbus_provider = true;
+		ddata->mode = CPCAP_USB_HOST;
 
 		error = cpcap_usb_set_usb_mode(ddata);
 		if (error)
@@ -296,36 +309,53 @@ static void cpcap_usb_detect(struct work_struct *work)
 		return;
 	}
 
-	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3,
-				   CPCAP_BIT_VBUSSTBY_EN |
-				   CPCAP_BIT_VBUSEN_SPI, 0);
-	if (error)
-		goto out_err;
-
 	vbus = cpcap_usb_vbus_valid(ddata);
 
-	/* 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 */
+	if (vbus && ddata->mode != CPCAP_USB_HOST) {
+		if (!s.chrg_det || !s.sessvld || ddata->mode != CPCAP_IDLE)
+			return;
+
+		if (s.se1)
+			ddata->mode = CPCAP_CHARGER;
+		else if (!s.dm && !s.dp)
+			ddata->mode = CPCAP_USB;
+		else
+			return;
+
+		dev_dbg(ddata->dev, "connected to %s\n",
+			s.se1 ? "DCP" : "USB host");
+
+		error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3,
+					   CPCAP_BIT_VBUSSTBY_EN |
+					   CPCAP_BIT_VBUSEN_SPI, 0);
 		if (error)
 			goto out_err;
-		cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_VALID);
+
+		if (ddata->mode == CPCAP_CHARGER)
+			error = cpcap_usb_set_dcp_mode(ddata);
+		else
+			error = cpcap_usb_set_usb_mode(ddata);
+
+		if (error)
+			goto out_err;
+
+		if (ddata->mode == CPCAP_USB)
+			cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_VALID);
 
 		return;
 	}
 
-	ddata->vbus_provider = false;
-	ddata->docked = false;
+	if (ddata->mode == CPCAP_IDLE)
+		return;
+
+	ddata->mode = CPCAP_IDLE;
 	cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_OFF);
 
-	/* Default to debug UART mode */
-	error = cpcap_usb_set_uart_mode(ddata);
+	error = cpcap_usb_set_idle_mode(ddata);
 	if (error)
 		goto out_err;
 
-	dev_dbg(ddata->dev, "set UART mode\n");
-
 	return;
 
 out_err:
@@ -374,7 +404,7 @@ static const char * const cpcap_phy_irqs[] = {
 	"id_ground", "id_float",
 
 	/* REG_INT1 */
-	"se0conn", "vbusvld", "sessvld", "sessend", "se1",
+	"se0conn", "vbusvld", "sessvld", "sessend", "chrg_det",
 
 	/* REG_INT_3 */
 	"dm", "dp",
@@ -412,9 +442,10 @@ static int cpcap_usb_gpio_set_mode(struct cpcap_phy_ddata *ddata,
 	return 0;
 }
 
-static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
+static int cpcap_usb_set_idle_mode(struct cpcap_phy_ddata *ddata)
 {
 	int sysrq, error;
+	unsigned int val;
 
 	/* Disable sysrq to prevent random sysrq events on line glitches */
 	sysrq = sysrq_mask();
@@ -426,30 +457,52 @@ static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
 	if (error)
 		goto out_err;
 
-	if (ddata->pins_uart) {
-		error = pinctrl_select_state(ddata->pins, ddata->pins_uart);
+	if (cpcap_enable_uart && ddata->pins_uart) {
+		error = pinctrl_select_state(ddata->pins,  ddata->pins_uart);
+		if (error)
+			goto out_err;
+	} else if (!cpcap_enable_uart && ddata->pins_ulpi) {
+		error = pinctrl_select_state(ddata->pins, ddata->pins_ulpi);
 		if (error)
 			goto out_err;
 	}
 
 	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC1,
+				   CPCAP_BIT_DP150KPU |
+				   CPCAP_BIT_DP1K5PU |
+				   CPCAP_BIT_DM1K5PU |
+				   CPCAP_BIT_DPPD |
+				   CPCAP_BIT_DMPD |
 				   CPCAP_BIT_VBUSPD,
+				   CPCAP_BIT_DP150KPU |
 				   CPCAP_BIT_VBUSPD);
 	if (error)
 		goto out_err;
 
-	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC2,
-				   0xffff, CPCAP_BIT_UARTMUX0 |
-				   CPCAP_BIT_EMUMODE0);
+	val = CPCAP_BIT_USBSUSPEND;
+
+	if (cpcap_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;
 
-	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3, 0x7fff,
-				   CPCAP_BIT_IDPU_SPI);
+	val = CPCAP_BIT_VBUSSTBY_EN |
+	      CPCAP_BIT_VBUSPD_SPI |
+	      CPCAP_BIT_DMPD_SPI |
+	      CPCAP_BIT_DPPD_SPI |
+	      CPCAP_BIT_PU_SPI |
+	      CPCAP_BIT_IDPU_SPI |
+	      CPCAP_BIT_ULPI_SPI_SEL;
+
+	if (!cpcap_enable_uart)
+		val |= CPCAP_BIT_SUSPEND_SPI;
+
+	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3, 0x7fff, val);
 	if (error)
 		goto out_err;
 
-	/* Enable UART mode */
 	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_DM_DP);
 	if (error)
 		goto out_err;
@@ -457,6 +510,8 @@ static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
 	if (sysrq & 1)
 		sysrq_toggle_support(sysrq);
 
+	dev_dbg(ddata->dev, "set %s mode\n", cpcap_enable_uart ? "UART" : "IDLE");
+
 	return 0;
 
 out_err:
@@ -517,6 +572,42 @@ static int cpcap_usb_set_usb_mode(struct cpcap_phy_ddata *ddata)
 	return error;
 }
 
+static int cpcap_usb_set_dcp_mode(struct cpcap_phy_ddata *ddata)
+{
+	int error = 0;
+
+	/* Disable lines to prevent glitches from waking up mdm6600 */
+	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_UNKNOWN_DISABLED);
+	if (error)
+		return error;
+
+	if (ddata->pins_ulpi) {
+		error = pinctrl_select_state(ddata->pins, ddata->pins_ulpi);
+		if (error) {
+			dev_err(ddata->dev, "could not set usb mode: %i\n",
+				error);
+
+			return error;
+		}
+	}
+
+	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3,
+				   CPCAP_BIT_SUSPEND_SPI, 0);
+	if (error)
+		goto out_err;
+
+	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_DM_DP);
+	if (error)
+		goto out_err;
+
+	return 0;
+
+out_err:
+	dev_err(ddata->dev, "%s failed with %i\n", __func__, error);
+
+	return error;
+}
+
 static int cpcap_usb_init_optional_pins(struct cpcap_phy_ddata *ddata)
 {
 	ddata->pins = devm_pinctrl_get(ddata->dev);
@@ -546,9 +637,6 @@ static int cpcap_usb_init_optional_pins(struct cpcap_phy_ddata *ddata)
 		ddata->pins_uart = NULL;
 	}
 
-	if (ddata->pins_uart)
-		return pinctrl_select_state(ddata->pins, ddata->pins_uart);
-
 	return 0;
 }
 
@@ -601,6 +689,65 @@ static int cpcap_usb_init_iio(struct cpcap_phy_ddata *ddata)
 	return error;
 }
 
+static int cpcap_usb_init_usb(struct cpcap_phy_ddata *ddata)
+{
+	struct cpcap_usb_ints_state s;
+	int error;
+	int mask;
+
+	error = cpcap_phy_get_ints_state(ddata, &s);
+	if (error)
+		return error;
+
+	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_UNKNOWN_DISABLED);
+	if (error)
+		return error;
+
+	if (ddata->pins_ulpi) {
+		error = pinctrl_select_state(ddata->pins, ddata->pins_ulpi);
+		if (error)
+			return error;
+	}
+
+	/*
+	 * Do not touch DM/DP PD as this will break charger detection if charger
+	 * is connected before we detect it.
+	 */
+	if (s.se1) {
+		mask = CPCAP_BIT_VBUSEN_SPI | CPCAP_BIT_VBUSPU_SPI |
+		       CPCAP_BIT_SUSPEND_SPI | CPCAP_BIT_ULPI_SPI_SEL;
+	} else {
+		mask = CPCAP_BIT_VBUSEN_SPI | CPCAP_BIT_VBUSPU_SPI |
+		       CPCAP_BIT_DMPD_SPI | CPCAP_BIT_DPPD_SPI |
+		       CPCAP_BIT_SUSPEND_SPI | CPCAP_BIT_PU_SPI |
+		       CPCAP_BIT_ULPI_SPI_SEL;
+	}
+
+	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3, mask,
+				   CPCAP_BIT_ULPI_SPI_SEL);
+	if (error)
+		return error;
+
+	/*
+	 * Configure safe DP/DM termination state.
+	 *
+	 * Vendor kernel uses:
+	 *   DP150KPU = enabled
+	 *   all USB pullups/pulldowns = disabled
+	 */
+	error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC1,
+				   CPCAP_BIT_DP150KPU |
+				   CPCAP_BIT_DP1K5PU |
+				   CPCAP_BIT_DM1K5PU |
+				   CPCAP_BIT_DPPD |
+				   CPCAP_BIT_DMPD,
+				   CPCAP_BIT_DP150KPU);
+	if (error)
+		return error;
+
+	return cpcap_usb_gpio_set_mode(ddata, CPCAP_DM_DP);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id cpcap_usb_phy_id_table[] = {
 	{
@@ -678,6 +825,10 @@ static int cpcap_usb_phy_probe(struct platform_device *pdev)
 	if (error)
 		goto out_reg_disable;
 
+	error = cpcap_usb_init_usb(ddata);
+	if (error)
+		goto out_reg_disable;
+
 	error = cpcap_usb_init_interrupts(pdev, ddata);
 	if (error)
 		goto out_reg_disable;
@@ -697,13 +848,9 @@ static int cpcap_usb_phy_probe(struct platform_device *pdev)
 static void cpcap_usb_phy_remove(struct platform_device *pdev)
 {
 	struct cpcap_phy_ddata *ddata = platform_get_drvdata(pdev);
-	int error;
 
 	atomic_set(&ddata->active, 0);
-	error = cpcap_usb_set_uart_mode(ddata);
-	if (error)
-		dev_err(ddata->dev, "could not set UART mode\n");
-
+	cpcap_usb_set_idle_mode(ddata);
 	cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_OFF);
 
 	usb_remove_phy(&ddata->phy);
-- 
2.25.1

Re: [PATCH 2/5] phy: cpcap-usb: add DCP detection and make UART idle mode optional

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] Renaming the expected IRQ name from 'se1' to 'chrg_det' breaks compatibility with existing Device Tree binaries.
- [Medium] Early update of `ddata->mode` causes the state machine to become permanently stuck if the subsequent hardware configuration fails.
- [Medium] The system's SysRq functionality is permanently left disabled globally if a hardware configuration step fails in `cpcap_usb_set_idle_mode()`.
- [Medium] The module parameter `enable_uart` is ignored on boot, leaving the PHY in a partially uninitialized state.
- [Medium] A Time-of-Check to Time-of-Use (TOCTOU) race condition exists when reading the global `cpcap_enable_uart` module parameter.

Pre-existing issues:
- [Critical] A race condition during driver removal allows the devm-managed threaded IRQ handler to schedule `detect_work` after it has been canceled, leading to a Use-After-Free.
--

commit e8047562da705a40f07fe646c6694386f4dad156
Author: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

phy: cpcap-usb: add DCP detection and make UART idle mode optional
    
Handle DCP separately from USB host connections based on CPCAP charger
detection status.
    
Make the existing idle UART mode optional via the "enable_uart" module
parameter. When disabled, the PHY remains in its USB/charger detection
configuration while idle. This reduces power usage in idle by 25mW.
    
Also initialize the PHY into the baseline configuration required for
reliable charger detection during probe.

> diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
> index c6cc8fce6940f..3be2b3afaaf4d 100644
> --- a/drivers/phy/motorola/phy-cpcap-usb.c
> +++ b/drivers/phy/motorola/phy-cpcap-usb.c
> @@ -296,36 +309,53 @@ static void cpcap_usb_detect(struct work_struct *work)
>  		return;
>  	}
>  
>  	vbus = cpcap_usb_vbus_valid(ddata);
>  
> -	/* 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 */
> +	if (vbus && ddata->mode != CPCAP_USB_HOST) {
> +		if (!s.chrg_det || !s.sessvld || ddata->mode != CPCAP_IDLE)
> +			return;
> +
> +		if (s.se1)
> +			ddata->mode = CPCAP_CHARGER;
> +		else if (!s.dm && !s.dp)
> +			ddata->mode = CPCAP_USB;
> +		else
> +			return;

[Severity: Medium]
Can the state machine become permanently stuck if the subsequent hardware
configuration fails?

The state variable ddata->mode is eagerly updated here to CPCAP_CHARGER or
CPCAP_USB before applying the hardware state via cpcap_usb_set_usb_mode().
If the configuration fails and returns early, the state is never reverted.

On the next evaluation, the condition checking ddata->mode != CPCAP_IDLE
will evaluate to true and return immediately, preventing recovery until the
cable is physically unplugged.

> +
> +		dev_dbg(ddata->dev, "connected to %s\n",
> +			s.se1 ? "DCP" : "USB host");
> +
> +		error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC3,
> +					   CPCAP_BIT_VBUSSTBY_EN |
> +					   CPCAP_BIT_VBUSEN_SPI, 0);
>  		if (error)
>  			goto out_err;
> -		cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_VALID);
> +
> +		if (ddata->mode == CPCAP_CHARGER)
> +			error = cpcap_usb_set_dcp_mode(ddata);
> +		else
> +			error = cpcap_usb_set_usb_mode(ddata);
> +
> +		if (error)
> +			goto out_err;
> +
> +		if (ddata->mode == CPCAP_USB)
> +			cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_VALID);
>  
>  		return;
>  	}
>  
> -	ddata->vbus_provider = false;
> -	ddata->docked = false;
> +	if (ddata->mode == CPCAP_IDLE)
> +		return;

[Severity: Medium]
Does this early return prevent the enable_uart module parameter from being
applied on boot?

During initial probe, ddata->mode is zero-initialized to CPCAP_IDLE. When
cpcap_usb_detect() runs for the first time, it exits early here and completely
skips cpcap_usb_set_idle_mode(). This leaves the PHY in a partially
uninitialized state until a cable is attached and removed.

> +
> +	ddata->mode = CPCAP_IDLE;
>  	cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_OFF);
>  
> -	/* Default to debug UART mode */
> -	error = cpcap_usb_set_uart_mode(ddata);
> +	error = cpcap_usb_set_idle_mode(ddata);
>  	if (error)
>  		goto out_err;

[ ... ]

> @@ -374,7 +404,7 @@ static const char * const cpcap_phy_irqs[] = {
>  	"id_ground", "id_float",
>  
>  	/* REG_INT1 */
> -	"se0conn", "vbusvld", "sessvld", "sessend", "se1",
> +	"se0conn", "vbusvld", "sessvld", "sessend", "chrg_det",

[Severity: High]
Does this break compatibility with existing Device Tree binaries?

The driver now strictly requests the "chrg_det" interrupt by name, but older
DTBs only provide "se1". Without a fallback to check for the old name,
cpcap_usb_init_irq() will return -ENODEV and abort the probe.

[ ... ]

> @@ -426,30 +457,52 @@ static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
>  	if (error)
>  		goto out_err;
>  
> -	if (ddata->pins_uart) {
> -		error = pinctrl_select_state(ddata->pins, ddata->pins_uart);
> +	if (cpcap_enable_uart && ddata->pins_uart) {

[Severity: Medium]
Can a race condition occur when reading the global cpcap_enable_uart module
parameter?

Since it is modifiable via sysfs and evaluated multiple times independently
within cpcap_usb_set_idle_mode(), a concurrent write could change its value
mid-execution. This might result in a mismatched configuration, such as
setting UART pinctrl but then improperly applying SUSPEND_SPI later in the
function.

> +		error = pinctrl_select_state(ddata->pins,  ddata->pins_uart);
> +		if (error)
> +			goto out_err;
> +	} else if (!cpcap_enable_uart && ddata->pins_ulpi) {
> +		error = pinctrl_select_state(ddata->pins, ddata->pins_ulpi);
>  		if (error)
>  			goto out_err;
>  	}

[ ... ]

> @@ -457,6 +510,8 @@ static int cpcap_usb_set_uart_mode(struct cpcap_phy_ddata *ddata)
>  	if (sysrq & 1)
>  		sysrq_toggle_support(sysrq);
>  
> +	dev_dbg(ddata->dev, "set %s mode\n", cpcap_enable_uart ? "UART" : "IDLE");
> +
>  	return 0;
>  
>  out_err:
> +	dev_err(ddata->dev, "%s failed with %i\n", __func__, error);
> +
> +	return error;
> +}

[Severity: Medium]
Is it possible for the system's SysRq functionality to be permanently left
disabled on an error path?

In cpcap_usb_set_idle_mode(), sysrq is disabled to prevent line glitches.
If any subsequent hardware configuration step fails, the function jumps to the
out_err label here and returns without restoring the original sysrq state via
sysrq_toggle_support().

[ ... ]

> @@ -697,13 +848,9 @@ static int cpcap_usb_phy_probe(struct platform_device *pdev)
>  static void cpcap_usb_phy_remove(struct platform_device *pdev)
>  {
>  	struct cpcap_phy_ddata *ddata = platform_get_drvdata(pdev);
> -	int error;
>  
>  	atomic_set(&ddata->active, 0);
> -	error = cpcap_usb_set_uart_mode(ddata);
> -	if (error)
> -		dev_err(ddata->dev, "could not set UART mode\n");
> -
> +	cpcap_usb_set_idle_mode(ddata);
>  	cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_OFF);
>  
>  	usb_remove_phy(&ddata->phy);

[Severity: Critical]
This is a pre-existing issue, but does a race condition exist during driver
removal that allows the devm-managed threaded IRQ handler to schedule
detect_work after it has been canceled?

Because the IRQ is devm-managed, it is still active when
cancel_delayed_work_sync() is called at the end of cpcap_usb_phy_remove().

If the IRQ thread in cpcap_phy_irq_thread() evaluates ddata->active != 0
before it is cleared, but is preempted before calling schedule_delayed_work(),
the remove function can finish and cancel the work.

Devres will then wait for the preempted IRQ thread to finish. The thread will
resume and schedule the delayed work, and devres will then free ddata. When
the timer expires, the workqueue will execute cpcap_usb_detect() using the
freed ddata.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704073843.1750458-1-ivo.g.dimitrov.75@gmail.com?part=2

[PATCH 3/5] dt-bindings: phy: motorola,cpcap-usb: replace se1 interrupt with chrg_det

[Ivaylo Dimitrov]

The CPCAP USB PHY driver uses the CPCAP charger detection interrupt
for DCP detection rather than the SE1 interrupt.

Update the binding and example DTS to use the corresponding
"chrg_det" interrupt name.arch: arm: dts: cpcap-mapphone: xxx

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 .../devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml   | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml b/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml
index 0febd04a61f4..81c5f3b5997b 100644
--- a/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml
@@ -27,7 +27,7 @@ properties:
       - description: vbusvld interrupt
       - description: sessvld interrupt
       - description: sessend interrupt
-      - description: se1 interrupt
+      - description: charger detection interrupt
       - description: dm interrupt
       - description: dp interrupt
 
@@ -40,7 +40,7 @@ properties:
       - const: vbusvld
       - const: sessvld
       - const: sessend
-      - const: se1
+      - const: chrg_det
       - const: dm
       - const: dp
 
@@ -90,11 +90,11 @@ examples:
         #phy-cells = <0>;
         interrupts-extended = <
             &cpcap 15 0 &cpcap 14 0 &cpcap 28 0 &cpcap 19 0
-            &cpcap 18 0 &cpcap 17 0 &cpcap 16 0 &cpcap 49 0
+            &cpcap 18 0 &cpcap 17 0 &cpcap 13 0 &cpcap 49 0
             &cpcap 48 1
         >;
         interrupt-names = "id_ground", "id_float", "se0conn", "vbusvld",
-                          "sessvld", "sessend", "se1", "dm", "dp";
+                          "sessvld", "sessend", "chrg_det", "dm", "dp";
         io-channels = <&cpcap_adc 2>, <&cpcap_adc 7>;
         io-channel-names = "vbus", "id";
         vusb-supply = <&vusb>;
-- 
2.25.1

Re: [PATCH 3/5] dt-bindings: phy: motorola,cpcap-usb: replace se1 interrupt with chrg_det

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Critical] Device Tree ABI breakage: replacing 'se1' interrupt with 'chrg_det' breaks probe on older DTBs.
- [Medium] Global SysRq state is permanently disabled on error paths.

Pre-existing issues:
- [High] Race condition in driver removal allows the threaded IRQ handler to schedule work on freed memory (Use-After-Free).
--

commit d7b2a51c3f82ebdb2f326313f13ddf2d3edde1a7
Author: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

dt-bindings: phy: motorola,cpcap-usb: replace se1 interrupt with chrg_det

The CPCAP USB PHY driver uses the CPCAP charger detection interrupt for DCP
detection rather than the SE1 interrupt. This commit updates the binding and
example DTS to use the corresponding "chrg_det" interrupt name.

> diff --git a/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml b/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml
> index dd345cbd0a0b4..d54cf7fbf11a7 100644
> --- a/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml
> @@ -27,7 +27,7 @@ properties:
>        - description: vbusvld interrupt
>        - description: sessvld interrupt
>        - description: sessend interrupt
> -      - description: se1 interrupt
> +      - description: charger detection interrupt
>        - description: dm interrupt
>        - description: dp interrupt
>  
> @@ -40,7 +40,7 @@ properties:
>        - const: vbusvld
>        - const: sessvld
>        - const: sessend
> -      - const: se1
> +      - const: chrg_det

[Severity: Critical]
Does replacing 'se1' with 'chrg_det' break probe for older device trees?

Since cpcap_usb_init_irq() in drivers/phy/motorola/phy-cpcap-usb.c fails the
probe if an expected interrupt is not found, updating the binding to require
'chrg_det' instead of 'se1' means booting a new kernel with an older DTB
(which only provides 'se1') could fail to probe the USB PHY.

>        - const: dm
>        - const: dp
>  

[ ... ]

While reviewing this, I noticed a few other regressions in
drivers/phy/motorola/phy-cpcap-usb.c:

[Severity: Medium]
Can this error path in cpcap_usb_set_idle_mode() leave the global SysRq state
permanently disabled?

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_set_idle_mode() {
    ...
    sysrq = sysrq_mask();
    if (sysrq & 1)
        sysrq_toggle_support(sysrq & ~1);
    ...
out_err:
    dev_err(ddata->dev, "%s failed with %i\n", __func__, error);

    return error;
}

If an error occurs (e.g., during pinctrl_select_state), the function jumps to
out_err and returns without calling sysrq_toggle_support(sysrq) to restore it.


[Severity: High]
This is a pre-existing issue, but could there be a race condition during
driver removal that allows the threaded IRQ handler to schedule work on freed
memory?

During driver removal, cpcap_usb_phy_remove() cancels the work without
disabling the IRQ first:

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_phy_remove() {
    ...
    atomic_set(&ddata->active, 0);
    ...
    cancel_delayed_work_sync(&ddata->detect_work);
    regulator_disable(ddata->vusb);
}

Concurrently, if an IRQ fires and cpcap_phy_irq_thread() checks 'active' but
is preempted before scheduling the work:

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_phy_irq_thread() {
    ...
    if (!atomic_read(&ddata->active))
        return IRQ_NONE;

    schedule_delayed_work(&ddata->detect_work, msecs_to_jiffies(1));
    ...
}

Could the IRQ thread resume after cancel_delayed_work_sync() has completed,
scheduling the delayed work on a freed 'ddata' object?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704073843.1750458-1-ivo.g.dimitrov.75@gmail.com?part=3

[PATCH 4/5] ARM: dts: ti: cpcap-mapphone: use charger detection interrupt for CPCAP USB PHY

[Ivaylo Dimitrov]

Update the CPCAP USB PHY interrupt mapping to use the charger
detection interrupt and corresponding "chrg_det" interrupt name.

This matches the driver and binding updates for DCP detection.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
index 6b1195c2a768..b75a327caf1f 100644
--- a/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
+++ b/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
@@ -125,11 +125,11 @@ cpcap_usb2_phy: phy {
 			interrupts-extended =
 				<&cpcap 15 0>, <&cpcap 14 0>, <&cpcap 28 0>,
 				<&cpcap 19 0>, <&cpcap 18 0>, <&cpcap 17 0>,
-				<&cpcap 16 0>, <&cpcap 49 0>, <&cpcap 48 0>;
+				<&cpcap 13 0>, <&cpcap 49 0>, <&cpcap 48 0>;
 			interrupt-names =
 				"id_ground", "id_float", "se0conn",
 				"vbusvld", "sessvld", "sessend",
-				"se1", "dm", "dp";
+				"chrg_det", "dm", "dp";
 			mode-gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>,
 				     <&gpio1 0 GPIO_ACTIVE_HIGH>;
 			io-channels = <&cpcap_adc 2>, <&cpcap_adc 7>;
-- 
2.25.1

Re: [PATCH 4/5] ARM: dts: ti: cpcap-mapphone: use charger detection interrupt for CPCAP USB PHY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Backward Compatibility Breakage in DT interrupt name change
- [High] Missing Cleanup / Error Handling causes permanent sysrq disablement
- [Medium] Data Race on `cpcap_enable_uart` module parameter leads to mixed hardware state
--

commit 657e07c7931cc99eefeab1abc1d21d20f3271aa2
Author: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

ARM: dts: ti: cpcap-mapphone: use charger detection interrupt for CPCAP USB PHY

This commit updates the CPCAP USB PHY interrupt mapping in the device tree
to use the charger detection interrupt. It replaces the "se1" interrupt name
with "chrg_det", which matches driver and binding updates for DCP detection.

> diff --git a/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
> index 73e27b95105e1..f3ec85d73a16f 100644
> --- a/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
[ ... ]
> @@ -106,11 +106,11 @@ cpcap_usb2_phy: phy {
>  			interrupts-extended =
>  				<&cpcap 15 0>, <&cpcap 14 0>, <&cpcap 28 0>,
>  				<&cpcap 19 0>, <&cpcap 18 0>, <&cpcap 17 0>,
> -				<&cpcap 16 0>, <&cpcap 49 0>, <&cpcap 48 0>;
> +				<&cpcap 13 0>, <&cpcap 49 0>, <&cpcap 48 0>;
>  			interrupt-names =
>  				"id_ground", "id_float", "se0conn",
>  				"vbusvld", "sessvld", "sessend",
> -				"se1", "dm", "dp";
> +				"chrg_det", "dm", "dp";

[Severity: High]
Does replacing the "se1" interrupt name outright instead of appending it
break backward compatibility?

If a new kernel boots with an older device tree (which provides "se1"
instead of "chrg_det"), probe aborts. The driver strictly requires all
interrupts to be present:

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_phy_irqs:
	/* REG_INT1 */
	"se0conn", "vbusvld", "sessvld", "sessend", "chrg_det",

And in cpcap_usb_init_irq():
	irq = platform_get_irq_byname(pdev, name);
	if (irq < 0)
		return -ENODEV;

Probe is aborted if "chrg_det" is not found, without attempting to fall
back to "se1".

[Severity: High]
Can a transient error during PHY mode switching in the driver permanently
disable sysrq?

In drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_set_idle_mode(), sysrq is
disabled at the start:

	sysrq = sysrq_mask();
	if (sysrq & 1)
		sysrq_toggle_support(sysrq & ~1);

	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_UNKNOWN_DISABLED);
	if (error)
		goto out_err;

If hardware configuration fails, the function skips the restoration block:

	if (sysrq & 1)
		sysrq_toggle_support(sysrq);

	return 0;

out_err:
	dev_err(ddata->dev, "%s failed with %i\n", __func__, error);
	return error;

Does this silently strip the system of its diagnostic capabilities on a
single I2C/SPI bus error?

[Severity: Medium]
Could a concurrent modification to the cpcap_enable_uart module parameter
cause a mixed hardware state?

In drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_set_idle_mode(), the global
cpcap_enable_uart is queried independently multiple times:

	if (cpcap_enable_uart && ddata->pins_uart) {
		error = pinctrl_select_state(ddata->pins,  ddata->pins_uart);
...
	if (cpcap_enable_uart)
		val |= (CPCAP_BIT_UARTMUX0 | CPCAP_BIT_EMUMODE0);
...
	if (!cpcap_enable_uart)
		val |= CPCAP_BIT_SUSPEND_SPI;

If modified during execution, could this read true during the pinctrl check
and false during the regmap bitmask construction, leaving the hardware in an
inconsistent configuration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704073843.1750458-1-ivo.g.dimitrov.75@gmail.com?part=4

[PATCH 5/5] phy: cpcap-usb: add extcon support

[Ivaylo Dimitrov]

Register an Extcon device and report the detected cable state.

The driver already determines the type of cable attached during USB cable
detection. Export the detected state through the Extcon framework so
other drivers can consume it using a standard kernel interface.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/phy/motorola/phy-cpcap-usb.c | 68 ++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
index 3be2b3afaaf4..8978a95c1c19 100644
--- a/drivers/phy/motorola/phy-cpcap-usb.c
+++ b/drivers/phy/motorola/phy-cpcap-usb.c
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
+#include <linux/extcon-provider.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -123,6 +124,7 @@ struct cpcap_phy_ddata {
 	struct regmap *reg;
 	struct device *dev;
 	struct usb_phy phy;
+	struct extcon_dev *edev;
 	struct delayed_work detect_work;
 	struct pinctrl *pins;
 	struct pinctrl_state *pins_ulpi;
@@ -134,12 +136,23 @@ struct cpcap_phy_ddata {
 	struct regulator *vusb;
 	atomic_t active;
 	enum cpcap_mode mode;
+	int cable;
 };
 
 static bool cpcap_enable_uart;
 module_param_named(enable_uart, cpcap_enable_uart, bool, 0644);
 MODULE_PARM_DESC(enable_uart,
 		 "Enable UART on the USB connector while idle (increases power consumption)");
+
+/* List of detectable cables */
+static const unsigned int cpcap_extcon_cables[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+	EXTCON_CHG_USB_SDP,
+	EXTCON_CHG_USB_DCP,
+	EXTCON_NONE,
+};
+
 static bool cpcap_usb_vbus_valid(struct cpcap_phy_ddata *ddata)
 {
 	int error, value = 0;
@@ -226,6 +239,43 @@ static void cpcap_usb_try_musb_mailbox(struct cpcap_phy_ddata *ddata,
 		__func__, error);
 }
 
+static void cpcap_usb_set_extcon(struct cpcap_phy_ddata *ddata)
+{
+	int cable;
+
+	switch (ddata->mode) {
+	case CPCAP_CHARGER:
+		cable = EXTCON_CHG_USB_DCP;
+		break;
+	case CPCAP_USB:
+		cable = EXTCON_CHG_USB_SDP;
+		break;
+	case CPCAP_USB_HOST:
+	case CPCAP_DOCK:
+		cable = EXTCON_USB_HOST;
+		break;
+	case CPCAP_IDLE:
+		cable = EXTCON_NONE;
+		break;
+	}
+
+	if (cable == ddata->cable)
+		return;
+
+	if (ddata->cable == EXTCON_CHG_USB_SDP)
+		extcon_set_state(ddata->edev, EXTCON_USB, false);
+	else if (cable == EXTCON_CHG_USB_SDP)
+		extcon_set_state(ddata->edev, EXTCON_USB, true);
+
+	if (ddata->cable != EXTCON_NONE)
+		extcon_set_state_sync(ddata->edev, ddata->cable, false);
+
+	if (cable != EXTCON_NONE)
+		extcon_set_state_sync(ddata->edev, cable, true);
+
+	ddata->cable = cable;
+}
+
 static void cpcap_usb_detect(struct work_struct *work)
 {
 	struct cpcap_phy_ddata *ddata;
@@ -276,6 +326,7 @@ static void cpcap_usb_detect(struct work_struct *work)
 			goto out_err;
 
 		cpcap_usb_try_musb_mailbox(ddata, MUSB_ID_GROUND);
+		cpcap_usb_set_extcon(ddata);
 
 		/*
 		 * Force check state again after musb has reoriented,
@@ -306,6 +357,8 @@ static void cpcap_usb_detect(struct work_struct *work)
 		if (error)
 			goto out_err;
 
+		cpcap_usb_set_extcon(ddata);
+
 		return;
 	}
 
@@ -343,6 +396,8 @@ static void cpcap_usb_detect(struct work_struct *work)
 		if (ddata->mode == CPCAP_USB)
 			cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_VALID);
 
+		cpcap_usb_set_extcon(ddata);
+
 		return;
 	}
 
@@ -356,6 +411,8 @@ static void cpcap_usb_detect(struct work_struct *work)
 	if (error)
 		goto out_err;
 
+	cpcap_usb_set_extcon(ddata);
+
 	return;
 
 out_err:
@@ -781,11 +838,22 @@ static int cpcap_usb_phy_probe(struct platform_device *pdev)
 	if (!otg)
 		return -ENOMEM;
 
+	ddata->edev = devm_extcon_dev_allocate(&pdev->dev, cpcap_extcon_cables);
+	if (IS_ERR(ddata->edev)) {
+		return dev_err_probe(&pdev->dev, PTR_ERR(ddata->edev),
+				     "failed to allocate extcon device\n");
+	}
+
+	error = devm_extcon_dev_register(&pdev->dev, ddata->edev);
+	if (error < 0)
+		return error;
+
 	ddata->dev = &pdev->dev;
 	ddata->phy.dev = ddata->dev;
 	ddata->phy.label = "cpcap_usb_phy";
 	ddata->phy.otg = otg;
 	ddata->phy.type = USB_PHY_TYPE_USB2;
+	ddata->cable = EXTCON_NONE;
 	otg->set_host = cpcap_usb_phy_set_host;
 	otg->set_peripheral = cpcap_usb_phy_set_peripheral;
 	otg->usb_phy = &ddata->phy;
-- 
2.25.1

Re: [PATCH 5/5] phy: cpcap-usb: add extcon support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The EXTCON_USB state is updated using `extcon_set_state` without a corresponding `extcon_set_state_sync` or `extcon_sync`, preventing registered listeners from receiving state change notifications.
- [Medium] Extcon state can become permanently desynchronized from the hardware/software state when regmap updates fail.

Pre-existing issues:
- [High] The driver's remove path races with the devres-managed threaded IRQ handler, leading to a Use-After-Free of the driver data structure.
--

commit 5e21d209058ee93304da0e1964b84339cf8709c2
Author: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

phy: cpcap-usb: add extcon support

Register an Extcon device and report the detected cable state.
The driver already determines the type of cable attached during USB cable
detection. Export the detected state through the Extcon framework so
other drivers can consume it using a standard kernel interface.

> diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
> index 3be2b3afaaf4d..8978a95c1c199 100644
> --- a/drivers/phy/motorola/phy-cpcap-usb.c
> +++ b/drivers/phy/motorola/phy-cpcap-usb.c
> @@ -123,6 +124,7 @@ struct cpcap_phy_ddata {
>  	struct regmap *reg;
>  	struct device *dev;
>  	struct usb_phy phy;
> +	struct extcon_dev *edev;
>  	struct delayed_work detect_work;

[Severity: High]
This is a pre-existing issue, but does the remove path race with the
devres-managed threaded IRQ handler, leading to a use-after-free of this
driver data structure?

If the devres-managed IRQ thread cpcap_phy_irq_thread() fires and reads
ddata->active as 1, but is preempted before calling schedule_delayed_work():

And then the driver remove path cpcap_usb_phy_remove() executes, sets
ddata->active to 0, cancels any pending work, and returns.

Devres cleanup then begins, calling devm_irq_release() -> free_irq() ->
synchronize_irq(), which blocks waiting for the preempted IRQ thread.

When the IRQ thread resumes, it successfully queues the delayed work and
returns.

Once devm_irq_release() finishes, devres proceeds to free ddata.
The delayed work timer expires and executes cpcap_usb_detect() on the freed
ddata memory.

> [ ... ]
> @@ -226,6 +239,43 @@ static void cpcap_usb_try_musb_mailbox(struct cpcap_phy_ddata *ddata,
> [ ... ]
> +static void cpcap_usb_set_extcon(struct cpcap_phy_ddata *ddata)
> +{
> [ ... ]
> +	if (ddata->cable == EXTCON_CHG_USB_SDP)
> +		extcon_set_state(ddata->edev, EXTCON_USB, false);
> +	else if (cable == EXTCON_CHG_USB_SDP)
> +		extcon_set_state(ddata->edev, EXTCON_USB, true);

[Severity: High]
Does this prevent registered listeners from receiving state change
notifications for EXTCON_USB?

The EXTCON_USB state is updated here using extcon_set_state() without a
corresponding extcon_set_state_sync() or extcon_sync(). When the driver
changes the state to or from CPCAP_USB (which maps to EXTCON_CHG_USB_SDP),
the subsequent extcon_set_state_sync() calls only target the charger cable
ID and its specific notifier chain.

The notifier chain for EXTCON_USB might never be fired.

> +
> +	if (ddata->cable != EXTCON_NONE)
> +		extcon_set_state_sync(ddata->edev, ddata->cable, false);
> +
> +	if (cable != EXTCON_NONE)
> +		extcon_set_state_sync(ddata->edev, cable, true);
> +
> +	ddata->cable = cable;
> +}
> [ ... ]
> @@ -356,6 +411,8 @@ static void cpcap_usb_detect(struct work_struct *work)
> [ ... ]
>  	if (ddata->mode == CPCAP_IDLE)
>  		return;
>  
>  	ddata->mode = CPCAP_IDLE;
>  	cpcap_usb_try_musb_mailbox(ddata, MUSB_VBUS_OFF);
>  
>  	error = cpcap_usb_set_idle_mode(ddata);
>  	if (error)
>  		goto out_err;
>  
> +	cpcap_usb_set_extcon(ddata);
> +
>  	return;
>  
>  out_err:

[Severity: Medium]
Can the extcon state become permanently desynchronized from the hardware and
software state if a regmap update fails?

Earlier in cpcap_usb_detect(), the internal software state ddata->mode is
updated prior to hardware configuration. If the hardware register update
fails, the code branches to out_err and skips calling
cpcap_usb_set_extcon().

Because ddata->mode is already modified, subsequent executions of
cpcap_usb_detect() will take the early return path and will never retry
updating the extcon state, leaving it stale indefinitely.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704073843.1750458-1-ivo.g.dimitrov.75@gmail.com?part=5