[1/1] usb: typec: tcpm: PSSourceOffTimer timeout in PR_Swap enters ERROR_RECOVERY

Commit Message

joswang Feb. 9, 2025, 7:17 a.m. UTC

From: Jos Wang <[e-mail]>

As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is
used by the Policy Engine in Dual-Role Power device that is currently
acting as a Sink to timeout on a PS_RDY Message during a Power Role
Swap sequence. This condition leads to a Hard Reset for USB Type-A and
Type-B Plugs and Error Recovery for Type-C plugs and return to USB
Default Operation.

Therefore, after PSSourceOffTimer timeout, the tcpm state machine should
switch from PR_SWAP_SNK_SRC_SINK_OFF to ERROR_RECOVERY. This can also solve
the test items in the USB power delivery compliance test:
TEST.PD.PROT.SNK.12 PR_Swap – PSSourceOffTimer Timeout

[1] https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2025_OR.zip

Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
Cc: [e-mail]

Signed-off-by: Jos Wang <[e-mail]>
---
 drivers/usb/typec/tcpm/tcpm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Amit Sunil Dhamne Feb. 10, 2025, 11:02 p.m. UTC | #1

On 2/8/25 11:17 PM, joswang wrote:
> From: Jos Wang <[e-mail]>
>
> As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is
> used by the Policy Engine in Dual-Role Power device that is currently
> acting as a Sink to timeout on a PS_RDY Message during a Power Role
> Swap sequence. This condition leads to a Hard Reset for USB Type-A and
> Type-B Plugs and Error Recovery for Type-C plugs and return to USB
> Default Operation.
>
> Therefore, after PSSourceOffTimer timeout, the tcpm state machine should
> switch from PR_SWAP_SNK_SRC_SINK_OFF to ERROR_RECOVERY. This can also solve
> the test items in the USB power delivery compliance test:
> TEST.PD.PROT.SNK.12 PR_Swap – PSSourceOffTimer Timeout
>
> [1] https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2025_OR.zip
>
> Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
> Cc: [e-mail]
>
> Signed-off-by: Jos Wang <[e-mail]>

Tested-by: Amit Sunil Dhamne <[e-mail]>


Regards,

Amit

> ---
>   drivers/usb/typec/tcpm/tcpm.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 47be450d2be3..6bf1a22c785a 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -5591,8 +5591,7 @@ static void run_state_machine(struct tcpm_port *port)
>   		tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB,
>   						       port->pps_data.active, 0);
>   		tcpm_set_charge(port, false);
> -		tcpm_set_state(port, hard_reset_state(port),
> -			       port->timings.ps_src_off_time);
> +		tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
>   		break;
>   	case PR_SWAP_SNK_SRC_SOURCE_ON:
>   		tcpm_enable_auto_vbus_discharge(port, true);

Badhri Jagan Sridharan Feb. 10, 2025, 11:50 p.m. UTC | #2

On Mon, Feb 10, 2025 at 3:02 PM Amit Sunil Dhamne <[e-mail]> wrote:
>
>
> On 2/8/25 11:17 PM, joswang wrote:
> > From: Jos Wang <[e-mail]>
nit: From https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619

  - A ``from`` line specifying the patch author, followed by an empty
    line (only needed if the person sending the patch is not the author).

Given that you are the author, wondering why do you have an explicit "From:" ?

> >
> > As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is

nit: https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619

 - The body of the explanation, line wrapped at 75 columns, which will
    be copied to the permanent changelog to describe this patch.


> > used by the Policy Engine in Dual-Role Power device that is currently
> > acting as a Sink to timeout on a PS_RDY Message during a Power Role
> > Swap sequence. This condition leads to a Hard Reset for USB Type-A and
> > Type-B Plugs and Error Recovery for Type-C plugs and return to USB
> > Default Operation.
> >
> > Therefore, after PSSourceOffTimer timeout, the tcpm state machine should
> > switch from PR_SWAP_SNK_SRC_SINK_OFF to ERROR_RECOVERY. This can also solve
> > the test items in the USB power delivery compliance test:
> > TEST.PD.PROT.SNK.12 PR_Swap – PSSourceOffTimer Timeout

