From: Dan Carpenter <dan.carpenter@oracle.com>
To: paul.kocialkowski@bootlin.com
Cc: linux-spi@vger.kernel.org
Subject: [bug report] spi: core: Only check bits_per_word validity when explicitly provided
Date: Thu, 14 Apr 2022 09:49:48 +0300 [thread overview]
Message-ID: <20220414064948.GA9873@kili> (raw)
Hello Paul Kocialkowski,
The patch b3fe2e516741: "spi: core: Only check bits_per_word validity
when explicitly provided" from Apr 12, 2022, leads to the following
Smatch static checker warning:
drivers/spi/spi.c:3583 spi_setup()
error: uninitialized symbol 'status'.
drivers/spi/spi.c
3475 int spi_setup(struct spi_device *spi)
3476 {
3477 unsigned bad_bits, ugly_bits;
3478 int status;
3479
3480 /*
3481 * Check mode to prevent that any two of DUAL, QUAD and NO_MOSI/MISO
3482 * are set at the same time.
3483 */
3484 if ((hweight_long(spi->mode &
3485 (SPI_TX_DUAL | SPI_TX_QUAD | SPI_NO_TX)) > 1) ||
3486 (hweight_long(spi->mode &
3487 (SPI_RX_DUAL | SPI_RX_QUAD | SPI_NO_RX)) > 1)) {
3488 dev_err(&spi->dev,
3489 "setup: can not select any two of dual, quad and no-rx/tx at the same time\n");
3490 return -EINVAL;
3491 }
3492 /* If it is SPI_3WIRE mode, DUAL and QUAD should be forbidden */
3493 if ((spi->mode & SPI_3WIRE) && (spi->mode &
3494 (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL |
3495 SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL)))
3496 return -EINVAL;
3497 /*
3498 * Help drivers fail *cleanly* when they need options
3499 * that aren't supported with their current controller.
3500 * SPI_CS_WORD has a fallback software implementation,
3501 * so it is ignored here.
3502 */
3503 bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD |
3504 SPI_NO_TX | SPI_NO_RX);
3505 ugly_bits = bad_bits &
3506 (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL |
3507 SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL);
3508 if (ugly_bits) {
3509 dev_warn(&spi->dev,
3510 "setup: ignoring unsupported mode bits %x\n",
3511 ugly_bits);
3512 spi->mode &= ~ugly_bits;
3513 bad_bits &= ~ugly_bits;
3514 }
3515 if (bad_bits) {
3516 dev_err(&spi->dev, "setup: unsupported mode bits %x\n",
3517 bad_bits);
3518 return -EINVAL;
3519 }
3520
3521 if (!spi->bits_per_word) {
3522 spi->bits_per_word = 8;
"status" used to be set here, for sure. You would have to be pretty
unlucky to get through this function without setting status at all.
3523 } else {
3524 /*
3525 * Some controllers may not support the default 8 bits-per-word
3526 * so only perform the check when this is explicitly provided.
3527 */
3528 status = __spi_validate_bits_per_word(spi->controller,
3529 spi->bits_per_word);
3530 if (status)
3531 return status;
3532 }
3533
3534 if (spi->controller->max_speed_hz &&
3535 (!spi->max_speed_hz ||
3536 spi->max_speed_hz > spi->controller->max_speed_hz))
3537 spi->max_speed_hz = spi->controller->max_speed_hz;
3538
3539 mutex_lock(&spi->controller->io_mutex);
3540
3541 if (spi->controller->setup) {
3542 status = spi->controller->setup(spi);
3543 if (status) {
3544 mutex_unlock(&spi->controller->io_mutex);
3545 dev_err(&spi->controller->dev, "Failed to setup device: %d\n",
3546 status);
3547 return status;
3548 }
3549 }
3550
3551 if (spi->controller->auto_runtime_pm && spi->controller->set_cs) {
3552 status = pm_runtime_get_sync(spi->controller->dev.parent);
3553 if (status < 0) {
3554 mutex_unlock(&spi->controller->io_mutex);
3555 pm_runtime_put_noidle(spi->controller->dev.parent);
3556 dev_err(&spi->controller->dev, "Failed to power device: %d\n",
3557 status);
3558 return status;
3559 }
3560
3561 /*
3562 * We do not want to return positive value from pm_runtime_get,
3563 * there are many instances of devices calling spi_setup() and
3564 * checking for a non-zero return value instead of a negative
3565 * return value.
3566 */
3567 status = 0;
3568
3569 spi_set_cs(spi, false, true);
3570 pm_runtime_mark_last_busy(spi->controller->dev.parent);
3571 pm_runtime_put_autosuspend(spi->controller->dev.parent);
3572 } else {
3573 spi_set_cs(spi, false, true);
3574 }
3575
3576 mutex_unlock(&spi->controller->io_mutex);
3577
3578 if (spi->rt && !spi->controller->rt) {
3579 spi->controller->rt = true;
3580 spi_set_thread_rt(spi->controller);
3581 }
3582
3583 trace_spi_setup(spi, status);
3584
3585 dev_dbg(&spi->dev, "setup mode %lu, %s%s%s%s%u bits/w, %u Hz max --> %d\n",
3586 spi->mode & SPI_MODE_X_MASK,
3587 (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
3588 (spi->mode & SPI_LSB_FIRST) ? "lsb, " : "",
3589 (spi->mode & SPI_3WIRE) ? "3wire, " : "",
3590 (spi->mode & SPI_LOOP) ? "loopback, " : "",
3591 spi->bits_per_word, spi->max_speed_hz,
3592 status);
3593
3594 return status;
3595 }
regards,
dan carpenter
next reply other threads:[~2022-04-14 6:50 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-14 6:49 Dan Carpenter [this message]
2022-04-14 7:04 ` [bug report] spi: core: Only check bits_per_word validity when explicitly provided Paul Kocialkowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220414064948.GA9873@kili \
--to=dan.carpenter@oracle.com \
--cc=linux-spi@vger.kernel.org \
--cc=paul.kocialkowski@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox