[PATCH v2] sound/oss/dmasound: fix build when drivers are mixed =y/=m
When CONFIG_DMASOUND_ATARI=m and CONFIG_DMASOUND_Q40=y (or vice versa), dmasound_core.o can be built without dmasound_deinit() being defined, causing a build error:
ERROR: modpost: "dmasound_deinit" [sound/oss/dmasound/dmasound_atari.ko] undefined!
Modify dmasound_core.c so that dmasound_deinit() is always available.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Randy Dunlap rdunlap@infradead.org Reported-by: kernel test robot lkp@intel.com Link: lore.kernel.org/r/202204032138.EFT9qGEd-lkp@intel.com Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: Jaroslav Kysela perex@perex.cz Cc: Takashi Iwai tiwai@suse.com Cc: alsa-devel@alsa-project.org --- #Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") # "forever, but backport not important"
sound/oss/dmasound/dmasound.h | 4 ---- sound/oss/dmasound/dmasound_core.c | 10 +++++----- 2 files changed, 5 insertions(+), 9 deletions(-)
--- linux-next-20220401.orig/sound/oss/dmasound/dmasound_core.c +++ linux-next-20220401/sound/oss/dmasound/dmasound_core.c @@ -1424,27 +1424,29 @@ int dmasound_init(void) return 0; }
-#ifdef MODULE - void dmasound_deinit(void) { +#ifdef MODULE if (irq_installed) { sound_silence(); dmasound.mach.irqcleanup(); irq_installed = 0; } +#endif
write_sq_release_buffers();
+#ifdef MODULE if (mixer_unit >= 0) unregister_sound_mixer(mixer_unit); if (state_unit >= 0) unregister_sound_special(state_unit); if (sq_unit >= 0) unregister_sound_dsp(sq_unit); +#endif }
-#else /* !MODULE */ +#ifndef MODULE
static int dmasound_setup(char *str) { @@ -1577,9 +1579,7 @@ char dmasound_alaw2dma8[] = {
EXPORT_SYMBOL(dmasound); EXPORT_SYMBOL(dmasound_init); -#ifdef MODULE EXPORT_SYMBOL(dmasound_deinit); -#endif EXPORT_SYMBOL(dmasound_write_sq); EXPORT_SYMBOL(dmasound_catchRadius); #ifdef HAS_8BIT_TABLES --- linux-next-20220401.orig/sound/oss/dmasound/dmasound.h +++ linux-next-20220401/sound/oss/dmasound/dmasound.h @@ -88,11 +88,7 @@ static inline int ioctl_return(int __use */
extern int dmasound_init(void); -#ifdef MODULE extern void dmasound_deinit(void); -#else -#define dmasound_deinit() do { } while (0) -#endif
/* description of the set-up applies to either hard or soft settings */
Hi Randy,
On Mon, Apr 4, 2022 at 12:25 AM Randy Dunlap rdunlap@infradead.org wrote:
When CONFIG_DMASOUND_ATARI=m and CONFIG_DMASOUND_Q40=y (or vice versa), dmasound_core.o can be built without dmasound_deinit() being defined, causing a build error:
ERROR: modpost: "dmasound_deinit" [sound/oss/dmasound/dmasound_atari.ko] undefined!
Modify dmasound_core.c so that dmasound_deinit() is always available.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Randy Dunlap rdunlap@infradead.org
Thanks for spending more time on this ;-)
--- linux-next-20220401.orig/sound/oss/dmasound/dmasound_core.c +++ linux-next-20220401/sound/oss/dmasound/dmasound_core.c @@ -1424,27 +1424,29 @@ int dmasound_init(void) return 0; }
-#ifdef MODULE
void dmasound_deinit(void) { +#ifdef MODULE
I think this #ifdef must not be added: if the modular subdriver calls dmasound_deinit(), the resources should be freed, else a subsequent reload of the subdriver will not work. This does mean all variables protected by "#ifdef MODULE" must exist unconditionally.
Alternatively, the test can be replaced by "#ifdef CONFIG_MODULES".
One big caveat below...
if (irq_installed) { sound_silence(); dmasound.mach.irqcleanup(); irq_installed = 0; }+#endif
write_sq_release_buffers();+#ifdef MODULE
Likewise.
if (mixer_unit >= 0) unregister_sound_mixer(mixer_unit); if (state_unit >= 0) unregister_sound_special(state_unit); if (sq_unit >= 0) unregister_sound_dsp(sq_unit);+#endif }
-#else /* !MODULE */ +#ifndef MODULE
static int dmasound_setup(char *str) {
--- linux-next-20220401.orig/sound/oss/dmasound/dmasound.h +++ linux-next-20220401/sound/oss/dmasound/dmasound.h @@ -88,11 +88,7 @@ static inline int ioctl_return(int __use */
extern int dmasound_init(void); -#ifdef MODULE extern void dmasound_deinit(void); -#else -#define dmasound_deinit() do { } while (0) -#endif
/* description of the set-up applies to either hard or soft settings */
... Below, there is:
typedef struct { [...] #ifdef MODULE void (*irqcleanup)(void); #endif [...] } MACHINE;
This means the MACHINE struct is not compatible between builtin and modular code :-( Hence the "#ifdef MODULE" should be removed, or replaced by "#ifdef CONFIG_MODULES", too.
P.S. I think the younger myself is responsible for this mess. Please accept my apologies, after +25 years...
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert,
On 4/4/22 06:57, Geert Uytterhoeven wrote:
Hi Randy,
On Mon, Apr 4, 2022 at 12:25 AM Randy Dunlap rdunlap@infradead.org wrote:
When CONFIG_DMASOUND_ATARI=m and CONFIG_DMASOUND_Q40=y (or vice versa), dmasound_core.o can be built without dmasound_deinit() being defined, causing a build error:
ERROR: modpost: "dmasound_deinit" [sound/oss/dmasound/dmasound_atari.ko] undefined!
Modify dmasound_core.c so that dmasound_deinit() is always available.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Randy Dunlap rdunlap@infradead.org
Thanks for spending more time on this ;-)
Well that bot keeps reporting this problem, although I suppose that we could ask for it to be ignored...
--- linux-next-20220401.orig/sound/oss/dmasound/dmasound_core.c +++ linux-next-20220401/sound/oss/dmasound/dmasound_core.c @@ -1424,27 +1424,29 @@ int dmasound_init(void) return 0; }
-#ifdef MODULE
void dmasound_deinit(void) { +#ifdef MODULE
I think this #ifdef must not be added: if the modular subdriver calls dmasound_deinit(), the resources should be freed, else a subsequent reload of the subdriver will not work. This does mean all variables protected by "#ifdef MODULE" must exist unconditionally.
OK, I like that simplification.
Alternatively, the test can be replaced by "#ifdef CONFIG_MODULES".
One big caveat below...
if (irq_installed) { sound_silence(); dmasound.mach.irqcleanup(); irq_installed = 0; }+#endif
write_sq_release_buffers();+#ifdef MODULE
Likewise.
if (mixer_unit >= 0) unregister_sound_mixer(mixer_unit); if (state_unit >= 0) unregister_sound_special(state_unit); if (sq_unit >= 0) unregister_sound_dsp(sq_unit);+#endif }
-#else /* !MODULE */ +#ifndef MODULE
static int dmasound_setup(char *str) {
--- linux-next-20220401.orig/sound/oss/dmasound/dmasound.h +++ linux-next-20220401/sound/oss/dmasound/dmasound.h @@ -88,11 +88,7 @@ static inline int ioctl_return(int __use */
extern int dmasound_init(void); -#ifdef MODULE extern void dmasound_deinit(void); -#else -#define dmasound_deinit() do { } while (0) -#endif
/* description of the set-up applies to either hard or soft settings */
... Below, there is:
typedef struct { [...] #ifdef MODULE void (*irqcleanup)(void); #endif [...] } MACHINE;This means the MACHINE struct is not compatible between builtin and modular code :-( Hence the "#ifdef MODULE" should be removed, or replaced by "#ifdef CONFIG_MODULES", too.
ditto
P.S. I think the younger myself is responsible for this mess. Please accept my apologies, after +25 years...
:)
I'll see how it goes. Thanks.
participants (2)
-
Geert Uytterhoeven -
Randy Dunlap