Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: VR+3QcAulfk/HIxfCiq8T2S1bybY+5vQK8lxbQRkQNUGpEV2tQfeUNAqVZZRweEBOv/QS7Of1T
 5LPkzzBzO9zQ==
IronPort-SDR: yWGynPXoF/ygcZzdiuq4SnHzVEviyzMnEv8e3E0nomUDhYzbJYzVNCN8LMryjRcCns9bAmMmcb
 nwLn6d8Wq2LQ==
Date:   Wed, 19 Aug 2020 09:30:54 +0300
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     Badhri Jagan Sridharan <badhri@google.com>
Cc:     Guenter Roeck <linux@roeck-us.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2 v4] tcpm: During PR_SWAP, source caps should be sent
 only after tSwapSourceStart
Message-ID: <20200819063054.GA2772165@kuha.fi.intel.com>
References: <20200817183828.1895015-1-badhri@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200817183828.1895015-1-badhri@google.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Aug 17, 2020 at 11:38:27AM -0700, Badhri Jagan Sridharan wrote:
> The patch addresses the compliance test failures while running
> TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 of the "Deterministic PD
> Compliance MOI" test plan published in https://www.usb.org/usbc.
> For a product to be Type-C compliant, it's expected that these tests
> are run on usb.org certified Type-C compliance tester as mentioned in
> https://www.usb.org/usbc.
> 
> The purpose of the tests TD.PD.CP.E3, TD.PD.CP.E4, TD.PD.CP.E5 is to
> verify the PR_SWAP response of the device. While doing so, the test
> asserts that Source Capabilities message is NOT received from the test
> device within tSwapSourceStart min (20 ms) from the time the last bit
> of GoodCRC corresponding to the RS_RDY message sent by the UUT was
> sent. If it does then the test fails.
> 
> This is in line with the requirements from the USB Power Delivery
> Specification Revision 3.0, Version 1.2:
> "6.6.8.1 SwapSourceStartTimer
> The SwapSourceStartTimer Shall be used by the new Source, after a
> Power Role Swap or Fast Role Swap, to ensure that it does not send
> Source_Capabilities Message before the new Sink is ready to receive
> the
> Source_Capabilities Message. The new Source Shall Not send the
> Source_Capabilities Message earlier than tSwapSourceStart after the
> last bit of the EOP of GoodCRC Message sent in response to the PS_RDY
> Message sent by the new Source indicating that its power supply is
> ready."
> 
> The patch makes sure that TCPM does not send the Source_Capabilities
> Message within tSwapSourceStart(20ms) by transitioning into
> SRC_STARTUP only after  tSwapSourceStart(20ms).
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes since V1:
> - Comment on the permissible values of tSwapSourceStart
> 
> Changes since V2:
> - Fixing alignment issue pointed out by Guenter.
> - Added Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> Changes since V3:
> - Updated commit description and made it elaborate to address Heikki's
>   suggestion.
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 2 +-
>  include/linux/usb/pd.h        | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3ef37202ee37..d38347bd3335 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -3555,7 +3555,7 @@ static void run_state_machine(struct tcpm_port *port)
>  		 */
>  		tcpm_set_pwr_role(port, TYPEC_SOURCE);
>  		tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
> -		tcpm_set_state(port, SRC_STARTUP, 0);
> +		tcpm_set_state(port, SRC_STARTUP, PD_T_SWAP_SRC_START);
>  		break;
>  
>  	case VCONN_SWAP_ACCEPT:
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index b6c233e79bd4..1df895e4680b 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -473,6 +473,7 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
>  #define PD_T_SRCSWAPSTDBY      625     /* Maximum of 650ms */
>  #define PD_T_NEWSRC            250     /* Maximum of 275ms */
> +#define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
>  
>  #define PD_T_DRP_TRY		100	/* 75 - 150 ms */
>  #define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */

thanks,

-- 
heikki