All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Panagiotis Gravias" <graviaspanagiotis1@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 3/3] staging: fbtft: Replace udelay with usleep_range
Date: Tue, 14 Jul 2026 19:48:22 +0000	[thread overview]
Message-ID: <20260714194822.8D2CA1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260714190958.219437-6-graviaspanagiotis1@gmail.com>

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

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/20260714190958.219437-2-graviaspanagiotis1@gmail.com?part=3

  parent reply	other threads:[~2026-07-14 19:48 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 19:09 [PATCH 0/3] staging/fbtft: Minor cleanups Panagiotis Gravias
2026-07-14 19:09 ` [PATCH 1/3] staging/fbtft: Remove prohibited space before close parenthesis Panagiotis Gravias
2026-07-14 19:16   ` Andy Shevchenko
2026-07-14 19:33   ` sashiko-bot
2026-07-14 19:09 ` [PATCH 2/3] staging/fbtft: Align arguments to match open parenthesis Panagiotis Gravias
2026-07-14 19:16   ` Andy Shevchenko
2026-07-14 19:09 ` [PATCH 3/3] staging: fbtft: Replace udelay with usleep_range Panagiotis Gravias
2026-07-14 19:17   ` Andy Shevchenko
2026-07-14 19:48   ` sashiko-bot [this message]
2026-07-14 19:18 ` [PATCH 0/3] staging/fbtft: Minor cleanups Andy Shevchenko
2026-07-15  7:28 ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260714194822.8D2CA1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=graviaspanagiotis1@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.