Re: Patches to azx/patch_realtek.c

[Takashi Iwai <tiwai@suse.de>]

At Wed, 15 Dec 2004 14:34:44 -0800,
Stephen Warren wrote:
> 
> From: Takashi Iwai [mailto:tiwai@suse.de] 
> > 
> > At Wed, 15 Dec 2004 13:47:26 -0800,
> > Stephen Warren wrote:
> > > 
> > > * realtek.4.diff
> > > 
> > > spec->front_panel is no longer used, after Takashi's latest 
> > changes. So,
> > > remove it from the structure.
> > 
> > I'd like to keep this information in case we'll need some hacks for
> > the front panel switching or any boards without front panel in
> > future.
> 
> OK. I can put that back in. But looking at your latest code, it says
> this:
> 
> 	switch (spec->board_config) {
> 	case ALC880_3ST_DIG:
> 	case ALC880_5ST_DIG:
> 		spec->digital = 1;
> 		spec->front_panel = 1;
> 		break;
> 	case ALC880_3ST:
> 	case ALC880_5ST:
> 		spec->front_panel = 1;
> 		break;
> 	}
> 
> Which would set front_panel all the time, except for ACL880_MINIMAL. Is
> this correct?

Yes, it seems that all boards have a front panel, so far.

> > > * realtek.5.diff
> > > 
> > > Stop functions directly referencing global variables for 
> > configuration
> > > tables - put pointers to the tables into the spec data structure.
> > > 
> > > This allows alc880_init to point spec at different sets of data
> > > structures if it needs, and also stops code from knowing about
> > > spec->five_stack, and doing different things depending on the board
> > > type.
> > 
> > If we do this, let's merge alc260 code, too :)
> 
> That's the next patch in the series:-)

Fine.

> > Maybe it's better to use the array of mixer elem lists than
> > having separate spec->base_mixer and spec->side_mixer as the
> > generalization.
> 
> So, have this: ?
> 
> struct alc_mixer {
>     snd_kcontrol_new_t *controls;
>     int num_controls;
> };
> 
> struct alc_spec {
>     struct alc_mixer mixers[2];
>     int num_mixers;
> };
> 
> And have all the code loop over mixers[0..num_mixers-1]

That looks OK.  num_controls can be removed and thus struct alc_mixer
won't be needed when we change snd_hda_add_new_ctls() as you suggested
below.

> By the way, when an array of had_verb is passed to
> snd_hda_sequence_write, this function looks for a "blank" entry in the
> array to stop at. However, when passing arrays of snd_kcontrol_new_t to
> snd_hda_add_new_ctls, we have to tell it the size, which means we have
> to store that too. Can we make the utility code that processes arrays of
> snd_kcontrol_new_t look for empty entries too?

Yes, I had the same idea.  snd_hda_add_new_ctls() and
snd_hda_resume_ctls() can check entry->name == NULL.


Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/