Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Missing restoration of `par->fbtftops.write` on early error paths in `write_reg8_bus8()` permanently degrades performance. - [High] Unaligned 16-bit memory access when writing to the transmit buffer. -- commit f7268afc18ac218fe8bb9a291f101fccad374b35 Author: Georgii Druzhinin <[email protected]> staging: fbtft: fb_ra8875: prefer usleep_range over udelay This commit replaces udelay(100) with usleep_range(100, 120) in write_reg8_bus8() to avoid busy-waiting and allow the scheduler to utilize the CPU during the delay. > diff --git a/drivers/staging/fbtft/fb_ra8875.c > b/drivers/staging/fbtft/fb_ra8875.c > index 0ab1de6647d07..6058934e2ca2c 100644 > --- a/drivers/staging/fbtft/fb_ra8875.c > +++ b/drivers/staging/fbtft/fb_ra8875.c > @@ -210,7 +210,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int > len, ...) > } [Severity: High] This is a pre-existing issue, but returning early in the error paths above this code seems to skip restoring the write function pointer. If the SPI write fails earlier in write_reg8_bus8(), we return directly: write_reg8_bus8() { ... par->fbtftops.write = write_spi; ... ret = par->fbtftops.write(par, par->buf, 2); if (ret < 0) { va_end(args); dev_err(...); return; } ... } Does this leave the device in a permanently degraded state where all subsequent operations use the unoptimized write_spi function instead of restoring fbtft_write_spi at the end of the function? > len--; > > - udelay(100); > + usleep_range(100, 120); > > if (len) { > buf = (u8 *)par->buf; [ ... ] > @@ -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, 120); > } While reviewing this file, I also noticed another pre-existing issue in write_vmem16_bus8() that might need attention: [Severity: High] This is a pre-existing issue, but does write_vmem16_bus8() cause unaligned memory accesses on the transmit buffer? The txbuf16 pointer is initialized by adding 1 to a void pointer: write_vmem16_bus8() { ... txbuf16 = par->txbuf.buf + 1; ... for (i = 0; i < to_copy; i++) txbuf16[i] = cpu_to_be16(vmem16[i]); ... } Since par->txbuf.buf is a void pointer, adding 1 increments it by exactly 1 byte, making the address strictly odd. Will the subsequent 16-bit writes in the loop cause an alignment fault on strict-alignment architectures like older ARM or MIPS often targeted by these display drivers? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
