All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix some VDM AMS handling
@ 2021-05-21 13:01 Kyle Tso
  2021-05-21 13:01 ` [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS Kyle Tso
  2021-05-21 13:01 ` [PATCH 2/2] usb: typec: tcpm: Respond Not_Supported if no snk_vdo Kyle Tso
  0 siblings, 2 replies; 7+ messages in thread
From: Kyle Tso @ 2021-05-21 13:01 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh; +Cc: badhri, linux-usb, linux-kernel, Kyle Tso

usb: typec: tcpm: Properly interrupt VDM AMS
- If VDM AMS is interrupted by Messages other than VDM, the current VDM
  AMS should be finished before handling the just arrived request. I add
  intercept code in the beginning of the handler of the three types of
  requests (control/data/extended) to ensure that the AMS is finished
  first.

usb: typec: tcpm: Respond Not_Supported if no snk_vdo
- The snk_vdo is for the responses to incoming VDM Discover Identity. If
  there is no data in snk_vdo, it means that the port doesn't even
  support it. According to PD3 Spec "Table 6-64 Response to an incoming
  VDM", the port should send Not_Supported Message as the response. For
  PD2 cases, it is defined in PD2 Spec "Table 6-41 Applicability of
  Structured VDM Commands - Note 3" that the port should "Ignore" the
  command.

Kyle Tso (2):
  usb: typec: tcpm: Properly interrupt VDM AMS
  usb: typec: tcpm: Respond Not_Supported if no snk_vdo

 drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS
  2021-05-21 13:01 [PATCH 0/2] Fix some VDM AMS handling Kyle Tso
@ 2021-05-21 13:01 ` Kyle Tso
  2021-05-21 14:48   ` Guenter Roeck
  2021-05-22 20:22   ` kernel test robot
  2021-05-21 13:01 ` [PATCH 2/2] usb: typec: tcpm: Respond Not_Supported if no snk_vdo Kyle Tso
  1 sibling, 2 replies; 7+ messages in thread
From: Kyle Tso @ 2021-05-21 13:01 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh; +Cc: badhri, linux-usb, linux-kernel, Kyle Tso

When a VDM AMS is interrupted by Messages other than VDM, the AMS needs
to be finished properly. Also start a VDM AMS if receiving SVDM Commands
from the port partner to complement the functionality of tcpm_vdm_ams().

Fixes: 0908c5aca31e ("usb: typec: tcpm: AMS and Collision Avoidance")
Signed-off-by: Kyle Tso <kyletso@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 64133e586c64..deb8a9d01f73 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1550,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
 				typec_partner_set_svdm_version(port->partner,
 							       PD_VDO_SVDM_VER(p[0]));
+
+			tcpm_ams_start(port, DISCOVER_IDENTITY);
 			/* 6.4.4.3.1: Only respond as UFP (device) */
 			if (port->data_role == TYPEC_DEVICE &&
 			    port->nr_snk_vdo) {
@@ -1568,14 +1570,19 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			}
 			break;
 		case CMD_DISCOVER_SVID:
+			tcpm_ams_start(port, DISCOVER_SVIDS);
 			break;
 		case CMD_DISCOVER_MODES:
+			tcpm_ams_start(port, DISCOVER_MODES);
 			break;
 		case CMD_ENTER_MODE:
+			tcpm_ams_start(port, DFP_TO_UFP_ENTER_MODE);
 			break;
 		case CMD_EXIT_MODE:
+			tcpm_ams_start(port, DFP_TO_UFP_EXIT_MODE);
 			break;
 		case CMD_ATTENTION:
+			tcpm_ams_start(port, ATTENTION);
 			/* Attention command does not have response */
 			*adev_action = ADEV_ATTENTION;
 			return 0;
@@ -2287,6 +2294,12 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
 	bool frs_enable;
 	int ret;
 
