Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] If a SPI write fails in `write_reg8_bus8`, the function returns early without restoring `par->fbtftops.write` to its original value, permanently downgrading the SPI interface speed. - [High] The `txbuf16` pointer in `write_vmem16_bus8` is assigned to an odd-aligned address, causing unaligned memory writes that can trigger a kernel panic on strict architectures. -- --- Patch [3]: [PATCH 3/3] staging: fbtft: Replace udelay with usleep_range --- commit 7f6e8faf1241a2de5c5b0d51aa0632d9694e2167 Author: Panagiotis Gravias <[email protected]> staging: fbtft: Replace udelay with usleep_range This commit replaces calls to udelay() with usleep_range() in the fbtft driver. This addresses checkpatch warnings and provides an upper bound to allow the scheduler to coalesce timer interrupts. > diff --git a/drivers/staging/fbtft/fb_ra8875.c > b/drivers/staging/fbtft/fb_ra8875.c > index 0ab1de6647d07..d2400bb44f1c6 100644 > --- a/drivers/staging/fbtft/fb_ra8875.c > +++ b/drivers/staging/fbtft/fb_ra8875.c [ ... ] > @@ -231,7 +231,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int > len, ...) > > /* restore user spi-speed */ > par->fbtftops.write = fbtft_write_spi; > - udelay(100); > + usleep_range(100, 200); > } [Severity: High] This isn't a bug introduced by this patch, but I noticed something about the error handling in write_reg8_bus8() related to the SPI speed restore. When par->fbtftops.write() fails earlier in this function, it returns immediately: if (ret < 0) { va_end(args); dev_err(par->info->device, "write() failed and returned %dn", ret); return; } Does this leave par->fbtftops.write permanently assigned to the slow write_spi function? It appears that if a register write fails, the driver never restores the fast fbtft_write_spi pointer, which might severely impact the performance of all future bulk video memory writes. [Severity: High] This is also a pre-existing issue, but looking at the neighboring write_vmem16_bus8() function in the same file: write_vmem16_bus8() { ... txbuf16 = par->txbuf.buf + 1; ... for (i = 0; i < to_copy; i++) txbuf16[i] = cpu_to_be16(vmem16[i]); } Is there a risk of an alignment fault here? The txbuf16 pointer is assigned an odd-aligned address (buf + 1). On architectures that do not support unaligned memory access in hardware (which is common for systems with SPI displays), would these 16-bit writes trigger a kernel panic? Should this be using put_unaligned_be16() instead? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
