Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Mon, 4 Apr 2022 15:57:35 +0200 | Subject | Re: [PATCH v2] sound/oss/dmasound: fix build when drivers are mixed =y/=m |
| |
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
| |