[PATCH] fbdev/pxafb: Fix multiple clamped values in pxafb_adjust_timing

The variables were never clamped because the return value of clamp_val()
was not used. Fix this by assigning the clamped values, and use clamp()
instead of clamp_val().

Cc: stable@vger.kernel.org
Fixes: 3f16ff608a75 ("[ARM] pxafb: cleanup of the timing checking code")
Signed-off-by: Thorsten Blum <redacted>
---
 drivers/video/fbdev/pxafb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index b96a8a96bce8..e418eee825fb 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c

@@ -419,12 +419,12 @@ static int pxafb_adjust_timing(struct pxafb_info *fbi,
 	var->yres = max_t(int, var->yres, MIN_YRES);
 
 	if (!(fbi->lccr0 & LCCR0_LCDT)) {
-		clamp_val(var->hsync_len, 1, 64);
-		clamp_val(var->vsync_len, 1, 64);
-		clamp_val(var->left_margin,  1, 255);
-		clamp_val(var->right_margin, 1, 255);
-		clamp_val(var->upper_margin, 1, 255);
-		clamp_val(var->lower_margin, 1, 255);
+		var->hsync_len = clamp(var->hsync_len, 1, 64);
+		var->vsync_len = clamp(var->vsync_len, 1, 64);
+		var->left_margin  = clamp(var->left_margin,  1, 255);
+		var->right_margin = clamp(var->right_margin, 1, 255);
+		var->upper_margin = clamp(var->upper_margin, 1, 255);
+		var->lower_margin = clamp(var->lower_margin, 1, 255);
 	}
 
 	/* make sure each line is aligned on word boundary */

-- 
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6  9D84 7336 78FD 8DFE EAD4

On 12/2/25 19:15, Thorsten Blum wrote:

The variables were never clamped because the return value of clamp_val()
was not used. Fix this by assigning the clamped values, and use clamp()
instead of clamp_val().

Cc: stable@vger.kernel.org
Fixes: 3f16ff608a75 ("[ARM] pxafb: cleanup of the timing checking code")
Signed-off-by: Thorsten Blum <redacted>
---
  drivers/video/fbdev/pxafb.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)

Thanks for the patch!
It looks good, so I'll include it in the fbdev tree.
Out of curiosity:
How did you notice? Do you actually have the hardware and tested it?

Helge 

On 2. Dec 2025, at 19:28, Helge Deller wrote:

On 12/2/25 19:15, Thorsten Blum wrote:

quoted

The variables were never clamped because the return value of clamp_val()
was not used. Fix this by assigning the clamped values, and use clamp()
instead of clamp_val().
Cc: stable@vger.kernel.org
Fixes: 3f16ff608a75 ("[ARM] pxafb: cleanup of the timing checking code")
Signed-off-by: Thorsten Blum <redacted>
---
 drivers/video/fbdev/pxafb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Thanks for the patch!
It looks good, so I'll include it in the fbdev tree.
Out of curiosity:
How did you notice? Do you actually have the hardware and tested it?

I only compile-tested it.

I stumbled upon another driver with the same bug and then used grep to
search for other instances and found about 6 or 7, including this one.

Thanks,
Thorsten

On 12/2/25 19:36, Thorsten Blum wrote:

On 2. Dec 2025, at 19:28, Helge Deller wrote:

quoted

On 12/2/25 19:15, Thorsten Blum wrote:

quoted

The variables were never clamped because the return value of clamp_val()
was not used. Fix this by assigning the clamped values, and use clamp()
instead of clamp_val().
Cc: stable@vger.kernel.org
Fixes: 3f16ff608a75 ("[ARM] pxafb: cleanup of the timing checking code")
Signed-off-by: Thorsten Blum <redacted>
---
  drivers/video/fbdev/pxafb.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)

Thanks for the patch!
It looks good, so I'll include it in the fbdev tree.
Out of curiosity:
How did you notice? Do you actually have the hardware and tested it?

I only compile-tested it.

I stumbled upon another driver with the same bug and then used grep to
search for other instances and found about 6 or 7, including this one.

Ok. But this then means, maybe the clamping isn't needed (since nobody complained),
or that nobody noticed because nobody uses the driver any longer.
Anyway, I believe the patch is correct, so I leave it in.

Thank you!
Helge

On Tue, 2 Dec 2025 19:36:17 +0100
Thorsten Blum [off-list ref] wrote:

On 2. Dec 2025, at 19:28, Helge Deller wrote:

quoted

On 12/2/25 19:15, Thorsten Blum wrote:  

quoted

The variables were never clamped because the return value of clamp_val()
was not used. Fix this by assigning the clamped values, and use clamp()
instead of clamp_val().
Cc: stable@vger.kernel.org
Fixes: 3f16ff608a75 ("[ARM] pxafb: cleanup of the timing checking code")
Signed-off-by: Thorsten Blum <redacted>
---
 drivers/video/fbdev/pxafb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)  

Thanks for the patch!
It looks good, so I'll include it in the fbdev tree.
Out of curiosity:
How did you notice? Do you actually have the hardware and tested it?  

I only compile-tested it.

I stumbled upon another driver with the same bug and then used grep to
search for other instances and found about 6 or 7, including this one.

I've just hacked minmax.h so clamp() and clamp_t() (and thus clamp_val())
are 'static inline u64 __must_check clamp(...)'.

Didn't find any in any other files.
But I think you missed some double error in the same file (nearby).

	David

Thanks,
Thorsten

On Tue, Dec 02, 2025 at 08:36:08PM +0100, Helge Deller wrote:

On 12/2/25 19:36, Thorsten Blum wrote:

quoted

On 2. Dec 2025, at 19:28, Helge Deller wrote:

quoted

On 12/2/25 19:15, Thorsten Blum wrote:

...

quoted

quoted

How did you notice? Do you actually have the hardware and tested it?

I only compile-tested it.

I stumbled upon another driver with the same bug and then used grep to
search for other instances and found about 6 or 7, including this one.

Ok. But this then means, maybe the clamping isn't needed (since nobody complained),
or that nobody noticed because nobody uses the driver any longer.

I think it's a combination of factors: 1) rarely people have this hardware,
especially nowadays, to run more or less new kernel; 2) there are no conditions
happened that this patch fixes in their environments; 3) something else I
missed.


-- 
With Best Regards,
Andy Shevchenko

On Wed, Dec 03, 2025 at 11:49:07AM +0200, Andy Shevchenko wrote:

On Tue, Dec 02, 2025 at 08:36:08PM +0100, Helge Deller wrote:

quoted

On 12/2/25 19:36, Thorsten Blum wrote:

quoted

On 2. Dec 2025, at 19:28, Helge Deller wrote:

quoted

On 12/2/25 19:15, Thorsten Blum wrote:

...

quoted

quoted

quoted

How did you notice? Do you actually have the hardware and tested it?

I only compile-tested it.

I stumbled upon another driver with the same bug and then used grep to
search for other instances and found about 6 or 7, including this one.

Ok. But this then means, maybe the clamping isn't needed (since nobody complained),
or that nobody noticed because nobody uses the driver any longer.

I think it's a combination of factors: 1) rarely people have this hardware,
especially nowadays, to run more or less new kernel; 2) there are no conditions
happened that this patch fixes in their environments; 3) something else I
missed.

I had a quick look at commit 3f16ff608a75 and seems like there wasn't much
happening either way.

Raag