* [bug report] platform/chrome: Add ChromeOS EC ISHTP driver
@ 2019-05-14 21:08 Dan Carpenter
2019-05-15 10:56 ` Rushikesh S Kadam
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-05-14 21:08 UTC (permalink / raw)
To: kernel-janitors
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;
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] platform/chrome: Add ChromeOS EC ISHTP driver
2019-05-14 21:08 [bug report] platform/chrome: Add ChromeOS EC ISHTP driver Dan Carpenter
@ 2019-05-15 10:56 ` Rushikesh S Kadam
0 siblings, 0 replies; 2+ messages in thread
From: Rushikesh S Kadam @ 2019-05-15 10:56 UTC (permalink / raw)
To: kernel-janitors
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
--
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-05-15 10:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-14 21:08 [bug report] platform/chrome: Add ChromeOS EC ISHTP driver Dan Carpenter
2019-05-15 10:56 ` Rushikesh S Kadam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox