diff mbox series

usb: typec: tcpm: Use configured PD revision for negotiation

Message ID 20250508174756.1300942-1-chou.cosmo@gmail.com
State New
Headers show
Series usb: typec: tcpm: Use configured PD revision for negotiation | expand

Commit Message

Cosmo Chou May 8, 2025, 5:47 p.m. UTC
Initialize negotiated_rev and negotiated_rev_prime based on the port's
configured PD revision (rev_major) rather than always defaulting to
PD_MAX_REV. This ensures ports start PD communication using their
appropriate revision level.

This allows proper communication with devices that require specific
PD revision levels, especially for the hardware designed for PD 1.0
or 2.0 specifications.

Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Amit Sunil Dhamne May 12, 2025, 10 p.m. UTC | #1
Hi Cosmo,

Thanks for the patch!

On 5/8/25 10:47 AM, Cosmo Chou wrote:
> Initialize negotiated_rev and negotiated_rev_prime based on the port's
> configured PD revision (rev_major) rather than always defaulting to
> PD_MAX_REV. This ensures ports start PD communication using their
> appropriate revision level.
>
> This allows proper communication with devices that require specific
> PD revision levels, especially for the hardware designed for PD 1.0
I didn't know PD1.0 is still used.
> or 2.0 specifications.
>
> Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
> ---
>   drivers/usb/typec/tcpm/tcpm.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a99db4e025cd..5a58c21c4d14 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4782,8 +4782,13 @@ static void run_state_machine(struct tcpm_port *port)
>   		typec_set_pwr_opmode(port->typec_port, opmode);
>   		port->pwr_opmode = TYPEC_PWR_MODE_USB;
>   		port->caps_count = 0;
> -		port->negotiated_rev = PD_MAX_REV;
> -		port->negotiated_rev_prime = PD_MAX_REV;
> +		if (port->pd_rev.rev_major > 0 && port->pd_rev.rev_major <= PD_MAX_REV + 1) {

For better readability, I'd prefer you use macros for the numerical 
values and implement this logic as a switch case. This would make the 
value difference between PD specification revision in PD Message header 
vs PD max revision AMS transparent to the reader.

Thanks,

Amit

> +			port->negotiated_rev = port->pd_rev.rev_major - 1;
> +			port->negotiated_rev_prime = port->pd_rev.rev_major - 1;
> +		} else {
> +			port->negotiated_rev = PD_MAX_REV;
> +			port->negotiated_rev_prime = PD_MAX_REV;
> +		}
>   		port->message_id = 0;
>   		port->message_id_prime = 0;
>   		port->rx_msgid = -1;
> @@ -5048,8 +5053,13 @@ static void run_state_machine(struct tcpm_port *port)
>   					      port->cc2 : port->cc1);
>   		typec_set_pwr_opmode(port->typec_port, opmode);
>   		port->pwr_opmode = TYPEC_PWR_MODE_USB;
> -		port->negotiated_rev = PD_MAX_REV;
> -		port->negotiated_rev_prime = PD_MAX_REV;
> +		if (port->pd_rev.rev_major > 0 && port->pd_rev.rev_major <= PD_MAX_REV + 1) {
> +			port->negotiated_rev = port->pd_rev.rev_major - 1;
> +			port->negotiated_rev_prime = port->pd_rev.rev_major - 1;
> +		} else {
> +			port->negotiated_rev = PD_MAX_REV;
> +			port->negotiated_rev_prime = PD_MAX_REV;
> +		}
>   		port->message_id = 0;
>   		port->message_id_prime = 0;
>   		port->rx_msgid = -1;
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index a99db4e025cd..5a58c21c4d14 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4782,8 +4782,13 @@  static void run_state_machine(struct tcpm_port *port)
 		typec_set_pwr_opmode(port->typec_port, opmode);
 		port->pwr_opmode = TYPEC_PWR_MODE_USB;
 		port->caps_count = 0;
-		port->negotiated_rev = PD_MAX_REV;
-		port->negotiated_rev_prime = PD_MAX_REV;
+		if (port->pd_rev.rev_major > 0 && port->pd_rev.rev_major <= PD_MAX_REV + 1) {
+			port->negotiated_rev = port->pd_rev.rev_major - 1;
+			port->negotiated_rev_prime = port->pd_rev.rev_major - 1;
+		} else {
+			port->negotiated_rev = PD_MAX_REV;
+			port->negotiated_rev_prime = PD_MAX_REV;
+		}
 		port->message_id = 0;
 		port->message_id_prime = 0;
 		port->rx_msgid = -1;
@@ -5048,8 +5053,13 @@  static void run_state_machine(struct tcpm_port *port)
 					      port->cc2 : port->cc1);
 		typec_set_pwr_opmode(port->typec_port, opmode);
 		port->pwr_opmode = TYPEC_PWR_MODE_USB;
-		port->negotiated_rev = PD_MAX_REV;
-		port->negotiated_rev_prime = PD_MAX_REV;
+		if (port->pd_rev.rev_major > 0 && port->pd_rev.rev_major <= PD_MAX_REV + 1) {
+			port->negotiated_rev = port->pd_rev.rev_major - 1;
+			port->negotiated_rev_prime = port->pd_rev.rev_major - 1;
+		} else {
+			port->negotiated_rev = PD_MAX_REV;
+			port->negotiated_rev_prime = PD_MAX_REV;
+		}
 		port->message_id = 0;
 		port->message_id_prime = 0;
 		port->rx_msgid = -1;