Re: [PATCH] usb: typec: ucsi: Handle connectors already attached at init

"Christian A. Ehrhardt" <[email protected]>
Newsgroups org.kernel.vger.linux-usb,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hi William,

On Thu, Aug 13, 2026 at 10:38:40AM +0100, William Bright wrote:
> After a cold boot the PPM may already have a partner attached without a
> pending connector change to report. On a CYPD6125 the CCI register reads
> back 0 in ucsi_init(), so ucsi_connector_change() is never called and the
> Type-C mux is left unconfigured for the already-attached port.
> 
> Fix this by additionally checking the connector status of every
> connector and call ucsi_connector_change() for connectors that are
> already attached. Whilst also continuing to call ucsi_connector_change()
> for connectors that report a change via the CCI register.

I'm not sure if this is the right thing to do.

First, there is code in ucsi_register_port() that is supposed to
handle the case where a connection is already present. I wonder
why that does not work for you?

> Fixes: ce4c8d21054a ("usb: typec: ucsi: Fix connector check on init")
> Signed-off-by: William Bright <[email protected]>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index ecbda2a3783c..740ae4a14f7f 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -2136,8 +2136,11 @@ static int ucsi_init(struct ucsi *ucsi)
>  	mutex_unlock(&ucsi->ppm_lock);
>  	if (ret)
>  		return ret;
> -	if (UCSI_CCI_CONNECTOR(cci))
> -		ucsi_connector_change(ucsi, UCSI_CCI_CONNECTOR(cci));
> +
> +	for (i = 0; i < ucsi->cap.num_connectors; i++)
> +		if (UCSI_CONSTAT(&connector[i], CONNECTED) ||
> +		    UCSI_CCI_CONNECTOR(cci) == i + 1)
> +			ucsi_connector_change(ucsi, i + 1);

ucsi_connector_change() really is supposed to handle an actual event.
The second call to the function will be a NOP because EVENT_PENDING
is already set. So your fix only works for one established connection.

Additionally, calling ucsi_connector_change() will set
EVENT_PENDING which in turn will cause the next ACK to include
UCSI_ACK_CONNECTOR_CHANGE which acks a connector change event
that never happend. That is, unless such an event did happen in the
mean time. In this case you will ACK the event without handling
it.

Also, I seem to recall that some backends don't like it when you
ACK something that did not happen.

Best regards,
Christian

>  	return 0;
>  
> 
> ---
> base-commit: 55431f6adac42f818a0c76594ce2c6c02fe1034d
> change-id: 20260813-ucsi-cold-b35e11991b7c
> 
> Best regards,
> -- 
> William Bright <[email protected]>
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.