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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.