Thanks for fixing this !

> >
> > [1] https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2025_OR.zip
> >
> > Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
> > Cc: [e-mail]
> >
nit: Empty line not needed here.

> > Signed-off-by: Jos Wang <[e-mail]>
>
> Tested-by: Amit Sunil Dhamne <[e-mail]>


>
>
> Regards,
>
> Amit
>
> > ---
> >   drivers/usb/typec/tcpm/tcpm.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 47be450d2be3..6bf1a22c785a 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -5591,8 +5591,7 @@ static void run_state_machine(struct tcpm_port *port)
> >               tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB,
> >                                                      port->pps_data.active, 0);
> >               tcpm_set_charge(port, false);
> > -             tcpm_set_state(port, hard_reset_state(port),
> > -                            port->timings.ps_src_off_time);
> > +             tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
> >               break;
> >       case PR_SWAP_SNK_SRC_SOURCE_ON:
> >               tcpm_enable_auto_vbus_discharge(port, true);

joswang Feb. 11, 2025, 1:50 p.m. UTC | #3

On Tue, Feb 11, 2025 at 7:51 AM Badhri Jagan Sridharan
<[e-mail]> wrote:
>
> On Mon, Feb 10, 2025 at 3:02 PM Amit Sunil Dhamne <[e-mail]> wrote:
> >
> >
> > On 2/8/25 11:17 PM, joswang wrote:
> > > From: Jos Wang <[e-mail]>
> nit: From https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619
>
>   - A ``from`` line specifying the patch author, followed by an empty
>     line (only needed if the person sending the patch is not the author).
>
> Given that you are the author, wondering why do you have an explicit "From:" ?
>
Hello, thank you for your help in reviewing the code.
My company email address is [e-mail], and my personal gmail
email address is [e-mail], which is used to send patches.
Do you suggest deleting the "From:" line?
I am considering deleting the "From:" line, whether the author and
Signed-off-by in the patch need to be changed to
"[e-mail]".
> > >
> > > As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is
>
> nit: https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619
>
>  - The body of the explanation, line wrapped at 75 columns, which will
>     be copied to the permanent changelog to describe this patch.
>
"As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is"
This sentence doesn’t exceed 75 chars, right?
>
> > > used by the Policy Engine in Dual-Role Power device that is currently
> > > acting as a Sink to timeout on a PS_RDY Message during a Power Role
> > > Swap sequence. This condition leads to a Hard Reset for USB Type-A and
> > > Type-B Plugs and Error Recovery for Type-C plugs and return to USB
> > > Default Operation.
> > >
> > > Therefore, after PSSourceOffTimer timeout, the tcpm state machine should
> > > switch from PR_SWAP_SNK_SRC_SINK_OFF to ERROR_RECOVERY. This can also solve
> > > the test items in the USB power delivery compliance test:
> > > TEST.PD.PROT.SNK.12 PR_Swap – PSSourceOffTimer Timeout
>
> Thanks for fixing this !
>
> > >
> > > [1] https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2025_OR.zip
> > >
> > > Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
> > > Cc: [e-mail]
> > >
> nit: Empty line not needed here.
>
Modifications for the next version

> > > Signed-off-by: Jos Wang <[e-mail]>
> >
> > Tested-by: Amit Sunil Dhamne <[e-mail]>
>
>
> >
> >
> > Regards,
> >
> > Amit
> >
> > > ---
> > >   drivers/usb/typec/tcpm/tcpm.c | 3 +--
> > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > > index 47be450d2be3..6bf1a22c785a 100644
> > > --- a/drivers/usb/typec/tcpm/tcpm.c
> > > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > > @@ -5591,8 +5591,7 @@ static void run_state_machine(struct tcpm_port *port)
> > >               tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB,
> > >                                                      port->pps_data.active, 0);
> > >               tcpm_set_charge(port, false);
> > > -             tcpm_set_state(port, hard_reset_state(port),
> > > -                            port->timings.ps_src_off_time);
> > > +             tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
> > >               break;
> > >       case PR_SWAP_SNK_SRC_SOURCE_ON:
> > >               tcpm_enable_auto_vbus_discharge(port, true);

Badhri Jagan Sridharan Feb. 13, 2025, 7:16 a.m. UTC | #4

On Tue, Feb 11, 2025 at 5:50 AM Jos Wang <[e-mail]> wrote:
>
> On Tue, Feb 11, 2025 at 7:51 AM Badhri Jagan Sridharan
> <[e-mail]> wrote:
> >
> > On Mon, Feb 10, 2025 at 3:02 PM Amit Sunil Dhamne <[e-mail]> wrote:
> > >
> > >
> > > On 2/8/25 11:17 PM, joswang wrote:
> > > > From: Jos Wang <[e-mail]>
> > nit: From https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619
> >
> >   - A ``from`` line specifying the patch author, followed by an empty
> >     line (only needed if the person sending the patch is not the author).
> >
> > Given that you are the author, wondering why do you have an explicit "From:" ?
> >
> Hello, thank you for your help in reviewing the code.
> My company email address is [e-mail], and my personal gmail
> email address is [e-mail], which is used to send patches.
> Do you suggest deleting the "From:" line?
> I am considering deleting the "From:" line, whether the author and
> Signed-off-by in the patch need to be changed to
> "[e-mail]".

Yes, changing signed-off to [e-mail] will remove the need
for "From:".
Go ahead with it if it makes sense on your side.



> > > >
> > > > As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is
> >
> > nit: https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619
> >
> >  - The body of the explanation, line wrapped at 75 columns, which will
> >     be copied to the permanent changelog to describe this patch.
> >
> "As PD2.0 spec ("6.5.6.2 PSSourceOffTimer")，the PSSourceOffTimer is"
> This sentence doesn’t exceed 75 chars, right?

Apparently, It actually needs to be wrapped around 75 columns, not too
early either.

Thanks,
Badhri

> >
> > > > used by the Policy Engine in Dual-Role Power device that is currently
> > > > acting as a Sink to timeout on a PS_RDY Message during a Power Role
> > > > Swap sequence. This condition leads to a Hard Reset for USB Type-A and
> > > > Type-B Plugs and Error Recovery for Type-C plugs and return to USB
> > > > Default Operation.
> > > >
> > > > Therefore, after PSSourceOffTimer timeout, the tcpm state machine should
> > > > switch from PR_SWAP_SNK_SRC_SINK_OFF to ERROR_RECOVERY. This can also solve
> > > > the test items in the USB power delivery compliance test:
> > > > TEST.PD.PROT.SNK.12 PR_Swap – PSSourceOffTimer Timeout
> >
> > Thanks for fixing this !
> >
> > > >
> > > > [1] https://usb.org/document-library/usb-power-delivery-compliance-test-specification-0/USB_PD3_CTS_Q4_2025_OR.zip
> > > >
> > > > Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
> > > > Cc: [e-mail]
> > > >
> > nit: Empty line not needed here.
> >
> Modifications for the next version
>
> > > > Signed-off-by: Jos Wang <[e-mail]>
> > >
> > > Tested-by: Amit Sunil Dhamne <[e-mail]>
> >
> >
> > >
> > >
> > > Regards,
> > >
> > > Amit
> > >
> > > > ---
> > > >   drivers/usb/typec/tcpm/tcpm.c | 3 +--
> > > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > > > index 47be450d2be3..6bf1a22c785a 100644
> > > > --- a/drivers/usb/typec/tcpm/tcpm.c
> > > > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > > > @@ -5591,8 +5591,7 @@ static void run_state_machine(struct tcpm_port *port)
> > > >               tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB,
> > > >                                                      port->pps_data.active, 0);
> > > >               tcpm_set_charge(port, false);
> > > > -             tcpm_set_state(port, hard_reset_state(port),
> > > > -                            port->timings.ps_src_off_time);
> > > > +             tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
> > > >               break;
> > > >       case PR_SWAP_SNK_SRC_SOURCE_ON:
> > > >               tcpm_enable_auto_vbus_discharge(port, true);

