diff mbox series

[6/6] usb: typec: tcpm: Add WARN_ON ensure we are not tryong to send 2 VDM packets at the same time

Message ID 20200715132301.99816-7-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series usb: typec: tcpm: Fix AB BA lock inversion between tcpm and alt-mode drivers | expand

Commit Message

Hans de Goede July 15, 2020, 1:23 p.m. UTC
The tcpm.c code for sending VDMs assumes that there will only be one VDM
in flight at the time. The "queue" used by tcpm_queue_vdm is only 1 entry
deep.

This assumes that the higher layers (tcpm state-machine and alt-mode
drivers) ensure that queuing a new VDM before the old one has been
completely send (or it timed out) add a WARN_ON to check for this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Guenter Roeck July 15, 2020, 4:06 p.m. UTC | #1
On 7/15/20 6:23 AM, Hans de Goede wrote:
> The tcpm.c code for sending VDMs assumes that there will only be one VDM
> in flight at the time. The "queue" used by tcpm_queue_vdm is only 1 entry
> deep.
> 
> This assumes that the higher layers (tcpm state-machine and alt-mode
> drivers) ensure that queuing a new VDM before the old one has been
> completely send (or it timed out) add a WARN_ON to check for this.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

subject: s/tryong/trying/

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index ea14240423d1..5adc30666566 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -971,6 +971,9 @@ static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header,
>  {
>  	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);
> +
>  	port->vdo_count = cnt + 1;
>  	port->vdo_data[0] = header;
>  	memcpy(&port->vdo_data[1], data, sizeof(u32) * cnt);
>
Hans de Goede July 24, 2020, 3:59 p.m. UTC | #2
Hi,

On 7/15/20 6:06 PM, Guenter Roeck wrote:
> On 7/15/20 6:23 AM, Hans de Goede wrote:
>> The tcpm.c code for sending VDMs assumes that there will only be one VDM
>> in flight at the time. The "queue" used by tcpm_queue_vdm is only 1 entry
>> deep.
>>
>> This assumes that the higher layers (tcpm state-machine and alt-mode
>> drivers) ensure that queuing a new VDM before the old one has been
>> completely send (or it timed out) add a WARN_ON to check for this.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> subject: s/tryong/trying/

Ack, fixed for v2.

Regards,

Hans



> 
>> ---
>>   drivers/usb/typec/tcpm/tcpm.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>> index ea14240423d1..5adc30666566 100644
>> --- a/drivers/usb/typec/tcpm/tcpm.c
>> +++ b/drivers/usb/typec/tcpm/tcpm.c
>> @@ -971,6 +971,9 @@ static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header,
>>   {
>>   	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);
>> +
>>   	port->vdo_count = cnt + 1;
>>   	port->vdo_data[0] = header;
>>   	memcpy(&port->vdo_data[1], data, sizeof(u32) * cnt);
>>
>
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index ea14240423d1..5adc30666566 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -971,6 +971,9 @@  static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header,
 {
 	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);
+
 	port->vdo_count = cnt + 1;
 	port->vdo_data[0] = header;
 	memcpy(&port->vdo_data[1], data, sizeof(u32) * cnt);