drivers/staging/sm750fb/sm750_hw.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
The hardware wait loop in hw_sm750_de_wait used a hardcoded loop
counter (0x10000000), which depends on CPU speed and is unreliable.
Replace the loop counter with a jiffies-based timeout (1 second)
using time_before(). This ensures consistent delays across architectures.
Signed-off-by: Madhumitha Sundar <madhuananda18@gmail.com>
---
Changes in v2:
- Switched from loop counter to jiffies + cpu_relax() as requested by Greg KH.
---
drivers/staging/sm750fb/sm750_hw.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index ce46f240c..b24d27a77 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -523,19 +523,32 @@ int hw_sm750le_de_wait(void)
int hw_sm750_de_wait(void)
{
- int i = 0x10000000;
+ /* Wait for 1 second (HZ) */
+ unsigned long timeout = jiffies + HZ;
unsigned int mask = SYSTEM_CTRL_DE_STATUS_BUSY |
SYSTEM_CTRL_DE_FIFO_EMPTY |
SYSTEM_CTRL_DE_MEM_FIFO_EMPTY;
+ unsigned int val;
- while (i--) {
- unsigned int val = peek32(SYSTEM_CTRL);
+ /* Run while Current Time is BEFORE the Deadline */
+ while (time_before(jiffies, timeout)) {
+ val = peek32(SYSTEM_CTRL);
+ /* If Not Busy (0) AND Buffers Empty (1), we are good */
if ((val & mask) ==
(SYSTEM_CTRL_DE_FIFO_EMPTY | SYSTEM_CTRL_DE_MEM_FIFO_EMPTY))
return 0;
+
+ /* Polite pause to save power */
+ cpu_relax();
}
- /* timeout error */
+
+ /* Final check in case we succeeded at the last millisecond */
+ val = peek32(SYSTEM_CTRL);
+ if ((val & mask) ==
+ (SYSTEM_CTRL_DE_FIFO_EMPTY | SYSTEM_CTRL_DE_MEM_FIFO_EMPTY))
+ return 0;
+
return -1;
}
--
2.43.0
On Tue, Jan 27, 2026 at 04:48:16PM +0000, Madhumitha Sundar wrote:
> The hardware wait loop in hw_sm750_de_wait used a hardcoded loop
> counter (0x10000000), which depends on CPU speed and is unreliable.
>
> Replace the loop counter with a jiffies-based timeout (1 second)
> using time_before(). This ensures consistent delays across architectures.
>
> Signed-off-by: Madhumitha Sundar <madhuananda18@gmail.com>
> ---
> Changes in v2:
> - Switched from loop counter to jiffies + cpu_relax() as requested by Greg KH.
> ---
> drivers/staging/sm750fb/sm750_hw.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
> index ce46f240c..b24d27a77 100644
> --- a/drivers/staging/sm750fb/sm750_hw.c
> +++ b/drivers/staging/sm750fb/sm750_hw.c
> @@ -523,19 +523,32 @@ int hw_sm750le_de_wait(void)
>
> int hw_sm750_de_wait(void)
> {
> - int i = 0x10000000;
> + /* Wait for 1 second (HZ) */
No need for a comment, right?
> + unsigned long timeout = jiffies + HZ;
Why is a variable needed?
And are you sure 1 second is ok? How short was the original timeout?
> unsigned int mask = SYSTEM_CTRL_DE_STATUS_BUSY |
> SYSTEM_CTRL_DE_FIFO_EMPTY |
> SYSTEM_CTRL_DE_MEM_FIFO_EMPTY;
> + unsigned int val;
Why move this?
> - while (i--) {
> - unsigned int val = peek32(SYSTEM_CTRL);
> + /* Run while Current Time is BEFORE the Deadline */
Run what?
> + while (time_before(jiffies, timeout)) {
> + val = peek32(SYSTEM_CTRL);
>
> + /* If Not Busy (0) AND Buffers Empty (1), we are good */
> if ((val & mask) ==
> (SYSTEM_CTRL_DE_FIFO_EMPTY | SYSTEM_CTRL_DE_MEM_FIFO_EMPTY))
> return 0;
> +
> + /* Polite pause to save power */
> + cpu_relax();
Are you sure you can sleep here?
> }
> - /* timeout error */
> +
> + /* Final check in case we succeeded at the last millisecond */
I do not understand this, why would this matter?
> + val = peek32(SYSTEM_CTRL);
> + if ((val & mask) ==
> + (SYSTEM_CTRL_DE_FIFO_EMPTY | SYSTEM_CTRL_DE_MEM_FIFO_EMPTY))
> + return 0;
> +
Was this tested on real hardware?
thanks,
greg k-h
© 2016 - 2026 Red Hat, Inc.