On Wed, 24 Jun 2020 18:33:00 +0200, Jaroslav Kysela wrote: > > Dne 24. 06. 20 v 18:13 Pierre-Louis Bossart napsal(a): > > > > > > On 6/24/20 11:03 AM, Takashi Iwai wrote: > >> The module argument passed to snd_card_new() must be a valid non-NULL > >> pointer when the module support is enabled. Since ASoC driver passes > >> the argument from each snd_soc_card definition, one may forget to set > >> the owner field and lead to a NULL module easily. > >> > >> For catching such an overlook, add a WARN_ON() in snd_card_new(). > >> Also, put the card->module assignment in the ifdef block for a very > >> minor optimization. > >> > >> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > >> --- > >> sound/core/init.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/sound/core/init.c b/sound/core/init.c > >> index b02a99766351..0478847ba2b8 100644 > >> --- a/sound/core/init.c > >> +++ b/sound/core/init.c > >> @@ -203,7 +203,10 @@ int snd_card_new(struct device *parent, int idx, const char *xid, > >> mutex_unlock(&snd_card_mutex); > >> card->dev = parent; > >> card->number = idx; > >> +#ifdef MODULE > >> + WARN_ON(!module); > >> card->module = module; > >> +#endif > >> INIT_LIST_HEAD(&card->devices); > >> init_rwsem(&card->controls_rwsem); > >> rwlock_init(&card->ctl_files_rwlock); > > > > Would it make sense to also change the ASoC code to use THIS_MODULE > > instead of card->owner? > > > > /* card bind complete so register a sound card */ > > ret = snd_card_new(card->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, > > card->owner, 0, &card->snd_card); > > > > A quick grep shows we are setting .owner = THIS_MODULE pretty much all > > the time for machine drivers. > > > > THIS_MODULE is defined when the object file is created (compile > time). We want to assign the real module which creates the card here, > not "snd_soc_core" which is misleading. Right. We could make a helper macro to pass THIS_MODULE automagically but I don't think it worth. A simple check should suffice. Of course, we can put a similar check in ASoC side, too, so that it points more clearly what's wrong. thanks, Takashi
