At Wed, 27 Nov 2013 04:14:58 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, November 26, 2013 10:05 PM
+#ifdef CONFIG_PM
- bus->pm_wq = create_workqueue("hda-pm-workq");
Better to use a unique name, e.g. with a card number suffix.
Ok.
- if (!bus->pm_wq) {
snd_printk(KERN_ERR "cannot create PM workqueue\n");snd_hda_bus_free(bus);return -ENOMEM;- }
- workqueue_set_max_active(bus->pm_wq, HDA_MAX_NUM_CODECS);
As mentioned in another post, this can be set dynamically, e.g. at snd_hda_suspend() with bus->num_codecs.
Can I move this to snd_hda_codec_new() like this? list_add_tail(&codec->list, &bus->codec_list); +bus->num_codecs++; +workqueue_set_max_active(bus->pm_wq, bus->num_codecs);
The codec number does not change after initialization, and usually 1 or 2 codecs are connected to the bus.
It's not perfect but it'd be good enough, yes. (A codec might be dynamically removed via snd_hda_codec_free() in theory, but this wouldn't happen actually.)
err = snd_device_new(card, SNDRV_DEV_BUS, bus, &dev_ops); if (err < 0) { snd_hda_bus_free(bus); @@ -1434,6 +1451,8 @@ int snd_hda_codec_new(struct hda_bus *bus, */ hda_keep_power_on(codec); hda_call_pm_notify(bus, true);
- INIT_WORK(&codec->pm_work, hda_pm_work);
Better to put this before hda_keep_power_on() call.
Ok.
if (codec->bus->modelname) { @@ -1445,6 +1464,7 @@ int snd_hda_codec_new(struct hda_bus *bus, }
list_add_tail(&codec->list, &bus->codec_list);
bus->num_codecs++; bus->caddr_tbl[codec_addr] = codec;
codec->vendor_id = snd_hda_param_read(codec, AC_NODE_ROOT,
Also decrease bus->num_codecs in snd_hda_codec_free(), just to be sure.
Can I decrease bus->num_codecs in snd_hda_bus_free(), after the codec is freed? list_for_each_entry_safe(codec, n, &bus->codec_list, list) { snd_hda_codec_free(codec);
bus->num_codecs--;
It looks inconsistent. The increment is done in snd_hda_codec_new(), so the decrement should be in its counterpart.
thanks,
Takashi