[bug report] spi: core: Only check bits_per_word validity when explicitly provided

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

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