Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Input: edt-ft5x06 - unaligned DMA rx buffer sharing cacheline with tx --
commit 6c4cc2125db365d716cb624219a2c7fd67fb674b Author: Luka Panio <[email protected]> Input: edt-ft5x06 - add SPI transport and FT8722 support This commit adds an SPI transport driver for FocalTech touchscreen controllers handled by the edt-ft5x06 driver, along with firmware download support for the RAM-only FT8722 controller. > diff --git a/drivers/input/touchscreen/edt-ft5x06-spi.c > b/drivers/input/touchscreen/edt-ft5x06-spi.c > new file mode 100644 > index 0000000000000..d0ae0d4b621dd > --- /dev/null > +++ b/drivers/input/touchscreen/edt-ft5x06-spi.c > @@ -0,0 +1,475 @@ [ ... ] > +static int fts_spi_xfer(struct spi_device *spi, const u8 *tx, u8 *rx, > + size_t len) > +{ > + struct spi_transfer xfer = { > + .tx_buf = tx, > + .rx_buf = rx, > + .len = len, > + }; > + int ret; > + > + ret = spi_sync_transfer(spi, &xfer, 1); > + fsleep(FTS_SPI_CS_HIGH_US); > + > + return ret; > +} > + > +static int fts_spi_write(struct spi_device *spi, u8 reg, const void *val, > + size_t len) > +{ > + size_t xlen = FTS_SPI_HEADER_LEN + (len ? FTS_SPI_DUMMY_LEN + len : 0); > + u8 *tx, *rx; > + int ret, i; > + > + if (len > U16_MAX) > + return -EINVAL; > + > + tx = kzalloc(2 * xlen, GFP_KERNEL); > + if (!tx) > + return -ENOMEM; > + rx = tx + xlen; > + > + tx[0] = reg; > + tx[1] = FTS_SPI_CTRL_WRITE; > + put_unaligned_be16(len, &tx[2]); > + if (len) > + memcpy(&tx[FTS_SPI_HEADER_LEN + FTS_SPI_DUMMY_LEN], val, len); > + > + for (i = 0; i < FTS_SPI_RETRIES; i++) { > + ret = fts_spi_xfer(spi, tx, rx, xlen); > + if (!ret && !(rx[3] & FTS_SPI_STATUS_ERR)) > + break; > + if (!ret) > + ret = -EIO; > + } > + > + kfree(tx); > + return ret; > +} > + > +static int fts_spi_read(struct spi_device *spi, u8 reg, void *val, > + size_t len) > +{ > + size_t dp = FTS_SPI_HEADER_LEN + FTS_SPI_DUMMY_LEN; > + size_t xlen = dp + len + FTS_SPI_CRC_LEN; > + u8 *tx, *rx; > + int ret, i; > + > + if (!len || len > U16_MAX) > + return -EINVAL; > + > + tx = kzalloc(2 * xlen, GFP_KERNEL); > + if (!tx) > + return -ENOMEM; > + rx = tx + xlen; [Severity: High] Does placing rx at tx + xlen in fts_spi_write() and fts_spi_read() cause tx and rx to share a cacheline during full-duplex SPI DMA transfers? In both functions, a single buffer of size 2 * xlen is allocated, with tx at offset 0 and rx = tx + xlen. Both buffers are then passed to fts_spi_xfer() -> spi_sync_transfer() in a single full-duplex spi_transfer, where the SPI core maps tx as DMA_TO_DEVICE and rx as DMA_FROM_DEVICE concurrently. Because xlen is typically not aligned to ARCH_DMA_MINALIGN (for example, xlen is 4 or 8 for control writes, 10 or 11 for short reads, 71 for touch report reads in edt_ft5x06_ts_isr(), and 32759 for firmware RAM writes in fts_boot_write_ram()), rx starts in the middle of the cacheline where tx ends. On systems with DMA-capable SPI controllers, this overlapping mapping in opposite DMA directions triggers DMA-API debug EEXIST warnings and risks cacheline corruption between tx and rx during cache maintenance on non-coherent architectures (particularly when xlen is large enough to bypass swiotlb bouncing under CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC, or across retries in the FTS_SPI_RETRIES loop), as well as presenting an unaligned rx_buf pointer to SPI controllers that require aligned buffers. Could tx and rx be allocated separately, or could the offset of rx be aligned with ALIGN(xlen, ARCH_DMA_MINALIGN)? [ ... ] > -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
