[alsa-devel] [PATCH v2] ALSA: hda - suspend codecs in parallel

[Lin, Mengdong]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Wednesday, November 27, 2013 2:17 PM

> > > > +	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.)

Well, I'll put this to snd_hda_suspend() as you suggested. Perfect is better than good enough.
 
> > > >  	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.
Okay.

I'll also fixed work queue name and indent level error in v3 patch.

Regards
Mengdong