Thread (7 messages) 7 messages, 3 authors, 2021-05-23

Re: [PATCH 1/2] usb: typec: tcpm: Properly interrupt VDM AMS

From: Kyle Tso <hidden>
Date: 2021-05-23 02:03:18
Also in: lkml, oe-kbuild-all

On Sun, May 23, 2021 at 4:23 AM kernel test robot [off-list ref] 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 <redacted>

All warnings (new ones prefixed by >>):
quoted
quoted
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/ (local)

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) {
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help