diff mbox series

[2/4] usb: typec: tcpm: fix multiple times discover svids error

Message ID 20230313025843.17162-3-frank.wang@rock-chips.com
State Superseded
Headers show
Series Fix some defects related Type-C TCPM | expand

Commit Message

Frank Wang March 13, 2023, 2:58 a.m. UTC
PD3.0 Spec 6.4.4.3.2 say that only Responder supports 12 or more SVIDs,
the Discover SVIDs Command Shall be executed multiple times until a
Discover SVIDs VDO is returned ending either with a SVID value of
0x0000 in the last part of the last VDO or with a VDO containing two
SVIDs with values of 0x0000.

In the current implementation, if the last VDO does not find that the
Discover SVIDs Command would be executed multiple times even if the
Responder SVIDs are less than 12, and we found some odd dockers just
meet this case. So fix it.

Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Heikki Krogerus March 14, 2023, 9:22 a.m. UTC | #1
On Mon, Mar 13, 2023 at 10:58:41AM +0800, Frank Wang wrote:
> PD3.0 Spec 6.4.4.3.2 say that only Responder supports 12 or more SVIDs,
> the Discover SVIDs Command Shall be executed multiple times until a
> Discover SVIDs VDO is returned ending either with a SVID value of
> 0x0000 in the last part of the last VDO or with a VDO containing two
> SVIDs with values of 0x0000.
> 
> In the current implementation, if the last VDO does not find that the
> Discover SVIDs Command would be executed multiple times even if the
> Responder SVIDs are less than 12, and we found some odd dockers just
> meet this case. So fix it.
> 
> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 66de02a56f512..2962f7c261976 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1515,7 +1515,21 @@ static bool svdm_consume_svids(struct tcpm_port *port, const u32 *p, int cnt)
>  		pmdata->svids[pmdata->nsvids++] = svid;
>  		tcpm_log(port, "SVID %d: 0x%x", pmdata->nsvids, svid);
>  	}
> -	return true;
> +
> +	/*
> +	 * PD3.0 Spec 6.4.4.3.2: The SVIDs are returned 2 per VDO (see Table
> +	 * 6-43), and can be returned maximum 6 VDOs per response (see Figure
> +	 * 6-19). If the Respondersupports 12 or more SVID then the Discover
> +	 * SVIDs Command Shall be executed multiple times until a Discover
> +	 * SVIDs VDO is returned ending either with a SVID value of 0x0000 in
> +	 * the last part of the last VDO or with a VDO containing two SVIDs
> +	 * with values of 0x0000.
> +	 *
> +	 * However, some odd dockers support SVIDs less than 12 but without
> +	 * 0x0000 in the last VDO, so we need to break the Discover SVIDs
> +	 * request and return false here.
> +	 */
> +	return cnt == 7 ? true : false;

        return cnt == 7


thanks,
Frank Wang March 15, 2023, 2:57 a.m. UTC | #2
Hi Heikki,

On 2023/3/14 17:22, Heikki Krogerus wrote:
> On Mon, Mar 13, 2023 at 10:58:41AM +0800, Frank Wang wrote:
>> PD3.0 Spec 6.4.4.3.2 say that only Responder supports 12 or more SVIDs,
>> the Discover SVIDs Command Shall be executed multiple times until a
>> Discover SVIDs VDO is returned ending either with a SVID value of
>> 0x0000 in the last part of the last VDO or with a VDO containing two
>> SVIDs with values of 0x0000.
>>
>> In the current implementation, if the last VDO does not find that the
>> Discover SVIDs Command would be executed multiple times even if the
>> Responder SVIDs are less than 12, and we found some odd dockers just
>> meet this case. So fix it.
>>
>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>> ---
>>   drivers/usb/typec/tcpm/tcpm.c | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>> index 66de02a56f512..2962f7c261976 100644
>> --- a/drivers/usb/typec/tcpm/tcpm.c
>> +++ b/drivers/usb/typec/tcpm/tcpm.c
>> @@ -1515,7 +1515,21 @@ static bool svdm_consume_svids(struct tcpm_port *port, const u32 *p, int cnt)
>>   		pmdata->svids[pmdata->nsvids++] = svid;
>>   		tcpm_log(port, "SVID %d: 0x%x", pmdata->nsvids, svid);
>>   	}
>> -	return true;
>> +
>> +	/*
>> +	 * PD3.0 Spec 6.4.4.3.2: The SVIDs are returned 2 per VDO (see Table
>> +	 * 6-43), and can be returned maximum 6 VDOs per response (see Figure
>> +	 * 6-19). If the Respondersupports 12 or more SVID then the Discover
>> +	 * SVIDs Command Shall be executed multiple times until a Discover
>> +	 * SVIDs VDO is returned ending either with a SVID value of 0x0000 in
>> +	 * the last part of the last VDO or with a VDO containing two SVIDs
>> +	 * with values of 0x0000.
>> +	 *
>> +	 * However, some odd dockers support SVIDs less than 12 but without
>> +	 * 0x0000 in the last VDO, so we need to break the Discover SVIDs
>> +	 * request and return false here.
>> +	 */
>> +	return cnt == 7 ? true : false;
>          return cnt == 7

Okay, next to fix it.

BR.
Frank
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 66de02a56f512..2962f7c261976 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1515,7 +1515,21 @@  static bool svdm_consume_svids(struct tcpm_port *port, const u32 *p, int cnt)
 		pmdata->svids[pmdata->nsvids++] = svid;
 		tcpm_log(port, "SVID %d: 0x%x", pmdata->nsvids, svid);
 	}
-	return true;
+
+	/*
+	 * PD3.0 Spec 6.4.4.3.2: The SVIDs are returned 2 per VDO (see Table
+	 * 6-43), and can be returned maximum 6 VDOs per response (see Figure
+	 * 6-19). If the Respondersupports 12 or more SVID then the Discover
+	 * SVIDs Command Shall be executed multiple times until a Discover
+	 * SVIDs VDO is returned ending either with a SVID value of 0x0000 in
+	 * the last part of the last VDO or with a VDO containing two SVIDs
+	 * with values of 0x0000.
+	 *
+	 * However, some odd dockers support SVIDs less than 12 but without
+	 * 0x0000 in the last VDO, so we need to break the Discover SVIDs
+	 * request and return false here.
+	 */
+	return cnt == 7 ? true : false;
 abort:
 	tcpm_log(port, "SVID_DISCOVERY_MAX(%d) too low!", SVID_DISCOVERY_MAX);
 	return false;