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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> 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]);
>
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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> 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");
>
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 <[email protected]>
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/[email protected]
On Sun, May 23, 2021 at 4:23 AM kernel test robot <[email protected]> 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 <[email protected]>
>
> 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/[email protected]/
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/[email protected]