Re: [PATCH] usb: typec: tcpm: fix warning when handle discover_identity message

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

Hi,

On Wed, Feb 15, 2023 at 07:31:36PM +0800, Xu Yang wrote:
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 7f39cb9b3429..914bbaf4e25e 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1445,10 +1445,19 @@ static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams)
>  static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header,
>  			   const u32 *data, int cnt)
>  {
> +	u32 vdo_hdr = port->vdo_data[0];
> +
>  	WARN_ON(!mutex_is_locked(&port->lock));
>  
> -	/* Make sure we are not still processing a previous VDM packet */
> -	WARN_ON(port->vdm_state > VDM_STATE_DONE);
> +	/* If is sending discover_identity, handle received message firstly */
> +	if (PD_VDO_SVDM(vdo_hdr) &&
> +		PD_VDO_CMD(vdo_hdr) == CMD_DISCOVER_IDENT) {

One line is enough.

> +		port->send_discover = true;
> +		mod_send_discover_delayed_work(port,
> +					SEND_DISCOVER_RETRY_MS);

Ditto.

> +	} else
> +		/* Make sure we are not still processing a previous VDM packet */
> +		WARN_ON(port->vdm_state > VDM_STATE_DONE);

You need braces around the else statement in this case.

>  
>  	port->vdo_count = cnt + 1;
>  	port->vdo_data[0] = header;
> @@ -1950,9 +1959,11 @@ static void vdm_run_state_machine(struct tcpm_port *port)
>  				res = tcpm_ams_start(port, DISCOVER_IDENTITY);
>  				if (res == 0)
>  					port->send_discover = false;
> -				else if (res == -EAGAIN)
> +				else if (res == -EAGAIN) {
> +					port->vdo_data[0] = 0;
>  					mod_send_discover_delayed_work(port,
>  								       SEND_DISCOVER_RETRY_MS);
> +				}
>  				break;
>  			case CMD_DISCOVER_SVID:
>  				res = tcpm_ams_start(port, DISCOVER_SVIDS);
> @@ -2035,6 +2046,7 @@ static void vdm_run_state_machine(struct tcpm_port *port)
>  			unsigned long timeout;
>  
>  			port->vdm_retries = 0;
> +			port->vdo_data[0] = 0;
>  			port->vdm_state = VDM_STATE_BUSY;
>  			timeout = vdm_ready_timeout(vdo_hdr);
>  			mod_vdm_delayed_work(port, timeout);

thanks,

-- 
heikki