+	if (tcpm_vdm_ams(port) && type != PD_DATA_VENDOR_DEF) {
+		port->vdm_state == VDM_STATE_ERR_BUSY;
+		tcpm_ams_finish(port);
+		mod_vdm_delayed_work(port, 0);
+	}
+
 	switch (type) {
 	case PD_DATA_SOURCE_CAP:
 		for (i = 0; i < cnt; i++)
@@ -2459,6 +2472,16 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 	enum pd_ctrl_msg_type type = pd_header_type_le(msg->header);
 	enum tcpm_state next_state;
 
+	/*
+	 * Stop VDM state machine if interrupted by other Messages while NOT_SUPP is allowed in
+	 * VDM AMS if waiting for VDM responses and will be handled later.
+	 */
+	if (tcpm_vdm_ams(port) && type != PD_CTRL_NOT_SUPP && type != PD_CTRL_GOOD_CRC) {
+		port->vdm_state = VDM_STATE_ERR_BUSY;
+		tcpm_ams_finish(port);
+		mod_vdm_delayed_work(port, 0);
+	}
+
 	switch (type) {
 	case PD_CTRL_GOOD_CRC:
 	case PD_CTRL_PING:
@@ -2717,6 +2740,13 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
 	enum pd_ext_msg_type type = pd_header_type_le(msg->header);
 	unsigned int data_size = pd_ext_header_data_size_le(msg->ext_msg.header);
 
+	/* stopping VDM state machine if interrupted by other Messages */
+	if (tcpm_vdm_ams(port)) {
+		port->vdm_state = VDM_STATE_ERR_BUSY;
+		tcpm_ams_finish(port);
+		mod_vdm_delayed_work(port, 0);
+	}
+
 	if (!(msg->ext_msg.header & PD_EXT_HDR_CHUNKED)) {
 		tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
 		tcpm_log(port, "Unchunked extended messages unsupported");
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] usb: typec: tcpm: Respond Not_Supported if no snk_vdo
  2021-05-21 13:01 [PATCH 0/2] Fix some VDM AMS handling Kyle Tso
  2021-05-21 13:01 ` [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS Kyle Tso
@ 2021-05-21 13:01 ` Kyle Tso
  2021-05-21 14:48   ` Guenter Roeck
  1 sibling, 1 reply; 7+ messages in thread
From: Kyle Tso @ 2021-05-21 13:01 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh; +Cc: badhri, linux-usb, linux-kernel, Kyle Tso

If snk_vdo is not populated from fwnode, it implies the port does not
support responding to SVDM commands. Not_Supported Message shall be sent
if the contract is in PD3. And for PD2, the port shall ignore the
commands.

Fixes: 193a68011fdc ("staging: typec: tcpm: Respond to Discover Identity commands")
Signed-off-by: Kyle Tso <kyletso@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index deb8a9d01f73..d32caa875d9a 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2430,7 +2430,10 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
 					   NONE_AMS);
 		break;
 	case PD_DATA_VENDOR_DEF:
-		tcpm_handle_vdm_request(port, msg->payload, cnt);
+		if (tcpm_vdm_ams(port) || port->nr_snk_vdo)
+			tcpm_handle_vdm_request(port, msg->payload, cnt);
+		else if (port->negotiated_rev > PD_REV20)
+			tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
 		break;
 	case PD_DATA_BIST:
 		port->bist_request = le32_to_cpu(msg->payload[0]);
-- 
2.31.1.818.g46aad6cb9e-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS
  2021-05-21 13:01 ` [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS Kyle Tso
@ 2021-05-21 14:48   ` Guenter Roeck
  2021-05-22 20:22   ` kernel test robot
  1 sibling, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2021-05-21 14:48 UTC (permalink / raw)
  To: Kyle Tso, heikki.krogerus, gregkh; +Cc: badhri, linux-usb, linux-kernel

On 5/21/21 6:01 AM, Kyle Tso wrote:
> When a VDM AMS is interrupted by Messages other than VDM, the AMS needs
> to be finished properly. Also start a VDM AMS if receiving SVDM Commands
> from the port partner to complement the functionality of tcpm_vdm_ams().
> 
> Fixes: 0908c5aca31e ("usb: typec: tcpm: AMS and Collision Avoidance")
> Signed-off-by: Kyle Tso <kyletso@google.com>

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

> ---
>   drivers/usb/typec/tcpm/tcpm.c | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 64133e586c64..deb8a9d01f73 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1550,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>   			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
>   				typec_partner_set_svdm_version(port->partner,
>   							       PD_VDO_SVDM_VER(p[0]));
> +
> +			tcpm_ams_start(port, DISCOVER_IDENTITY);
>   			/* 6.4.4.3.1: Only respond as UFP (device) */
>   			if (port->data_role == TYPEC_DEVICE &&
>   			    port->nr_snk_vdo) {
> @@ -1568,14 +1570,19 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>   			}
>   			break;
>   		case CMD_DISCOVER_SVID:
> +			tcpm_ams_start(port, DISCOVER_SVIDS);
>   			break;
>   		case CMD_DISCOVER_MODES:
> +			tcpm_ams_start(port, DISCOVER_MODES);
>   			break;
>   		case CMD_ENTER_MODE:
> +			tcpm_ams_start(port, DFP_TO_UFP_ENTER_MODE);
>   			break;
>   		case CMD_EXIT_MODE:
> +			tcpm_ams_start(port, DFP_TO_UFP_EXIT_MODE);
>   			break;
>   		case CMD_ATTENTION:
> +			tcpm_ams_start(port, ATTENTION);
>   			/* Attention command does not have response */
>   			*adev_action = ADEV_ATTENTION;
>   			return 0;
> @@ -2287,6 +2294,12 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
>   	bool frs_enable;
>   	int ret;
>   
> +	if (tcpm_vdm_ams(port) && type != PD_DATA_VENDOR_DEF) {
> +		port->vdm_state == VDM_STATE_ERR_BUSY;
> +		tcpm_ams_finish(port);
> +		mod_vdm_delayed_work(port, 0);
> +	}
> +
>   	switch (type) {
>   	case PD_DATA_SOURCE_CAP:
>   		for (i = 0; i < cnt; i++)
> @@ -2459,6 +2472,16 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>   	enum pd_ctrl_msg_type type = pd_header_type_le(msg->header);
>   	enum tcpm_state next_state;
>   
> +	/*
> +	 * Stop VDM state machine if interrupted by other Messages while NOT_SUPP is allowed in
> +	 * VDM AMS if waiting for VDM responses and will be handled later.
> +	 */
> +	if (tcpm_vdm_ams(port) && type != PD_CTRL_NOT_SUPP && type != PD_CTRL_GOOD_CRC) {
> +		port->vdm_state = VDM_STATE_ERR_BUSY;
> +		tcpm_ams_finish(port);
> +		mod_vdm_delayed_work(port, 0);
> +	}
> +
>   	switch (type) {
>   	case PD_CTRL_GOOD_CRC:
>   	case PD_CTRL_PING:
> @@ -2717,6 +2740,13 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
>   	enum pd_ext_msg_type type = pd_header_type_le(msg->header);
>   	unsigned int data_size = pd_ext_header_data_size_le(msg->ext_msg.header);
>   
> +	/* stopping VDM state machine if interrupted by other Messages */
> +	if (tcpm_vdm_ams(port)) {
> +		port->vdm_state = VDM_STATE_ERR_BUSY;
> +		tcpm_ams_finish(port);
> +		mod_vdm_delayed_work(port, 0);
> +	}
> +
>   	if (!(msg->ext_msg.header & PD_EXT_HDR_CHUNKED)) {
>   		tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
>   		tcpm_log(port, "Unchunked extended messages unsupported");
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] usb: typec: tcpm: Respond Not_Supported if no snk_vdo
  2021-05-21 13:01 ` [PATCH 2/2] usb: typec: tcpm: Respond Not_Supported if no snk_vdo Kyle Tso
@ 2021-05-21 14:48   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2021-05-21 14:48 UTC (permalink / raw)
  To: Kyle Tso, heikki.krogerus, gregkh; +Cc: badhri, linux-usb, linux-kernel

On 5/21/21 6:01 AM, Kyle Tso wrote:
> If snk_vdo is not populated from fwnode, it implies the port does not
> support responding to SVDM commands. Not_Supported Message shall be sent
> if the contract is in PD3. And for PD2, the port shall ignore the
> commands.
> 
> Fixes: 193a68011fdc ("staging: typec: tcpm: Respond to Discover Identity commands")
> Signed-off-by: Kyle Tso <kyletso@google.com>

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

> ---
>   drivers/usb/typec/tcpm/tcpm.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index deb8a9d01f73..d32caa875d9a 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2430,7 +2430,10 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
>   					   NONE_AMS);
>   		break;
>   	case PD_DATA_VENDOR_DEF:
> -		tcpm_handle_vdm_request(port, msg->payload, cnt);
> +		if (tcpm_vdm_ams(port) || port->nr_snk_vdo)
> +			tcpm_handle_vdm_request(port, msg->payload, cnt);
> +		else if (port->negotiated_rev > PD_REV20)
> +			tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
>   		break;
>   	case PD_DATA_BIST:
>   		port->bist_request = le32_to_cpu(msg->payload[0]);
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS
  2021-05-21 13:01 ` [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS Kyle Tso
  2021-05-21 14:48   ` Guenter Roeck
@ 2021-05-22 20:22   ` kernel test robot
  2021-05-23  2:02     ` Kyle Tso
  1 sibling, 1 reply; 7+ messages in thread
From: kernel test robot @ 2021-05-22 20:22 UTC (permalink / raw)
  To: Kyle Tso, linux, heikki.krogerus, gregkh
  Cc: kbuild-all, clang-built-linux, badhri, linux-usb, linux-kernel, Kyle Tso

[-- Attachment #1: Type: text/plain, Size: 8718 bytes --]

Hi Kyle,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.13-rc2 next-20210521]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kyle-Tso/Fix-some-VDM-AMS-handling/20210522-205431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-a006-20210523 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/6e05980ae46e87d0a409e1e653658ae6fd7b3a32
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kyle-Tso/Fix-some-VDM-AMS-handling/20210522-205431
        git checkout 6e05980ae46e87d0a409e1e653658ae6fd7b3a32
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/typec/tcpm/tcpm.c:2326:19: warning: equality comparison result unused [-Wunused-comparison]
                   port->vdm_state == VDM_STATE_ERR_BUSY;
                   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
   drivers/usb/typec/tcpm/tcpm.c:2326:19: note: use '=' to turn this equality comparison into an assignment
                   port->vdm_state == VDM_STATE_ERR_BUSY;
                                   ^~
                                   =
   1 warning generated.


vim +2326 drivers/usb/typec/tcpm/tcpm.c

  2313	
  2314	static void tcpm_pd_data_request(struct tcpm_port *port,
  2315					 const struct pd_message *msg)
  2316	{
  2317		enum pd_data_msg_type type = pd_header_type_le(msg->header);
  2318		unsigned int cnt = pd_header_cnt_le(msg->header);
  2319		unsigned int rev = pd_header_rev_le(msg->header);
  2320		unsigned int i;
  2321		enum frs_typec_current partner_frs_current;
  2322		bool frs_enable;
  2323		int ret;
  2324	
  2325		if (tcpm_vdm_ams(port) && type != PD_DATA_VENDOR_DEF) {
> 2326			port->vdm_state == VDM_STATE_ERR_BUSY;
  2327			tcpm_ams_finish(port);
  2328			mod_vdm_delayed_work(port, 0);
  2329		}
  2330	
  2331		switch (type) {
  2332		case PD_DATA_SOURCE_CAP:
  2333			for (i = 0; i < cnt; i++)
  2334				port->source_caps[i] = le32_to_cpu(msg->payload[i]);
  2335	
  2336			port->nr_source_caps = cnt;
  2337	
  2338			tcpm_log_source_caps(port);
  2339	
  2340			tcpm_validate_caps(port, port->source_caps,
  2341					   port->nr_source_caps);
  2342	
  2343			/*
  2344			 * Adjust revision in subsequent message headers, as required,
  2345			 * to comply with 6.2.1.1.5 of the USB PD 3.0 spec. We don't
  2346			 * support Rev 1.0 so just do nothing in that scenario.
  2347			 */
  2348			if (rev == PD_REV10) {
  2349				if (port->ams == GET_SOURCE_CAPABILITIES)
  2350					tcpm_ams_finish(port);
  2351				break;
  2352			}
  2353	
  2354			if (rev < PD_MAX_REV)
  2355				port->negotiated_rev = rev;
  2356	
  2357			if (port->pwr_role == TYPEC_SOURCE) {
  2358				if (port->ams == GET_SOURCE_CAPABILITIES)
  2359					tcpm_pd_handle_state(port, SRC_READY, NONE_AMS, 0);
  2360				/* Unexpected Source Capabilities */
  2361				else
  2362					tcpm_pd_handle_msg(port,
  2363							   port->negotiated_rev < PD_REV30 ?
  2364							   PD_MSG_CTRL_REJECT :
  2365							   PD_MSG_CTRL_NOT_SUPP,
  2366							   NONE_AMS);
  2367			} else if (port->state == SNK_WAIT_CAPABILITIES) {
  2368			/*
  2369			 * This message may be received even if VBUS is not
  2370			 * present. This is quite unexpected; see USB PD
  2371			 * specification, sections 8.3.3.6.3.1 and 8.3.3.6.3.2.
  2372			 * However, at the same time, we must be ready to
  2373			 * receive this message and respond to it 15ms after
  2374			 * receiving PS_RDY during power swap operations, no matter
  2375			 * if VBUS is available or not (USB PD specification,
  2376			 * section 6.5.9.2).
  2377			 * So we need to accept the message either way,
  2378			 * but be prepared to keep waiting for VBUS after it was
  2379			 * handled.
  2380			 */
  2381				port->ams = POWER_NEGOTIATION;
  2382				port->in_ams = true;
  2383				tcpm_set_state(port, SNK_NEGOTIATE_CAPABILITIES, 0);
  2384			} else {
  2385				if (port->ams == GET_SOURCE_CAPABILITIES)
  2386					tcpm_ams_finish(port);
  2387				tcpm_pd_handle_state(port, SNK_NEGOTIATE_CAPABILITIES,
  2388						     POWER_NEGOTIATION, 0);
  2389			}
  2390			break;
  2391		case PD_DATA_REQUEST:
  2392			/*
  2393			 * Adjust revision in subsequent message headers, as required,
  2394			 * to comply with 6.2.1.1.5 of the USB PD 3.0 spec. We don't
  2395			 * support Rev 1.0 so just reject in that scenario.
  2396			 */
  2397			if (rev == PD_REV10) {
  2398				tcpm_pd_handle_msg(port,
  2399						   port->negotiated_rev < PD_REV30 ?
  2400						   PD_MSG_CTRL_REJECT :
  2401						   PD_MSG_CTRL_NOT_SUPP,
  2402						   NONE_AMS);
  2403				break;
  2404			}
  2405	
  2406			if (rev < PD_MAX_REV)
  2407				port->negotiated_rev = rev;
  2408	
  2409			if (port->pwr_role != TYPEC_SOURCE || cnt != 1) {
  2410				tcpm_pd_handle_msg(port,
  2411						   port->negotiated_rev < PD_REV30 ?
  2412						   PD_MSG_CTRL_REJECT :
  2413						   PD_MSG_CTRL_NOT_SUPP,
  2414						   NONE_AMS);
  2415				break;
  2416			}
  2417	
  2418			port->sink_request = le32_to_cpu(msg->payload[0]);
  2419	
  2420			if (port->vdm_sm_running && port->explicit_contract) {
  2421				tcpm_pd_handle_msg(port, PD_MSG_CTRL_WAIT, port->ams);
  2422				break;
  2423			}
  2424	
  2425			if (port->state == SRC_SEND_CAPABILITIES)
  2426				tcpm_set_state(port, SRC_NEGOTIATE_CAPABILITIES, 0);
  2427			else
  2428				tcpm_pd_handle_state(port, SRC_NEGOTIATE_CAPABILITIES,
  2429						     POWER_NEGOTIATION, 0);
  2430			break;
  2431		case PD_DATA_SINK_CAP:
  2432			/* We don't do anything with this at the moment... */
  2433			for (i = 0; i < cnt; i++)
  2434				port->sink_caps[i] = le32_to_cpu(msg->payload[i]);
  2435	
  2436			partner_frs_current = (port->sink_caps[0] & PDO_FIXED_FRS_CURR_MASK) >>
  2437				PDO_FIXED_FRS_CURR_SHIFT;
  2438			frs_enable = partner_frs_current && (partner_frs_current <=
  2439							     port->new_source_frs_current);
  2440			tcpm_log(port,
  2441				 "Port partner FRS capable partner_frs_current:%u port_frs_current:%u enable:%c",
  2442				 partner_frs_current, port->new_source_frs_current, frs_enable ? 'y' : 'n');
  2443			if (frs_enable) {
  2444				ret  = port->tcpc->enable_frs(port->tcpc, true);
  2445				tcpm_log(port, "Enable FRS %s, ret:%d\n", ret ? "fail" : "success", ret);
  2446			}
  2447	
  2448			port->nr_sink_caps = cnt;
  2449			port->sink_cap_done = true;
  2450			if (port->ams == GET_SINK_CAPABILITIES)
  2451				tcpm_set_state(port, ready_state(port), 0);
  2452			/* Unexpected Sink Capabilities */
  2453			else
  2454				tcpm_pd_handle_msg(port,
  2455						   port->negotiated_rev < PD_REV30 ?
  2456						   PD_MSG_CTRL_REJECT :
  2457						   PD_MSG_CTRL_NOT_SUPP,
  2458						   NONE_AMS);
  2459			break;
  2460		case PD_DATA_VENDOR_DEF:
  2461			tcpm_handle_vdm_request(port, msg->payload, cnt);
  2462			break;
  2463		case PD_DATA_BIST:
  2464			port->bist_request = le32_to_cpu(msg->payload[0]);
  2465			tcpm_pd_handle_state(port, BIST_RX, BIST, 0);
  2466			break;
  2467		case PD_DATA_ALERT:
  2468			tcpm_handle_alert(port, msg->payload, cnt);
  2469			break;
  2470		case PD_DATA_BATT_STATUS:
  2471		case PD_DATA_GET_COUNTRY_INFO:
  2472			/* Currently unsupported */
  2473			tcpm_pd_handle_msg(port, port->negotiated_rev < PD_REV30 ?
  2474					   PD_MSG_CTRL_REJECT :
  2475					   PD_MSG_CTRL_NOT_SUPP,
  2476					   NONE_AMS);
  2477			break;
  2478		default:
  2479			tcpm_pd_handle_msg(port, port->negotiated_rev < PD_REV30 ?
  2480					   PD_MSG_CTRL_REJECT :
  2481					   PD_MSG_CTRL_NOT_SUPP,
  2482					   NONE_AMS);
  2483			tcpm_log(port, "Unrecognized data message type %#x", type);
  2484			break;
  2485		}
  2486	}
  2487	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35370 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS
  2021-05-22 20:22   ` kernel test robot
@ 2021-05-23  2:02     ` Kyle Tso
  0 siblings, 0 replies; 7+ messages in thread
From: Kyle Tso @ 2021-05-23  2:02 UTC (permalink / raw)
  To: kernel test robot
  Cc: Guenter Roeck, Heikki Krogerus, Greg KH, kbuild-all,
	clang-built-linux, Badhri Jagan Sridharan, USB, LKML

On Sun, May 23, 2021 at 4:23 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Kyle,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v5.13-rc2 next-20210521]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Kyle-Tso/Fix-some-VDM-AMS-handling/20210522-205431
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> config: x86_64-randconfig-a006-20210523 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install x86_64 cross compiling tool for clang build
>         # apt-get install binutils-x86-64-linux-gnu
>         # https://github.com/0day-ci/linux/commit/6e05980ae46e87d0a409e1e653658ae6fd7b3a32
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Kyle-Tso/Fix-some-VDM-AMS-handling/20210522-205431
>         git checkout 6e05980ae46e87d0a409e1e653658ae6fd7b3a32
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/usb/typec/tcpm/tcpm.c:2326:19: warning: equality comparison result unused [-Wunused-comparison]
>                    port->vdm_state == VDM_STATE_ERR_BUSY;
>                    ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
>    drivers/usb/typec/tcpm/tcpm.c:2326:19: note: use '=' to turn this equality comparison into an assignment
>                    port->vdm_state == VDM_STATE_ERR_BUSY;
>                                    ^~
>                                    =
>    1 warning generated.
>
>

Fixed in v2
https://lore.kernel.org/linux-usb/20210523015855.1785484-1-kyletso@google.com/

thanks,
Kyle

> vim +2326 drivers/usb/typec/tcpm/tcpm.c
>
>   2313
>   2314  static void tcpm_pd_data_request(struct tcpm_port *port,
>   2315                                   const struct pd_message *msg)
>   2316  {
>   2317          enum pd_data_msg_type type = pd_header_type_le(msg->header);
>   2318          unsigned int cnt = pd_header_cnt_le(msg->header);
>   2319          unsigned int rev = pd_header_rev_le(msg->header);
>   2320          unsigned int i;
>   2321          enum frs_typec_current partner_frs_current;
>   2322          bool frs_enable;
>   2323          int ret;
>   2324
>   2325          if (tcpm_vdm_ams(port) && type != PD_DATA_VENDOR_DEF) {
> > 2326                  port->vdm_state == VDM_STATE_ERR_BUSY;
>   2327                  tcpm_ams_finish(port);
>   2328                  mod_vdm_delayed_work(port, 0);
>   2329          }
>   2330
>   2331          switch (type) {
>   2332          case PD_DATA_SOURCE_CAP:
>   2333                  for (i = 0; i < cnt; i++)
>   2334                          port->source_caps[i] = le32_to_cpu(msg->payload[i]);
>   2335
>   2336                  port->nr_source_caps = cnt;
>   2337
>   2338                  tcpm_log_source_caps(port);
>   2339
>   2340                  tcpm_validate_caps(port, port->source_caps,
>   2341                                     port->nr_source_caps);
>   2342
>   2343                  /*
>   2344                   * Adjust revision in subsequent message headers, as required,
>   2345                   * to comply with 6.2.1.1.5 of the USB PD 3.0 spec. We don't
>   2346                   * support Rev 1.0 so just do nothing in that scenario.
>   2347                   */
>   2348                  if (rev == PD_REV10) {
>   2349                          if (port->ams == GET_SOURCE_CAPABILITIES)
>   2350                                  tcpm_ams_finish(port);
>   2351                          break;
>   2352                  }
>   2353
>   2354                  if (rev < PD_MAX_REV)
>   2355                          port->negotiated_rev = rev;
>   2356
>   2357                  if (port->pwr_role == TYPEC_SOURCE) {
>   2358                          if (port->ams == GET_SOURCE_CAPABILITIES)
>   2359                                  tcpm_pd_handle_state(port, SRC_READY, NONE_AMS, 0);
>   2360                          /* Unexpected Source Capabilities */
>   2361                          else
>   2362                                  tcpm_pd_handle_msg(port,
>   2363                                                     port->negotiated_rev < PD_REV30 ?
>   2364                                                     PD_MSG_CTRL_REJECT :
>   2365                                                     PD_MSG_CTRL_NOT_SUPP,
>   2366                                                     NONE_AMS);
>   2367                  } else if (port->state == SNK_WAIT_CAPABILITIES) {
>   2368                  /*
>   2369                   * This message may be received even if VBUS is not
>   2370                   * present. This is quite unexpected; see USB PD
>   2371                   * specification, sections 8.3.3.6.3.1 and 8.3.3.6.3.2.
>   2372                   * However, at the same time, we must be ready to
>   2373                   * receive this message and respond to it 15ms after
>   2374                   * receiving PS_RDY during power swap operations, no matter
>   2375                   * if VBUS is available or not (USB PD specification,
>   2376                   * section 6.5.9.2).
>   2377                   * So we need to accept the message either way,
>   2378                   * but be prepared to keep waiting for VBUS after it was
>   2379                   * handled.
>   2380                   */
>   2381                          port->ams = POWER_NEGOTIATION;
>   2382                          port->in_ams = true;
>   2383                          tcpm_set_state(port, SNK_NEGOTIATE_CAPABILITIES, 0);
>   2384                  } else {
>   2385                          if (port->ams == GET_SOURCE_CAPABILITIES)
>   2386                                  tcpm_ams_finish(port);
>   2387                          tcpm_pd_handle_state(port, SNK_NEGOTIATE_CAPABILITIES,
>   2388                                               POWER_NEGOTIATION, 0);
>   2389                  }
>   2390                  break;
>   2391          case PD_DATA_REQUEST:
>   2392                  /*
>   2393                   * Adjust revision in subsequent message headers, as required,
>   2394                   * to comply with 6.2.1.1.5 of the USB PD 3.0 spec. We don't
>   2395                   * support Rev 1.0 so just reject in that scenario.
>   2396                   */
>   2397                  if (rev == PD_REV10) {
>   2398                          tcpm_pd_handle_msg(port,
>   2399                                             port->negotiated_rev < PD_REV30 ?
>   2400                                             PD_MSG_CTRL_REJECT :
>   2401                                             PD_MSG_CTRL_NOT_SUPP,
>   2402                                             NONE_AMS);
>   2403                          break;
>   2404                  }
>   2405
>   2406                  if (rev < PD_MAX_REV)
>   2407                          port->negotiated_rev = rev;
>   2408
>   2409                  if (port->pwr_role != TYPEC_SOURCE || cnt != 1) {
>   2410                          tcpm_pd_handle_msg(port,
>   2411                                             port->negotiated_rev < PD_REV30 ?
>   2412                                             PD_MSG_CTRL_REJECT :
>   2413                                             PD_MSG_CTRL_NOT_SUPP,
>   2414                                             NONE_AMS);
>   2415                          break;
>   2416                  }
>   2417
>   2418                  port->sink_request = le32_to_cpu(msg->payload[0]);
>   2419
>   2420                  if (port->vdm_sm_running && port->explicit_contract) {
>   2421                          tcpm_pd_handle_msg(port, PD_MSG_CTRL_WAIT, port->ams);
>   2422                          break;
>   2423                  }
>   2424
>   2425                  if (port->state == SRC_SEND_CAPABILITIES)
>   2426                          tcpm_set_state(port, SRC_NEGOTIATE_CAPABILITIES, 0);
>   2427                  else
>   2428                          tcpm_pd_handle_state(port, SRC_NEGOTIATE_CAPABILITIES,
>   2429                                               POWER_NEGOTIATION, 0);
>   2430                  break;
>   2431          case PD_DATA_SINK_CAP:
>   2432                  /* We don't do anything with this at the moment... */
>   2433                  for (i = 0; i < cnt; i++)
>   2434                          port->sink_caps[i] = le32_to_cpu(msg->payload[i]);
>   2435
>   2436                  partner_frs_current = (port->sink_caps[0] & PDO_FIXED_FRS_CURR_MASK) >>
>   2437                          PDO_FIXED_FRS_CURR_SHIFT;
>   2438                  frs_enable = partner_frs_current && (partner_frs_current <=
>   2439                                                       port->new_source_frs_current);
>   2440                  tcpm_log(port,
>   2441                           "Port partner FRS capable partner_frs_current:%u port_frs_current:%u enable:%c",
>   2442                           partner_frs_current, port->new_source_frs_current, frs_enable ? 'y' : 'n');
>   2443                  if (frs_enable) {
>   2444                          ret  = port->tcpc->enable_frs(port->tcpc, true);
>   2445                          tcpm_log(port, "Enable FRS %s, ret:%d\n", ret ? "fail" : "success", ret);
>   2446                  }
>   2447
>   2448                  port->nr_sink_caps = cnt;
>   2449                  port->sink_cap_done = true;
>   2450                  if (port->ams == GET_SINK_CAPABILITIES)
>   2451                          tcpm_set_state(port, ready_state(port), 0);
>   2452                  /* Unexpected Sink Capabilities */
>   2453                  else
>   2454                          tcpm_pd_handle_msg(port,
>   2455                                             port->negotiated_rev < PD_REV30 ?
>   2456                                             PD_MSG_CTRL_REJECT :
>   2457                                             PD_MSG_CTRL_NOT_SUPP,
>   2458                                             NONE_AMS);
>   2459                  break;
>   2460          case PD_DATA_VENDOR_DEF:
>   2461                  tcpm_handle_vdm_request(port, msg->payload, cnt);
>   2462                  break;
>   2463          case PD_DATA_BIST:
>   2464                  port->bist_request = le32_to_cpu(msg->payload[0]);
>   2465                  tcpm_pd_handle_state(port, BIST_RX, BIST, 0);
>   2466                  break;
>   2467          case PD_DATA_ALERT:
>   2468                  tcpm_handle_alert(port, msg->payload, cnt);
>   2469                  break;
>   2470          case PD_DATA_BATT_STATUS:
>   2471          case PD_DATA_GET_COUNTRY_INFO:
>   2472                  /* Currently unsupported */
>   2473                  tcpm_pd_handle_msg(port, port->negotiated_rev < PD_REV30 ?
>   2474                                     PD_MSG_CTRL_REJECT :
>   2475                                     PD_MSG_CTRL_NOT_SUPP,
>   2476                                     NONE_AMS);
>   2477                  break;
>   2478          default:
>   2479                  tcpm_pd_handle_msg(port, port->negotiated_rev < PD_REV30 ?
>   2480                                     PD_MSG_CTRL_REJECT :
>   2481                                     PD_MSG_CTRL_NOT_SUPP,
>   2482                                     NONE_AMS);
>   2483                  tcpm_log(port, "Unrecognized data message type %#x", type);
>   2484                  break;
>   2485          }
>   2486  }
>   2487
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-23  2:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21 13:01 [PATCH 0/2] Fix some VDM AMS handling Kyle Tso
2021-05-21 13:01 ` [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS Kyle Tso
2021-05-21 14:48   ` Guenter Roeck
2021-05-22 20:22   ` kernel test robot
2021-05-23  2:02     ` Kyle Tso
2021-05-21 13:01 ` [PATCH 2/2] usb: typec: tcpm: Respond Not_Supported if no snk_vdo Kyle Tso
2021-05-21 14:48   ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.