Hi Randy,
On Mon, Apr 4, 2022 at 12:25 AM Randy Dunlap <rdunlap@xxxxxxxxxxxxx> 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@xxxxxxxx>
> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx
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