Greg KH Feb. 13, 2025, 9:47 a.m. UTC | #5

On Wed, Feb 12, 2025 at 11:16:35PM -0800, Badhri Jagan Sridharan wrote:
> On Tue, Feb 11, 2025 at 5:50 AM Jos Wang <[e-mail]> wrote:
> >
> > On Tue, Feb 11, 2025 at 7:51 AM Badhri Jagan Sridharan
> > <[e-mail]> wrote:
> > >
> > > On Mon, Feb 10, 2025 at 3:02 PM Amit Sunil Dhamne <[e-mail]> wrote:
> > > >
> > > >
> > > > On 2/8/25 11:17 PM, joswang wrote:
> > > > > From: Jos Wang <[e-mail]>
> > > nit: From https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619
> > >
> > >   - A ``from`` line specifying the patch author, followed by an empty
> > >     line (only needed if the person sending the patch is not the author).
> > >
> > > Given that you are the author, wondering why do you have an explicit "From:" ?
> > >
> > Hello, thank you for your help in reviewing the code.
> > My company email address is [e-mail], and my personal gmail
> > email address is [e-mail], which is used to send patches.
> > Do you suggest deleting the "From:" line?
> > I am considering deleting the "From:" line, whether the author and
> > Signed-off-by in the patch need to be changed to
> > "[e-mail]".
> 
> Yes, changing signed-off to [e-mail] will remove the need
> for "From:".
> Go ahead with it if it makes sense on your side.

No, what was done here originally is correct, please do not ask people
to not actually follow the correct procedure.

Jos, thank you, there is nothing wrong with the way you sent this.

thanks,

greg k-h

joswang Feb. 13, 2025, 1:45 p.m. UTC | #6

On Thu, Feb 13, 2025 at 5:47 PM Greg KH <[e-mail]> wrote:
>
> On Wed, Feb 12, 2025 at 11:16:35PM -0800, Badhri Jagan Sridharan wrote:
> > On Tue, Feb 11, 2025 at 5:50 AM Jos Wang <[e-mail]> wrote:
> > >
> > > On Tue, Feb 11, 2025 at 7:51 AM Badhri Jagan Sridharan
> > > <[e-mail]> wrote:
> > > >
> > > > On Mon, Feb 10, 2025 at 3:02 PM Amit Sunil Dhamne <[e-mail]> wrote:
> > > > >
> > > > >
> > > > > On 2/8/25 11:17 PM, joswang wrote:
> > > > > > From: Jos Wang <[e-mail]>
> > > > nit: From https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/submitting-patches.rst#L619
> > > >
> > > >   - A ``from`` line specifying the patch author, followed by an empty
> > > >     line (only needed if the person sending the patch is not the author).
> > > >
> > > > Given that you are the author, wondering why do you have an explicit "From:" ?
> > > >
> > > Hello, thank you for your help in reviewing the code.
> > > My company email address is [e-mail], and my personal gmail
> > > email address is [e-mail], which is used to send patches.
> > > Do you suggest deleting the "From:" line?
> > > I am considering deleting the "From:" line, whether the author and
> > > Signed-off-by in the patch need to be changed to
> > > "[e-mail]".
> >
> > Yes, changing signed-off to [e-mail] will remove the need
> > for "From:".
> > Go ahead with it if it makes sense on your side.
>
> No, what was done here originally is correct, please do not ask people
> to not actually follow the correct procedure.
>
> Jos, thank you, there is nothing wrong with the way you sent this.
>
> thanks,
>
> greg k-h

Thanks，I will submit the v2 version patch soon.

Thanks,

Jos

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 47be450d2be3..6bf1a22c785a 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -5591,8 +5591,7 @@  static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB,
 						       port->pps_data.active, 0);
 		tcpm_set_charge(port, false);
-		tcpm_set_state(port, hard_reset_state(port),
-			       port->timings.ps_src_off_time);
+		tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
 		break;
 	case PR_SWAP_SNK_SRC_SOURCE_ON:
 		tcpm_enable_auto_vbus_discharge(port, true);