Re: [PATCH v6 09/14] usb: typec: tcpm: Add support for Sink Fast Role SWAP(FRS)

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Mon, Aug 31, 2020 at 07:59:22PM -0700, Badhri Jagan Sridharan wrote:
> PD 3.0 spec defines a new mechanism for power role swap called
> Fast role swap. This change enables TCPM to support FRS when
> acting as sink.
> 
> Once the explicit contract is negotiated, sink port is
> expected to query the source port for sink caps to
> determine whether the source is FRS capable.
> Bits 23 & 24 of fixed pdo of the sink caps from the source, when
> set, indicates the current needed by the source when fast role
> swap is in progress(Implicit contract phasae). 0 indicates that
> the source does not support Fast Role Swap.
> 
> Upon receiving the FRS signal from the source,
> TCPC(TCPM_FRS_EVENT) informs TCPM to start the Fast role swap sequence.
> 
> 1. TCPM sends FRS PD message: FR_SWAP_SEND
> 2. If response is not received within the expiry of
>    SenderResponseTimer, Error recovery is triggered.:
>    FR_SWAP_SEND_TIMEOUT
> 3. Upon receipt of the accept message, TCPM waits for
>    PSSourceOffTimer for PS_READY message from the partner:
>    FR_SWAP_SNK_SRC_NEW_SINK_READY.
> 
> TCPC is expected to autonomously turn on vbus once the FRS
> signal is received and vbus voltage falls below vsafe5v within
> tSrcFrSwap. This is different from traditional power role swap
> where the vbus sourcing is turned on by TCPM.
> 
> 4. By this time, TCPC most likely would have started to
>    source vbus, TCPM waits for tSrcFrSwap to see  if the
>    lower level TCPC driver signals TCPM_SOURCING_VBUS event:
>    FR_SWAP_SNK_SRC_SOURCE_VBUS_APPLIED.
> 5. When TCPC signals sourcing vbus, TCPM sends PS_READY msg and
>    changes the CC pin from Rd to Rp. This is the end of fast
>    role swap sequence and TCPM initiates the sequnce to negotiate
>    explicit contract by transitioning into SRC_STARTUP after
>    SwapSrcStart.
> 
> The code is written based on the sequence described in "Figure 8-107:
> Dual-role Port in Sink to Source Fast Role Swap State Diagram" of
> USB Power Delivery Specification Revision 3.0, Version 1.2.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
> Changes since v1:
> - Changing patch version to v6 to fix version number confusion.
> - Rebased on top of usb-next and resolved conflicts due to the below
>   changes:
>   3ed8e1c2ac99 usb: typec: tcpm: Migrate workqueue to RT priority for processing events
>   6bbe2a90a0bb usb: typec: tcpm: During PR_SWAP, source caps should be sent only after tSwapSourceStart
> - enable_frs sequence is now run as part of the same kthread that runs
>   the state machines.
> - Fixed the implicit fallthrough warning in the switch case for
>   FR_SWAP_CANCEL case.
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 217 +++++++++++++++++++++++++++++++++-
>  include/linux/usb/pd.h        |  19 +--
>  include/linux/usb/tcpm.h      |   8 +-
>  include/linux/usb/typec.h     |  13 ++
>  4 files changed, 245 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 92806547f485..083e7af107b2 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -106,6 +106,13 @@
>  	S(VCONN_SWAP_TURN_ON_VCONN),		\
>  	S(VCONN_SWAP_TURN_OFF_VCONN),		\
>  						\
> +	S(FR_SWAP_SEND),			\
> +	S(FR_SWAP_SEND_TIMEOUT),		\
> +	S(FR_SWAP_SNK_SRC_TRANSITION_TO_OFF),			\
> +	S(FR_SWAP_SNK_SRC_NEW_SINK_READY),		\
> +	S(FR_SWAP_SNK_SRC_SOURCE_VBUS_APPLIED),	\
> +	S(FR_SWAP_CANCEL),			\
> +						\
>  	S(SNK_TRY),				\
>  	S(SNK_TRY_WAIT),			\
>  	S(SNK_TRY_WAIT_DEBOUNCE),               \
> @@ -127,6 +134,9 @@
>  	S(GET_PPS_STATUS_SEND),			\
>  	S(GET_PPS_STATUS_SEND_TIMEOUT),		\
>  						\
> +	S(GET_SINK_CAP),			\
> +	S(GET_SINK_CAP_TIMEOUT),		\
> +						\
>  	S(ERROR_RECOVERY),			\
>  	S(PORT_RESET),				\
>  	S(PORT_RESET_WAIT_OFF)
> @@ -175,6 +185,8 @@ enum adev_actions {
>  #define TCPM_CC_EVENT		BIT(0)
>  #define TCPM_VBUS_EVENT		BIT(1)
>  #define TCPM_RESET_EVENT	BIT(2)
> +#define TCPM_FRS_EVENT		BIT(3)
> +#define TCPM_SOURCING_VBUS	BIT(4)
>  
>  #define LOG_BUFFER_ENTRIES	1024
>  #define LOG_BUFFER_ENTRY_SIZE	128
> @@ -184,6 +196,8 @@ enum adev_actions {
>  #define SVID_DISCOVERY_MAX	16
>  #define ALTMODE_DISCOVERY_MAX	(SVID_DISCOVERY_MAX * MODE_DISCOVERY_MAX)
>  
> +#define GET_SINK_CAP_RETRY_MS	100
> +
>  struct pd_mode_data {
>  	int svid_index;		/* current SVID index		*/
>  	int nsvids;
> @@ -261,6 +275,8 @@ struct tcpm_port {
>  	struct kthread_work state_machine;
>  	struct hrtimer vdm_state_machine_timer;
>  	struct kthread_work vdm_state_machine;
> +	struct hrtimer enable_frs_timer;
> +	struct kthread_work enable_frs;
>  	bool state_machine_running;
>  
>  	struct completion tx_complete;
> @@ -335,6 +351,12 @@ struct tcpm_port {
>  	/* port belongs to a self powered device */
>  	bool self_powered;
>  
> +	/* FRS */
> +	enum frs_typec_current frs_current;

Do you use this member anywhere?

I can't see anywhere it being used, but I think you should use it
instead the one in port_caps.

> +	/* Sink caps have been queried */
> +	bool sink_cap_done;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	struct dentry *dentry;
>  	struct mutex logbuffer_lock;	/* log buffer access lock */
> @@ -940,6 +962,16 @@ static void mod_vdm_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
>  	}
>  }

<snip>

> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 9cb1bec94b71..2ec7451a14ab 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -204,6 +204,18 @@ struct typec_operations {
>  			     enum typec_port_type type);
>  };
>  
> +/*
> + * Initial current capability of the new source when vSafe5V is applied during PD3.0 Fast Role Swap.
> + * Based on "Table 6-14 Fixed Supply PDO - Sink" of "USB Power Delivery Specification Revision 3.0,
> + * Version 1.2"
> + */
> +enum frs_typec_current {
> +	FRS_NOT_SUPPORTED,
> +	FRS_DEFAULT_POWER,
> +	FRS_5V_1P5A,
> +	FRS_5V_3A,
> +};
> +
>  /*
>   * struct typec_capability - USB Type-C Port Capabilities
>   * @type: Supported power role of the port
> @@ -226,6 +238,7 @@ struct typec_capability {
>  	int			prefer_role;
>  	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
>  	unsigned int		orientation_aware:1;
> +	enum frs_typec_current	frs_current;

So forget about this. We can add it here if/when there is another
driver that needs that. You already have that member in struct
tcpm_port, so just use that for now.

And you can also introduce enum frs_typec_current in tcpm.c for now.

>  	struct fwnode_handle	*fwnode;
>  	void			*driver_data;
> -- 
> 2.28.0.402.g5ffc5be6b7-goog

thanks,

-- 
heikki