mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] m68k: amiga: Mark sine_data as __initconst
@ 2026-11-08 13:45 Thorsten Blum
  2026-11-08 13:45 ` [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound() Thorsten Blum
  0 siblings, 1 reply; 4+ messages in thread
From: Thorsten Blum @ 2026-11-08 13:45 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Thorsten Blum, linux-m68k, linux-kernel

sine_data is only read by the __init function amiga_init_sound().

Signed-off-by: Thorsten Blum <blum@kernel.org>
---
 arch/m68k/amiga/amisound.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/m68k/amiga/amisound.c b/arch/m68k/amiga/amisound.c
index 5fd93dfab809..5ae1f017b39b 100644
--- a/arch/m68k/amiga/amisound.c
+++ b/arch/m68k/amiga/amisound.c
@@ -19,7 +19,7 @@
 #include "amiga.h"
 
 static unsigned short *snd_data;
-static const signed char sine_data[] = {
+static const signed char sine_data[] __initconst = {
 	0,  39,  75,  103,  121,  127,  121,  103,  75,  39,
 	0, -39, -75, -103, -121, -127, -121, -103, -75, -39
 };

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound()
  2026-11-08 13:45 [PATCH 1/2] m68k: amiga: Mark sine_data as __initconst Thorsten Blum
@ 2026-11-08 13:45 ` Thorsten Blum
  2026-11-08 22:58   ` David Laight
  0 siblings, 1 reply; 4+ messages in thread
From: Thorsten Blum @ 2026-11-08 13:45 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Thorsten Blum, linux-m68k, linux-kernel

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);
 		custom.aud[2].audvol = 32; /* 50% of maxvol */
 
 		if (ticks) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound()
  2026-11-08 13:45 ` [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound() Thorsten Blum
@ 2026-11-08 22:58   ` David Laight
  2026-11-08 23:22     ` Thorsten Blum
  0 siblings, 1 reply; 4+ messages in thread
From: David Laight @ 2026-11-08 22:58 UTC (permalink / raw)
  To: Thorsten Blum; +Cc: Geert Uytterhoeven, linux-m68k, linux-kernel

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) {
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound()
  2026-11-08 22:58   ` David Laight
@ 2026-11-08 23:22     ` Thorsten Blum
  0 siblings, 0 replies; 4+ messages in thread
From: Thorsten Blum @ 2026-11-08 23:22 UTC (permalink / raw)
  To: David Laight; +Cc: Geert Uytterhoeven, linux-m68k, linux-kernel

On Sun, Nov 08, 2026 at 10:58:48PM +0000, David Laight wrote:
> 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.

amiga_audio_min_period is an unsigned short and can't exceed MAX_PERIOD,
which is defined as 65535.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-08 23:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-08 13:45 [PATCH 1/2] m68k: amiga: Mark sine_data as __initconst Thorsten Blum
2026-11-08 13:45 ` [PATCH 2/2] m68k: amiga: Use clamp() in amiga_mksound() Thorsten Blum
2026-11-08 22:58   ` David Laight
2026-11-08 23:22     ` Thorsten Blum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®