Re: [PATCH] staging: fbtft: fb_ra8875: replace udelay with usleep_range

[sashiko-bot@kernel.org]

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 <mashikiselby@gmail.com>

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/20260716-staging-testing-v1-1-2ebc0c6bb27a@gmail.com?part=1