Hi Dan On Wed, May 15, 2019 at 12:08:46AM +0300, Dan Carpenter wrote: > Hello Rushikesh S Kadam, > > The patch 4f346361cf42: "platform/chrome: Add ChromeOS EC ISHTP > driver" from May 4, 2019, leads to the following static checker > warning: > > drivers/platform/chrome/cros_ec_ishtp.c:527 cros_ec_pkt_xfer_ish() > warn: inconsistent returns 'read_sem:&init_lock'. > Locked on: line 475 > line 482 > Unlocked on: line 467 > line 527 > > drivers/platform/chrome/cros_ec_ishtp.c > 450 static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev, > 451 struct cros_ec_command *msg) > 452 { > 453 int rv; > 454 struct ishtp_cl *cros_ish_cl = ec_dev->priv; > 455 struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl); > 456 struct device *dev = cl_data_to_dev(client_data); > 457 struct cros_ish_in_msg *in_msg = (struct cros_ish_in_msg *)ec_dev->din; > 458 struct cros_ish_out_msg *out_msg = > 459 (struct cros_ish_out_msg *)ec_dev->dout; > 460 size_t in_size = sizeof(struct cros_ish_in_msg) + msg->insize; > 461 size_t out_size = sizeof(struct cros_ish_out_msg) + msg->outsize; > 462 > 463 /* Proceed only if reset-init is not in progress */ > 464 if (!down_read_trylock(&init_lock)) { > 465 dev_warn(dev, > 466 "Host is not ready to send messages to ISH. Try again\n"); > 467 return -EAGAIN; > 468 } > 469 > 470 /* Sanity checks */ > 471 if (in_size > ec_dev->din_size) { > 472 dev_err(dev, > 473 "Incoming payload size %zu is too large for ec_dev->din_size %d\n", > 474 in_size, ec_dev->din_size); > 475 return -EMSGSIZE; > ^^^^^^^^^^^^^^^^^ > 476 } > 477 > 478 if (out_size > ec_dev->dout_size) { > 479 dev_err(dev, > 480 "Outgoing payload size %zu is too large for ec_dev->dout_size %d\n", > 481 out_size, ec_dev->dout_size); > 482 return -EMSGSIZE; > ^^^^^^^^^^^^^^^^^ > Could we move these sanity checks to before we take the lock? Otherwise > we need to goto end_error; Thanks for reporting. I submitted updated patch with fix here https://lkml.org/lkml/2019/5/15/263 Regards Rushikesh > > 483 } > 484 > 485 /* Prepare the package to be sent over ISH TP */ > 486 out_msg->hdr.channel = CROS_EC_COMMAND; > 487 out_msg->hdr.status = 0; > 488 > 489 ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE; > 490 cros_ec_prepare_tx(ec_dev, msg); > 491 ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE; > 492 > 493 dev_dbg(dev, > 494 "out_msg: struct_ver=0x%x checksum=0x%x command=0x%x command_ver=0x%x data_len=0x%x\n", > 495 out_msg->ec_request.struct_version, > 496 out_msg->ec_request.checksum, > 497 out_msg->ec_request.command, > 498 out_msg->ec_request.command_version, > 499 out_msg->ec_request.data_len); > 500 > 501 /* Send command to ISH EC firmware and read response */ > 502 rv = ish_send(client_data, > 503 (u8 *)out_msg, out_size, > 504 (u8 *)in_msg, in_size); > 505 if (rv < 0) > 506 goto end_error; > 507 > 508 rv = prepare_cros_ec_rx(ec_dev, in_msg, msg); > 509 if (rv) > 510 goto end_error; > 511 > 512 rv = in_msg->ec_response.data_len; > 513 > 514 dev_dbg(dev, > 515 "in_msg: struct_ver=0x%x checksum=0x%x result=0x%x data_len=0x%x\n", > 516 in_msg->ec_response.struct_version, > 517 in_msg->ec_response.checksum, > 518 in_msg->ec_response.result, > 519 in_msg->ec_response.data_len); > 520 > 521 end_error: > 522 if (msg->command == EC_CMD_REBOOT_EC) > 523 msleep(EC_REBOOT_DELAY_MS); > 524 > 525 up_read(&init_lock); > 526 > 527 return rv; > 528 } > > regards, > dan carpenter --