Re: [PATCH v1 06/11] spi: dw: send cmd and addr to start the spi transfer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Incomplete implementation of enhanced SPI memory operations causes data corruption.
- [Critical] Divide-by-zero kernel panic when calculating wait cycles for dummy-less SPI memory operations.
--

--- Patch [6]: [PATCH v1 06/11] spi: dw: send cmd and addr to start the spi transfer ---
commit 81aa9a8a81c58018d2abaeeaaa7a7b9aed04510d
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: send cmd and addr to start the spi transfer

In enhanced spi mode, read or write will start by sending the cmd
and address (if present).

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 58159e8269f53..6a52860654280 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c

[ ... ]

> @@ -895,6 +908,8 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
>  
>  	dw_spi_enable_chip(dws, 1);
>  
> +	dw_spi_enh_write_cmd_addr(dws, op);
> +
>  	return 0;
>  }

[Severity: Critical]
Does this code return 0 (success) without ever transferring data (e.g., via
polling or DMA) or tearing down the transfer?

The dw_spi_exec_enh_mem_op() function configures the SPI controller, enables
the chip, and writes the command and address, but returns 0 immediately. It
omits the data transfer phase (writing outbound data to or reading inbound
data from the SPI registers).

Does this skip waiting for the operation to complete or disabling the chip
select, leaving caller buffers uninitialized on reads or writes silently
dropped?

[Severity: Critical]
When calculating wait cycles for dummy-less SPI memory operations in
dw_spi_exec_enh_mem_op(), could this cause a divide-by-zero regression?

If an enhanced SPI memory operation is executed without any dummy cycles
(op->dummy.nbytes == 0), op->dummy.buswidth is initialized to 0.

Earlier in dw_spi_exec_enh_mem_op(), enh_cfg.wait_c is calculated as:

    enh_cfg.wait_c = (op->dummy.nbytes * (BITS_PER_BYTE / op->dummy.buswidth));

Due to the explicit parentheses, does the division evaluate first and
unconditionally crash the kernel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709055204.138168-1-changhuang.liang@starfivetech.com?part=6