Re: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional
From: Ivaylo Dimitrov
Date: Sun Sep 13 2026 - 13:07:08 EST
On 9.09.26 г. 19:07 ч., Manivannan Sadhasivam wrote:
On Sat, Jul 11, 2026 at 11:42:07PM +0300, Ivaylo Dimitrov wrote:will spliting in two:
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. When disabled (default), the PHY remains in its USB/charger
detection configuration while idle.
Also initialize the PHY into the baseline configuration required for
reliable charger detection during probe.
Use the optional "safe" pinctrl state before switching between modes to
avoid glitches on USB or UART lines.
Looks like this change is doing multiple things at once. Please split the
changes logically to separate patches.
patch1: enable_uart + safe pinctrl
patch2: DCP detection + init on probe
be ok or you want me to split even more? To me it makes sense as enable_uart will be few lines only if sent as a separate patch and I don't think splitting DCP detection + init on probe makes sense.
Note: Enabling UART idle mode increases idle power consumption (by 25mW
on droid4).
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@xxxxxxxxx>
# Conflicts:
# drivers/phy/motorola/phy-cpcap-usb.c
What is this conflict?
an artefact from nth local rebase/merge before submission :) .
---
drivers/phy/motorola/phy-cpcap-usb.c | 301 +++++++++++++++++++++------
1 file changed, 238 insertions(+), 63 deletions(-)
diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
index 741145c89e5b..2d770ff19e93 100644
--- a/drivers/phy/motorola/phy-cpcap-usb.c
+++ b/drivers/phy/motorola/phy-cpcap-usb.c
@@ -110,6 +110,15 @@ enum cpcap_gpio_mode {
CPCAP_OTG_DM_DP,
};
+enum cpcap_mode {
+ CPCAP_UNKNOWN,
+ CPCAP_IDLE,
+ CPCAP_CHARGER,
+ CPCAP_USB,
+ CPCAP_USB_HOST,
+ CPCAP_DOCK,
+};
+
struct cpcap_phy_ddata {
struct regmap *reg;
struct device *dev;
@@ -119,15 +128,19 @@ struct cpcap_phy_ddata {
struct pinctrl_state *pins_ulpi;
struct pinctrl_state *pins_utmi;
struct pinctrl_state *pins_uart;
+ struct pinctrl_state *pins_safe;
struct gpio_desc *gpio[2];
struct iio_channel *vbus;
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)");
Use of module params is discouraged these days. Also, you are disabling it by
default, which could cause surprises to users who have boards wired up for debug
console. But considering that it consumes a lot of power, I think it is OK to
disable it this way. I can't think of another way to add this knob.
Me neither, that's why I came up with a module parameter. Yes, I understand disabling it by default may cause regression for some (presumably knowledgeable) users, however, I think stripping ~25% from idle power usage for the others worths it.
static bool cpcap_usb_vbus_valid(struct cpcap_phy_ddata *ddata)
{
int error, value = 0;
@@ -196,8 +209,9 @@ static int cpcap_phy_get_ints_state(struct cpcap_phy_ddata *ddata,
return 0;
}
[...]
@@ -473,21 +559,10 @@ static int cpcap_usb_set_usb_mode(struct cpcap_phy_ddata *ddata)
{
int error;
- /* Disable lines to prevent glitches from waking up mdm6600 */
- error = cpcap_usb_gpio_set_mode(ddata, CPCAP_UNKNOWN_DISABLED);
+ error = cpcap_usb_set_safe_mode(ddata);
if (error)
return error;
- if (ddata->pins_utmi) {
- error = pinctrl_select_state(ddata->pins, ddata->pins_utmi);
- if (error) {
- dev_err(ddata->dev, "could not set usb mode: %i\n",
- error);
-
- return error;
- }
- }
-
error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC1,
CPCAP_BIT_VBUSPD, 0);
if (error)
@@ -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,
As Sashiko noted, you are not clearing CPCAP_BIT_USBSUSPEND bit set in
cpcap_usb_set_idle_mode().
Vendor kernel does not do it and we are using the patch with CPCAP_BIT_USBSUSPEND not cleared for few months with no issues whatsoever, so I am not convinced this is needed. However, tests on the device didn't show any difference if I clear the bit so OK, will do.
CPCAP_BIT_USBXCVREN);
if (error)
goto out_err;
+ if (ddata->pins_utmi) {
+ error = pinctrl_select_state(ddata->pins, ddata->pins_utmi);
+ if (error) {
+ dev_err(ddata->dev, "could not set usb mode: %i\n",
+ error);
+
+ return error;
+ }
+ }
+
/* Enable USB mode */
error = cpcap_usb_gpio_set_mode(ddata, CPCAP_OTG_DM_DP);
if (error)
@@ -521,6 +608,38 @@ 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;
+
+ error = cpcap_usb_set_safe_mode(ddata);
+ if (error)
+ return error;
+
+ error = regmap_update_bits(ddata->reg, CPCAP_REG_USBC2,
+ CPCAP_BIT_USBXCVREN |
+ CPCAP_BIT_UARTMUX0 |
+ CPCAP_BIT_EMUMODE0, 0);
+ if (error)
+ goto out_err;
+
+ 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);
Don't print function names in the error log.
Ok.
Will send new series, just LMK if you want the patch split in 2 or more patches.
Thanks,
Ivo