Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [Critical] Unconditional NULL Pointer Dereference on Big Endian Architectures - [High] Missing State Restoration on Error Path - [High] Unaligned Memory Access causing alignment faults -- --- Patch [1]: [PATCH] staging: fbtft: fb_ra8875: replace udelay with usleep_range --- commit 0b662df2d1169732bcf1a8f82e3c8a0905497245 Author: Selby Mashiki <[email protected]> staging: fbtft: fb_ra8875: replace udelay with usleep_range udelay() busy-waits for the specified duration, consuming CPU cycles unnecessarily. usleep_range() is preferred for delays that can sleep, as it allows the scheduler to run other tasks during the wait. > 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 > @@ -210,7 +210,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int > len, ...) > } > len--; > > - udelay(100); > + usleep_range(100, 200); > > 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, 200); > } [Severity: High] This isn't a bug introduced by this patch, but I noticed the error paths in write_reg8_bus8() return early: write_reg8_bus8() { ... ret = par->fbtftops.write(par, par->buf, len + 1); if (ret < 0) { va_end(args); dev_err(par->info->device, "write() failed and returned %dn", ret); return; } ... } Does this early return leave par->fbtftops.write pointing to the slower write_spi function, permanently degrading SPI performance? Should it restore fbtft_write_spi before returning? > > static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t > len) [Severity: Critical] This is a pre-existing issue, but does write_vmem16_bus8() unconditionally dereference par->txbuf.buf? write_vmem16_bus8() { ... *(u8 *)(par->txbuf.buf) = 0x00; ... } If par->txbuf.buf is only dynamically allocated on Little Endian architectures or when txbuflen > 0, could this cause a kernel panic on Big Endian systems where it might be NULL? [Severity: High] Also a pre-existing issue in write_vmem16_bus8(): is there a risk of alignment faults on architectures that do not support unaligned memory access? write_vmem16_bus8() { ... txbuf16 = par->txbuf.buf + 1; ... for (i = 0; i < to_copy; i++) txbuf16[i] = cpu_to_be16(vmem16[i]); ... } Since txbuf16 is cast from an odd byte offset (par->txbuf.buf + 1), does the 16-bit assignment in the loop cause an unaligned access? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
