Re: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional

Re: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional

[sashiko-bot]

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 <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 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.

> 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

[ ... ]

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

[ ... ]

> @@ -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;

[ ... ]

> @@ -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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 1/7] dt-bindings: phy: motorola,cpcap-usb: add chrg_det interrupt

[Rob Herring (Arm)]

On Sat, 11 Jul 2026 23:42:04 +0300, Ivaylo Dimitrov wrote:
> Document the optional CPCAP charger detection interrupt in the USB PHY
> binding.
> 
> Update the example DTS to include the corresponding "chrg_det" interrupt
> name.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  .../devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml   | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/7] dt-bindings: phy: motorola,cpcap-usb-phy: add optional safe pinctrl state

[Rob Herring (Arm)]

On Sat, 11 Jul 2026 23:42:05 +0300, Ivaylo Dimitrov wrote:
> Document the optional "safe" pinctrl state for the CPCAP USB PHY.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  .../devicetree/bindings/phy/motorola,cpcap-usb-phy.yaml      | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Manivannan Sadhasivam]

On Sat, Jul 11, 2026 at 11:42:03PM +0300, Ivaylo Dimitrov wrote:
> 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 extends the CPCAP USB PHY driver to detect DCP connections
> and export the detected cable state through the Extcon framework. 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 updates the Device Tree binding for the optional charger
> detection interrupt and the optional "safe" pinctrl state. Corresponding
> mapphone Device Tree entries are added to describe the charger detection
> interrupt and enable the safe pinctrl state.
> 
> The charger detection interrupt is not currently used by the driver.
> However, it is added to the binding and Device Tree because it describes
> a hardware capability of the CPCAP USB PHY.
> 
> The series has been tested on Motorola Droid 4 hardware.
> 

Due to mangled CC "--cc=linux-phy@lists.infradead.org" PHY list was not CCed to
this series. Please fix it in the next version.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/7] dt-bindings: phy: motorola,cpcap-usb-phy: add optional safe pinctrl state

[Manivannan Sadhasivam]

On Sat, Jul 11, 2026 at 11:42:05PM +0300, Ivaylo Dimitrov wrote:
> Document the optional "safe" pinctrl state for the CPCAP USB PHY.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 1/7] dt-bindings: phy: motorola,cpcap-usb: add chrg_det interrupt

[Manivannan Sadhasivam]

On Sat, Jul 11, 2026 at 11:42:04PM +0300, Ivaylo Dimitrov wrote:
> Document the optional CPCAP charger detection interrupt in the USB PHY
> binding.
> 
> Update the example DTS to include the corresponding "chrg_det" interrupt
> name.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 3/7] phy: cpcap-usb: fix IRQ teardown race

[Manivannan Sadhasivam]

On Sat, Jul 11, 2026 at 11:42:06PM +0300, Ivaylo Dimitrov wrote:
> there is a race between IRQ handler and remove:
> 
> IRQ thread:
>     ddata->active == 1
> 
> remove():
>     ddata->active = 0
>     cancel_delayed_work_sync()
> 
> IRQ thread:
>     schedule_delayed_work()
> 
> The IRQ handler can therefore queue detect work after it has been canceled
> during remove(). Free the IRQ handlers before canceling detect work to
> prevent new work from being scheduled during teardown.
> 
> Reported by Sashiko

Add a proper tag.

> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  drivers/phy/motorola/phy-cpcap-usb.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/motorola/phy-cpcap-usb.c b/drivers/phy/motorola/phy-cpcap-usb.c
> index 7cb020dd3423..741145c89e5b 100644
> --- a/drivers/phy/motorola/phy-cpcap-usb.c
> +++ b/drivers/phy/motorola/phy-cpcap-usb.c
> @@ -393,6 +393,19 @@ static int cpcap_usb_init_interrupts(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static void cpcap_usb_fini_interrupts(struct platform_device *pdev,

s/fini/free

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional

[Manivannan Sadhasivam]

On Sat, Jul 11, 2026 at 11:42:07PM +0300, Ivaylo Dimitrov wrote:
> 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.

> Note: Enabling UART idle mode increases idle power consumption (by 25mW
> on droid4).
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> 
> # Conflicts:
> #	drivers/phy/motorola/phy-cpcap-usb.c

What is this conflict?

> ---
>  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.

>  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().

>  				   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.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Manivannan Sadhasivam]

On Sat, Jul 11, 2026 at 11:42:08PM +0300, Ivaylo Dimitrov wrote:
> 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>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

- Mani

-- 
மணிவண்ணன் சதாசிவம்

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 4/7] phy: cpcap-usb: add DCP detection and make UART idle mode optional

[Ivaylo Dimitrov]

On 9.09.26 г. 19:07 ч., Manivannan Sadhasivam wrote:
> On Sat, Jul 11, 2026 at 11:42:07PM +0300, Ivaylo Dimitrov wrote:
>> 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.
> 
will spliting in two:

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@gmail.com>
>>
>> # 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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy