Re: [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound()

[David Laight <david.laight.linux@gmail.com>]

On Sun,  8 Nov 2026 14:45:01 +0100
Thorsten Blum <blum@kernel.org> wrote:

> Replace the open-coded period bounds checks with clamp() and remove the
> redundant comment.
> 
> Signed-off-by: Thorsten Blum <blum@kernel.org>
> ---
>  arch/m68k/amiga/amisound.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/m68k/amiga/amisound.c b/arch/m68k/amiga/amisound.c
> index 5ae1f017b39b..5b9d265b2cac 100644
> --- a/arch/m68k/amiga/amisound.c
> +++ b/arch/m68k/amiga/amisound.c
> @@ -12,6 +12,7 @@
>  #include <linux/timer.h>
>  #include <linux/init.h>
>  #include <linux/string.h>
> +#include <linux/minmax.h>
>  #include <linux/module.h>
>  
>  #include <asm/amigahw.h>
> @@ -81,17 +82,10 @@ void amiga_mksound( unsigned int hz, unsigned int ticks )
>  	timer_delete(&sound_timer);
>  
>  	if (hz > 20 && hz < 32767) {
> -		unsigned long period = (clock_constant / hz);
> -
> -		if (period < amiga_audio_min_period)
> -			period = amiga_audio_min_period;
> -		if (period > MAX_PERIOD)
> -			period = MAX_PERIOD;
> -
> -		/* setup pointer to data, period, length and volume */
>  		custom.aud[2].audlc = snd_data;
>  		custom.aud[2].audlen = sizeof(sine_data)/2;
> -		custom.aud[2].audper = (unsigned short)period;
> +		custom.aud[2].audper = clamp(clock_constant / hz,
> +					     amiga_audio_min_period, MAX_PERIOD);

That requires that amiga_audio_min_period be less than MAX_PERIOD.
The order clamp() does the compares in is undefined.
I think it currently does high then low. (IIRC it got swapped a few years
back by a commit that said it didn't change anything!)

I've thought about proposing swapping the order and then allowing:
	clamp(int_var, 0, unsigned_limit);

David

>  		custom.aud[2].audvol = 32; /* 50% of maxvol */
>  
>  		if (ticks) {
>