[PATCH 1/3 v2] usb: typec: tcpci: Support BIST test data mode for compliance.

[PATCH 1/3 v2] usb: typec: tcpci: Support BIST test data mode for compliance.

[Badhri Jagan Sridharan]

Quoting from TCPCI spec:
"Setting this bit to 1 is intended to be used only when a USB compliance
tester is using USB BIST Test Data to test the PHY layer of the TCPC. The
TCPM should clear this bit when a disconnect is detected.
0: Normal Operation. Incoming messages enabled by RECEIVE_DETECT
passed to TCPM via Alert.
1: BIST Test Mode. Incoming messages enabled by RECEIVE_DETECT
result in GoodCRC response but may not be passed to the TCPM via
Alert."

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
Version history:
Changes since V1:(Guenter's suggestions)
- Split the change into two: TCPM and TCPCI
- Move BIST log to TCPM log
- Alignment and column count changes
---
 drivers/usb/typec/tcpm/tcpci.c | 9 +++++++++
 drivers/usb/typec/tcpm/tcpci.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 753645bb25273a..f57d91fd0e0924 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -227,6 +227,14 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
 				enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
 }
 
+static int tcpci_set_bist_data(struct tcpc_dev *tcpc, bool enable)
+{
+	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+
+	return regmap_update_bits(tcpci->regmap, TCPC_TCPC_CTRL, TCPC_TCPC_CTRL_BIST_TM,
+				 enable ? TCPC_TCPC_CTRL_BIST_TM : 0);
+}
+
 static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
 			   enum typec_role role, enum typec_data_role data)
 {
@@ -530,6 +538,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
 	tcpci->tcpc.set_roles = tcpci_set_roles;
 	tcpci->tcpc.pd_transmit = tcpci_pd_transmit;
+	tcpci->tcpc.set_bist_data = tcpci_set_bist_data;
 
 	err = tcpci_parse_config(tcpci);
 	if (err < 0)
diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
index 303ebde265465c..11c36d086c8608 100644
--- a/drivers/usb/typec/tcpm/tcpci.h
+++ b/drivers/usb/typec/tcpm/tcpci.h
@@ -36,6 +36,7 @@
 
 #define TCPC_TCPC_CTRL			0x19
 #define TCPC_TCPC_CTRL_ORIENTATION	BIT(0)
+#define TCPC_TCPC_CTRL_BIST_TM		BIT(1)
 
 #define TCPC_ROLE_CTRL			0x1a
 #define TCPC_ROLE_CTRL_DRP		BIT(6)
-- 
2.27.0.389.gc38d7665816-goog

[PATCH 2/3 v2] usb: typec: tcpm: Support bist test data mode for compliance

[Badhri Jagan Sridharan]

TCPM supports BIST carried mode. PD compliance tests require
BIST Test Data to be supported as well.

Introducing set_bist_data callback to signal tcpc driver for
configuring the port controller hardware to enable/disable
BIST Test Data mode.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
Version history:
Changes since V1:(Guenter's suggestions)
- Split the change into two: TCPM and TCPCI
- Move BIST log to TCPM log
---
 drivers/usb/typec/tcpm/tcpm.c | 11 +++++++++++
 include/linux/usb/tcpm.h      |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 82b19ebd7838e0..379fcab9dbd973 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2746,6 +2746,11 @@ static void tcpm_detach(struct tcpm_port *port)
 	if (!port->attached)
 		return;
 
+	if (port->tcpc->set_bist_data) {
+		tcpm_log(port, "disable BIST MODE TESTDATA");
+		port->tcpc->set_bist_data(port->tcpc, false);
+	}
+
 	if (tcpm_port_is_disconnected(port))
 		port->hard_reset_count = 0;
 
@@ -3555,6 +3560,12 @@ static void run_state_machine(struct tcpm_port *port)
 		case BDO_MODE_CARRIER2:
 			tcpm_pd_transmit(port, TCPC_TX_BIST_MODE_2, NULL);
 			break;
+		case BDO_MODE_TESTDATA:
+			if (port->tcpc->set_bist_data) {
+				tcpm_log(port, "Enable BIST MODE TESTDATA");
+				port->tcpc->set_bist_data(port->tcpc, true);
+			}
+			break;
 		default:
 			break;
 		}
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index e7979c01c3517c..89f58760cf4800 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -79,6 +79,7 @@ enum tcpm_transmit_type {
  * @try_role:	Optional; called to set a preferred role
  * @pd_transmit:Called to transmit PD message
  * @mux:	Pointer to multiplexer data
+ * @set_bist_data: Turn on/off bist data mode for compliance testing
  */
 struct tcpc_dev {
 	struct fwnode_handle *fwnode;
@@ -103,6 +104,7 @@ struct tcpc_dev {
 	int (*try_role)(struct tcpc_dev *dev, int role);
 	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
 			   const struct pd_message *msg);
+	int (*set_bist_data)(struct tcpc_dev *dev, bool on);
 };
 
 struct tcpm_port;
-- 
2.27.0.389.gc38d7665816-goog

[PATCH 3/3 v2] usb: typec: tcpm: Stay in BIST mode till hardreset or unattached

[Badhri Jagan Sridharan]

Port starts to toggle when transitioning to unattached state.
This is incorrect while in BIST mode.

6.4.3.1 BIST Carrier Mode
Upon receipt of a BIST Message, with a BIST Carrier Mode BIST Data Object,
the UUT Shall send out a continuous string of BMC encoded alternating "1"s
and “0”s. The UUT Shall exit the Continuous BIST Mode within
tBISTContMode of this Continuous BIST Mode being enabled(see
Section 6.6.7.2).

6.4.3.2 BIST Test Data
Upon receipt of a BIST Message, with a BIST Test Data BIST Data Object,
the UUT Shall return a GoodCRC Message and Shall enter a test mode in which
it sends no further Messages except for GoodCRC Messages in response to
received Messages. See Section 5.9.2 for the definition of the Test Data
Frame. The test Shall be ended by sending Hard Reset Signaling to reset the
UUT.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
Version history:
Changes since V1:
-  None
---
 drivers/usb/typec/tcpm/tcpm.c | 8 ++++++--
 include/linux/usb/pd.h        | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 379fcab9dbd973..245cfe80948502 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -3559,6 +3559,8 @@ static void run_state_machine(struct tcpm_port *port)
 		switch (BDO_MODE_MASK(port->bist_request)) {
 		case BDO_MODE_CARRIER2:
 			tcpm_pd_transmit(port, TCPC_TX_BIST_MODE_2, NULL);
+			tcpm_set_state(port, unattached_state(port),
+				       PD_T_BIST_CONT_MODE);
 			break;
 		case BDO_MODE_TESTDATA:
 			if (port->tcpc->set_bist_data) {
@@ -3569,8 +3571,6 @@ static void run_state_machine(struct tcpm_port *port)
 		default:
 			break;
 		}
-		/* Always switch to unattached state */
-		tcpm_set_state(port, unattached_state(port), 0);
 		break;
 	case GET_STATUS_SEND:
 		tcpm_pd_send_control(port, PD_CTRL_GET_STATUS);
@@ -3960,6 +3960,10 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 static void _tcpm_pd_hard_reset(struct tcpm_port *port)
 {
 	tcpm_log_force(port, "Received hard reset");
+	if (port->bist_request ==  BDO_MODE_TESTDATA &&
+	    port->tcpc->set_bist_data)
+		port->tcpc->set_bist_data(port->tcpc, false);
+
 	/*
 	 * If we keep receiving hard reset requests, executing the hard reset
 	 * must have failed. Revert to error recovery if that happens.
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index a665d7f211424d..b420d8d613cd23 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -483,4 +483,5 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
 #define PD_N_HARD_RESET_COUNT	2
 
+#define PD_T_BIST_CONT_MODE	60 /* 30 - 60 ms */
 #endif /* __LINUX_USB_PD_H */
-- 
2.27.0.389.gc38d7665816-goog

Re: [PATCH 1/3 v2] usb: typec: tcpci: Support BIST test data mode for compliance.

[Guenter Roeck]

On 7/14/20 4:12 PM, Badhri Jagan Sridharan wrote:
> Quoting from TCPCI spec:
> "Setting this bit to 1 is intended to be used only when a USB compliance
> tester is using USB BIST Test Data to test the PHY layer of the TCPC. The
> TCPM should clear this bit when a disconnect is detected.
> 0: Normal Operation. Incoming messages enabled by RECEIVE_DETECT
> passed to TCPM via Alert.
> 1: BIST Test Mode. Incoming messages enabled by RECEIVE_DETECT
> result in GoodCRC response but may not be passed to the TCPM via
> Alert."
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

This needs to be the second patch in the series. The set_bist_data
callback doesn't exist yet.

Other than that, for the code itself:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Version history:
> Changes since V1:(Guenter's suggestions)
> - Split the change into two: TCPM and TCPCI
> - Move BIST log to TCPM log
> - Alignment and column count changes
> ---
>  drivers/usb/typec/tcpm/tcpci.c | 9 +++++++++
>  drivers/usb/typec/tcpm/tcpci.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 753645bb25273a..f57d91fd0e0924 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -227,6 +227,14 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
>  				enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
>  }
>  
> +static int tcpci_set_bist_data(struct tcpc_dev *tcpc, bool enable)
> +{
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +
> +	return regmap_update_bits(tcpci->regmap, TCPC_TCPC_CTRL, TCPC_TCPC_CTRL_BIST_TM,
> +				 enable ? TCPC_TCPC_CTRL_BIST_TM : 0);
> +}
> +
>  static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
>  			   enum typec_role role, enum typec_data_role data)
>  {
> @@ -530,6 +538,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>  	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>  	tcpci->tcpc.set_roles = tcpci_set_roles;
>  	tcpci->tcpc.pd_transmit = tcpci_pd_transmit;
> +	tcpci->tcpc.set_bist_data = tcpci_set_bist_data;
>  
>  	err = tcpci_parse_config(tcpci);
>  	if (err < 0)
> diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
> index 303ebde265465c..11c36d086c8608 100644
> --- a/drivers/usb/typec/tcpm/tcpci.h
> +++ b/drivers/usb/typec/tcpm/tcpci.h
> @@ -36,6 +36,7 @@
>  
>  #define TCPC_TCPC_CTRL			0x19
>  #define TCPC_TCPC_CTRL_ORIENTATION	BIT(0)
> +#define TCPC_TCPC_CTRL_BIST_TM		BIT(1)
>  
>  #define TCPC_ROLE_CTRL			0x1a
>  #define TCPC_ROLE_CTRL_DRP		BIT(6)
> 

Re: [PATCH 2/3 v2] usb: typec: tcpm: Support bist test data mode for compliance

[Guenter Roeck]

On 7/14/20 4:12 PM, Badhri Jagan Sridharan wrote:
> TCPM supports BIST carried mode. PD compliance tests require
> BIST Test Data to be supported as well.
> 
> Introducing set_bist_data callback to signal tcpc driver for
> configuring the port controller hardware to enable/disable
> BIST Test Data mode.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

This needs to be the first patch in the series. For the code itself:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Version history:
> Changes since V1:(Guenter's suggestions)
> - Split the change into two: TCPM and TCPCI
> - Move BIST log to TCPM log
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 11 +++++++++++
>  include/linux/usb/tcpm.h      |  2 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 82b19ebd7838e0..379fcab9dbd973 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2746,6 +2746,11 @@ static void tcpm_detach(struct tcpm_port *port)
>  	if (!port->attached)
>  		return;
>  
> +	if (port->tcpc->set_bist_data) {
> +		tcpm_log(port, "disable BIST MODE TESTDATA");
> +		port->tcpc->set_bist_data(port->tcpc, false);
> +	}
> +
>  	if (tcpm_port_is_disconnected(port))
>  		port->hard_reset_count = 0;
>  
> @@ -3555,6 +3560,12 @@ static void run_state_machine(struct tcpm_port *port)
>  		case BDO_MODE_CARRIER2:
>  			tcpm_pd_transmit(port, TCPC_TX_BIST_MODE_2, NULL);
>  			break;
> +		case BDO_MODE_TESTDATA:
> +			if (port->tcpc->set_bist_data) {
> +				tcpm_log(port, "Enable BIST MODE TESTDATA");
> +				port->tcpc->set_bist_data(port->tcpc, true);
> +			}
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index e7979c01c3517c..89f58760cf4800 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -79,6 +79,7 @@ enum tcpm_transmit_type {
>   * @try_role:	Optional; called to set a preferred role
>   * @pd_transmit:Called to transmit PD message
>   * @mux:	Pointer to multiplexer data
> + * @set_bist_data: Turn on/off bist data mode for compliance testing
>   */
>  struct tcpc_dev {
>  	struct fwnode_handle *fwnode;
> @@ -103,6 +104,7 @@ struct tcpc_dev {
>  	int (*try_role)(struct tcpc_dev *dev, int role);
>  	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>  			   const struct pd_message *msg);
> +	int (*set_bist_data)(struct tcpc_dev *dev, bool on);
>  };
>  
>  struct tcpm_port;
> 

Re: [PATCH 3/3 v2] usb: typec: tcpm: Stay in BIST mode till hardreset or unattached

[Guenter Roeck]

On 7/14/20 4:12 PM, Badhri Jagan Sridharan wrote:
> Port starts to toggle when transitioning to unattached state.
> This is incorrect while in BIST mode.
> 
> 6.4.3.1 BIST Carrier Mode
> Upon receipt of a BIST Message, with a BIST Carrier Mode BIST Data Object,
> the UUT Shall send out a continuous string of BMC encoded alternating "1"s
> and “0”s. The UUT Shall exit the Continuous BIST Mode within
> tBISTContMode of this Continuous BIST Mode being enabled(see
> Section 6.6.7.2).
> 
> 6.4.3.2 BIST Test Data
> Upon receipt of a BIST Message, with a BIST Test Data BIST Data Object,
> the UUT Shall return a GoodCRC Message and Shall enter a test mode in which
> it sends no further Messages except for GoodCRC Messages in response to
> received Messages. See Section 5.9.2 for the definition of the Test Data
> Frame. The test Shall be ended by sending Hard Reset Signaling to reset the
> UUT.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
> Version history:
> Changes since V1:
> -  None
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 8 ++++++--
>  include/linux/usb/pd.h        | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 379fcab9dbd973..245cfe80948502 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -3559,6 +3559,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		switch (BDO_MODE_MASK(port->bist_request)) {
>  		case BDO_MODE_CARRIER2:
>  			tcpm_pd_transmit(port, TCPC_TX_BIST_MODE_2, NULL);
> +			tcpm_set_state(port, unattached_state(port),
> +				       PD_T_BIST_CONT_MODE);

One line should now be sufficient.

>  			break;
>  		case BDO_MODE_TESTDATA:
>  			if (port->tcpc->set_bist_data) {
> @@ -3569,8 +3571,6 @@ static void run_state_machine(struct tcpm_port *port)
>  		default:
>  			break;
>  		}
> -		/* Always switch to unattached state */
> -		tcpm_set_state(port, unattached_state(port), 0);
>  		break;
>  	case GET_STATUS_SEND:
>  		tcpm_pd_send_control(port, PD_CTRL_GET_STATUS);
> @@ -3960,6 +3960,10 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
>  static void _tcpm_pd_hard_reset(struct tcpm_port *port)
>  {
>  	tcpm_log_force(port, "Received hard reset");
> +	if (port->bist_request ==  BDO_MODE_TESTDATA &&

Nit: Extra space after "=="

Also, I think this now fits into one line (line length limit is 100).

> +	    port->tcpc->set_bist_data)
> +		port->tcpc->set_bist_data(port->tcpc, false);
> +
>  	/*
>  	 * If we keep receiving hard reset requests, executing the hard reset
>  	 * must have failed. Revert to error recovery if that happens.
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index a665d7f211424d..b420d8d613cd23 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -483,4 +483,5 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
>  #define PD_N_HARD_RESET_COUNT	2
>  
> +#define PD_T_BIST_CONT_MODE	60 /* 30 - 60 ms */

Maybe a bit less to ensure that it is disabled within 60 ms. If we use
the maximum, we may end up having it enabled for more than 60 ms, which
would violate the specification and may tick some picky compliance test
system.

Thanks,
Guenter

>  #endif /* __LINUX_USB_PD_H */
>