From 709b5be0a48d4e5bb95ef0b107c56045a8fe67c6 Mon Sep 17 00:00:00 2001 From: Ramin Moussavi Date: Tue, 22 Jul 2025 20:27:09 +0200 Subject: [PATCH 1/6] spi: atmel_qspi: fix race condition in transfer completion check In atmel_qspi_transfer(), the status register is polled with: imr = QSPI_SR_INSTRE | QSPI_SR_CSR; return readl_poll_timeout(aq->regs + QSPI_SR, sr, (sr & imr) == imr, ATMEL_QSPI_TIMEOUT); However, this is racy: QSPI_SR_INSTRE can be set before QSPI_SR_CSR, and will then be cleared by the read. If that happens, the condition "(sr & imr) == imr" can never be true, and the function times out. This race condition is avoided in at91bootstrap by accumulating the status bits across reads until both bits have been observed: /* Poll INSTruction End and Chip Select Rise flags. */ imr = (QSPI_SR_INSTRE | QSPI_SR_CSR); sr = 0; do { udelay(1); sr |= qspi_readl(qspi, QSPI_SR) & imr; } while ((--timeout) && (sr != imr)); Update U-Boot's atmel_qspi_transfer() to use the same pattern, ensuring that both flags are observed even if they are not set simultaneously. Signed-off-by: Ramin Moussavi [eugen.hristev@linaro.org: remove 'sr' and fix commit msg] Signed-off-by: Eugen Hristev --- drivers/spi/atmel-quadspi.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c index 8aa7a83aef4..b2b96e1c4b9 100644 --- a/drivers/spi/atmel-quadspi.c +++ b/drivers/spi/atmel-quadspi.c @@ -615,7 +615,8 @@ static int atmel_qspi_set_cfg(struct atmel_qspi *aq, static int atmel_qspi_transfer(struct atmel_qspi *aq, const struct spi_mem_op *op, u32 offset) { - u32 sr, imr; + u32 imr, val = 0; + unsigned long timeout; /* Skip to the final steps if there is no data */ if (op->data.nbytes) { @@ -636,8 +637,16 @@ static int atmel_qspi_transfer(struct atmel_qspi *aq, /* Poll INSTruction End and Chip Select Rise flags. */ imr = QSPI_SR_INSTRE | QSPI_SR_CSR; - return readl_poll_timeout(aq->regs + QSPI_SR, sr, (sr & imr) == imr, - ATMEL_QSPI_TIMEOUT); + + timeout = timer_get_us() + ATMEL_QSPI_TIMEOUT; + while (1) { + val |= readl(aq->regs + QSPI_SR) & imr; + if ((val & imr) == imr) + return 0; + + if (time_after(timer_get_us(), timeout)) + return -ETIMEDOUT; + } } static int atmel_qspi_sama7g5_set_cfg(struct atmel_qspi *aq, From 29ea990a1c4a2455f432d5e71b217e93b406fc12 Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Wed, 23 Jul 2025 15:13:48 +0100 Subject: [PATCH 2/6] clk: at91: Fix testing of unsigned variable to be negative The variable 'index' is declared as unsigned but used to receive the return value of a function returning 'int'. This value is then tested for being less than zero to detect an error condition but as index is unsigned this can never be true. Change the variable 'index' to be an int so that the error condition can be detected. This issue was found by Smatch. Signed-off-by: Andrew Goodbody --- drivers/clk/at91/clk-main.c | 3 ++- drivers/clk/at91/clk-master.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/clk/at91/clk-main.c b/drivers/clk/at91/clk-main.c index a5186f885f0..0542b066788 100644 --- a/drivers/clk/at91/clk-main.c +++ b/drivers/clk/at91/clk-main.c @@ -315,7 +315,8 @@ static int clk_sam9x5_main_set_parent(struct clk *clk, struct clk *parent) { struct clk_main *main = to_clk_main(clk); void __iomem *reg = main->reg; - unsigned int tmp, index; + unsigned int tmp; + int index; index = at91_clk_mux_val_to_index(main->clk_mux_table, main->num_parents, AT91_CLK_ID_TO_DID(parent->id)); diff --git a/drivers/clk/at91/clk-master.c b/drivers/clk/at91/clk-master.c index cdc5271fa83..530205b8c6b 100644 --- a/drivers/clk/at91/clk-master.c +++ b/drivers/clk/at91/clk-master.c @@ -335,8 +335,8 @@ struct clk *at91_clk_sama7g5_register_master(void __iomem *base, { struct clk_master *master; struct clk *clk; - u32 val, index; - int ret; + u32 val; + int ret, index; if (!base || !name || !num_parents || !parent_names || !mux_table || !clk_mux_table || id > MASTER_MAX_ID) From da13ce8a6b1a37c49d2215ef78ee7984bae1c068 Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Wed, 23 Jul 2025 15:13:49 +0100 Subject: [PATCH 3/6] clk: at91: Fix use of unsigned loop index The use of the unsigned variable 'i' as a loop index leads to the test for i being non-negative always being true. Instead declare 'i' as an int so that the for loop will terminate as expected. If the original for loop completes 'i' will be 1 past the end of the array so decrement it in the subsequent error path to prevent an out of bounds access occurring. This issue was found by Smatch. Signed-off-by: Andrew Goodbody --- drivers/clk/at91/sckc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c index 6d6f12578db..3fde8ea7138 100644 --- a/drivers/clk/at91/sckc.c +++ b/drivers/clk/at91/sckc.c @@ -74,8 +74,8 @@ static struct clk *at91_sam9x60_clk_register_td_slck(struct sam9x60_sckc *sckc, int num_parents) { struct clk *clk; - int ret = -ENOMEM; - u32 val, i; + int ret = -ENOMEM, i; + u32 val; if (!sckc || !name || !parent_names || num_parents != 2) return ERR_PTR(-EINVAL); @@ -99,8 +99,10 @@ static struct clk *at91_sam9x60_clk_register_td_slck(struct sam9x60_sckc *sckc, clk = &sckc->clk; ret = clk_register(clk, UBOOT_DM_CLK_AT91_SAM9X60_TD_SLCK, name, parent_names[val]); - if (ret) + if (ret) { + i--; goto free; + } return clk; From b824136ab4f743e09c81af531d430dbe6a2d92bf Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Mon, 28 Jul 2025 17:12:13 +0100 Subject: [PATCH 4/6] mfd: atmel-smc: Ensure match is initialised If the test in the for loop is never matched then the variable 'match' will never be assigned to. Provide an initial value so this cannot be a problem. This issue was found by Smatch. Signed-off-by: Andrew Goodbody --- drivers/mfd/atmel-smc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mfd/atmel-smc.c b/drivers/mfd/atmel-smc.c index 15296f71a16..94e5e7b67a0 100644 --- a/drivers/mfd/atmel-smc.c +++ b/drivers/mfd/atmel-smc.c @@ -347,7 +347,7 @@ const struct atmel_hsmc_reg_layout * atmel_hsmc_get_reg_layout(ofnode np) { int i; - const struct udevice_id *match; + const struct udevice_id *match = NULL; const char *name; int len; From 4fb189a58eacd8bac267f2f330b41e7700bcfa12 Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Tue, 29 Jul 2025 17:16:16 +0100 Subject: [PATCH 5/6] mmc: gen_atmel_mci: Remove duplicate checks Remove duplicate checks on status from mci_data_read and mci_data_write which are guaranteed to be true as exiting the above do..while loop above requires that to be so. This issue was found by Smatch. Signed-off-by: Andrew Goodbody Reviewed-by: Peng Fan --- drivers/mmc/gen_atmel_mci.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index 6a531fa0961..62aa6b3cb0b 100644 --- a/drivers/mmc/gen_atmel_mci.c +++ b/drivers/mmc/gen_atmel_mci.c @@ -206,10 +206,9 @@ static u32 mci_data_read(atmel_mci_t *mci, u32* data, u32 error_flags) goto io_fail; } while (!(status & MMCI_BIT(RXRDY))); - if (status & MMCI_BIT(RXRDY)) { - *data = readl(&mci->rdr); - status = 0; - } + *data = readl(&mci->rdr); + status = 0; + io_fail: return status; } @@ -225,10 +224,9 @@ static u32 mci_data_write(atmel_mci_t *mci, u32* data, u32 error_flags) goto io_fail; } while (!(status & MMCI_BIT(TXRDY))); - if (status & MMCI_BIT(TXRDY)) { - writel(*data, &mci->tdr); - status = 0; - } + writel(*data, &mci->tdr); + status = 0; + io_fail: return status; } From c1168f99387b40f5e3323ff16c670427c2c8a66e Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Tue, 29 Jul 2025 17:16:17 +0100 Subject: [PATCH 6/6] mmc: gen_atmel_mci: NULL check variable before use In mci_send_cmd the pointer 'data' is optional so guard its use with a NULL check to prevent any attempt to dereference it when not provided. This issue was found by Smatch. Signed-off-by: Andrew Goodbody Reviewed-by: Peng Fan --- drivers/mmc/gen_atmel_mci.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/gen_atmel_mci.c b/drivers/mmc/gen_atmel_mci.c index 62aa6b3cb0b..838b4f4a65a 100644 --- a/drivers/mmc/gen_atmel_mci.c +++ b/drivers/mmc/gen_atmel_mci.c @@ -263,13 +263,15 @@ mci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) /* Figure out the transfer arguments */ cmdr = mci_encode_cmd(cmd, data, &error_flags); - mci_set_blklen(mci, data->blocksize); + if (data) { + mci_set_blklen(mci, data->blocksize); - /* For multi blocks read/write, set the block register */ - if ((cmd->cmdidx == MMC_CMD_READ_MULTIPLE_BLOCK) - || (cmd->cmdidx == MMC_CMD_WRITE_MULTIPLE_BLOCK)) - writel(data->blocks | MMCI_BF(BLKLEN, data->blocksize), - &mci->blkr); + /* For multi blocks read/write, set the block register */ + if (cmd->cmdidx == MMC_CMD_READ_MULTIPLE_BLOCK || + cmd->cmdidx == MMC_CMD_WRITE_MULTIPLE_BLOCK) + writel(data->blocks | MMCI_BF(BLKLEN, data->blocksize), + &mci->blkr); + } /* Send the command */ writel(cmd->cmdarg, &mci->argr);