From 9a6246ff78ac33af78f82704cde6fec361597eea Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 27 Feb 2015 18:17:28 +0100 Subject: [PATCH] ALSA: hda - Implement unbind more safely Now we have all pieces ready, and put them into places: - add the hda_pcm refcount to azx_pcm_open() and azx_pcm_close(), - call the most of cleanup code in hda_codec_reset() from the codec driver remove, - call the same code also from the hda_codec object free. Then the codec driver can be unbound more safely now. Signed-off-by: Takashi Iwai --- sound/pci/hda/hda_bind.c | 3 +- sound/pci/hda/hda_codec.c | 70 ++++++++++++++++++---------------- sound/pci/hda/hda_codec.h | 1 + sound/pci/hda/hda_controller.c | 3 ++ sound/pci/hda/hda_local.h | 1 + 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c index 2d00417494e2..311896a23cd1 100644 --- a/sound/pci/hda/hda_bind.c +++ b/sound/pci/hda/hda_bind.c @@ -125,8 +125,7 @@ static int hda_codec_driver_remove(struct device *dev) if (codec->patch_ops.free) codec->patch_ops.free(codec); - codec->preset = NULL; - memset(&codec->patch_ops, 0, sizeof(codec->patch_ops)); + snd_hda_codec_cleanup_for_unbind(codec); module_put(dev->driver->owner); return 0; } diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 3bd9158addc2..2c7e481a9171 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1160,36 +1160,62 @@ struct hda_pcm *snd_hda_codec_pcm_new(struct hda_codec *codec, } EXPORT_SYMBOL_GPL(snd_hda_codec_pcm_new); +/* + * codec destructor + */ static void codec_release_pcms(struct hda_codec *codec) { struct hda_pcm *pcm, *n; list_for_each_entry_safe(pcm, n, &codec->pcm_list_head, list) { list_del_init(&pcm->list); + if (pcm->pcm) + snd_device_disconnect(codec->card, pcm->pcm); snd_hda_codec_pcm_put(pcm); } } -/* - * codec destructor - */ +void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec) +{ + cancel_delayed_work_sync(&codec->jackpoll_work); + flush_workqueue(codec->bus->workq); + if (!codec->in_freeing) + snd_hda_ctls_clear(codec); + codec_release_pcms(codec); + snd_hda_detach_beep_device(codec); + memset(&codec->patch_ops, 0, sizeof(codec->patch_ops)); + snd_hda_jack_tbl_clear(codec); + codec->proc_widget_hook = NULL; + codec->spec = NULL; + + free_hda_cache(&codec->amp_cache); + free_hda_cache(&codec->cmd_cache); + init_hda_cache(&codec->amp_cache, sizeof(struct hda_amp_info)); + init_hda_cache(&codec->cmd_cache, sizeof(struct hda_cache_head)); + + /* free only driver_pins so that init_pins + user_pins are restored */ + snd_array_free(&codec->driver_pins); + snd_array_free(&codec->cvt_setups); + snd_array_free(&codec->spdif_out); + snd_array_free(&codec->verbs); + codec->preset = NULL; + codec->slave_dig_outs = NULL; + codec->spdif_status_reset = 0; + snd_array_free(&codec->mixers); + snd_array_free(&codec->nids); + remove_conn_list(codec); +} + static void snd_hda_codec_free(struct hda_codec *codec) { if (!codec) return; - cancel_delayed_work_sync(&codec->jackpoll_work); - codec_release_pcms(codec); + codec->in_freeing = 1; if (device_is_registered(hda_codec_dev(codec))) device_del(hda_codec_dev(codec)); - snd_hda_jack_tbl_clear(codec); free_init_pincfgs(codec); flush_workqueue(codec->bus->workq); list_del(&codec->list); - snd_array_free(&codec->mixers); - snd_array_free(&codec->nids); - snd_array_free(&codec->cvt_setups); - snd_array_free(&codec->spdif_out); - remove_conn_list(codec); codec->bus->caddr_tbl[codec->addr] = NULL; clear_bit(codec->addr, &codec->bus->codec_powered); snd_hda_sysfs_clear(codec); @@ -2479,31 +2505,9 @@ int snd_hda_codec_reset(struct hda_codec *codec) return -EBUSY; /* OK, let it free */ - cancel_delayed_work_sync(&codec->jackpoll_work); - flush_workqueue(bus->workq); - snd_hda_ctls_clear(codec); - codec_release_pcms(codec); - snd_hda_detach_beep_device(codec); if (device_is_registered(hda_codec_dev(codec))) device_del(hda_codec_dev(codec)); - memset(&codec->patch_ops, 0, sizeof(codec->patch_ops)); - snd_hda_jack_tbl_clear(codec); - codec->proc_widget_hook = NULL; - codec->spec = NULL; - free_hda_cache(&codec->amp_cache); - free_hda_cache(&codec->cmd_cache); - init_hda_cache(&codec->amp_cache, sizeof(struct hda_amp_info)); - init_hda_cache(&codec->cmd_cache, sizeof(struct hda_cache_head)); - /* free only driver_pins so that init_pins + user_pins are restored */ - snd_array_free(&codec->driver_pins); - snd_array_free(&codec->cvt_setups); - snd_array_free(&codec->spdif_out); - snd_array_free(&codec->verbs); - codec->preset = NULL; - codec->slave_dig_outs = NULL; - codec->spdif_status_reset = 0; - /* allow device access again */ snd_hda_unlock_devices(bus); return 0; diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 2ccd6f9a91fe..fc62ca51fd35 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -350,6 +350,7 @@ struct hda_codec { #endif /* misc flags */ + unsigned int in_freeing:1; /* being released */ unsigned int spdif_status_reset :1; /* needs to toggle SPDIF for each * status change * (e.g. Realtek codecs) diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index ad85f9bfaf57..cae50d5ffb81 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -420,6 +420,7 @@ static int azx_pcm_close(struct snd_pcm_substream *substream) hinfo->ops.close(hinfo, apcm->codec, substream); snd_hda_power_down(apcm->codec); mutex_unlock(&chip->open_mutex); + snd_hda_codec_pcm_put(apcm->info); return 0; } @@ -806,6 +807,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) int err; int buff_step; + snd_hda_codec_pcm_get(apcm->info); mutex_lock(&chip->open_mutex); azx_dev = azx_assign_device(chip, substream); if (azx_dev == NULL) { @@ -887,6 +889,7 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) snd_hda_power_down(apcm->codec); unlock: mutex_unlock(&chip->open_mutex); + snd_hda_codec_pcm_put(apcm->info); return err; } diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 8588813163e3..1d001647fc47 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -150,6 +150,7 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name, #define snd_hda_add_vmaster(codec, name, tlv, slaves, suffix) \ __snd_hda_add_vmaster(codec, name, tlv, slaves, suffix, true, NULL) int snd_hda_codec_reset(struct hda_codec *codec); +void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec); enum { HDA_VMUTE_OFF, -- 2.20.1