[alsa-devel] [PATCH] ALSA: HDA: hda_local: Less magic numbers.

Takashi Iwai tiwai at suse.de
Tue Sep 16 14:10:53 CEST 2008


At Mon, 15 Sep 2008 13:16:15 -0700,
Robin H. Johnson wrote:
> 
> On Mon, Sep 15, 2008 at 09:10:10AM +0930, Jonathan Woithe wrote:
> > Shrug.  In my opinion the above patch simply reworks the explanation,
> > shifting the "magic" from the PIN_* definitions to the AC_PINCTL_* defines.
> > One could take things one step further and replace the PIN_* defines with
> > AC_PINCTL_* references throughout the code, but this would make the code far
> > more verbose and harder to follow.
> > 
> > At the end of the day I'm not fussed either way.  Having said that, I don't
> > really see the practical advantage of the change but if Takashi et al thing
> > it's a good idea then I wouldn't oppose it.
> A quick glance at the codebase shows:
> 
> AC_PINCTL* long form:
> patch_conexant.c
> patch_realtek.c
> patch_sigmatel.c
> 
> /PIN_(VREF|IN|OUT|HP)/
> hda_codec.c
> patch_analog.c
> patch_atihdmi.c
> patch_cmedia.c
> patch_conexant.c
> patch_realtek.c
> patch_sigmatel.c
> patch_via.c
> 
> Raw hex for AC_VERB_SET_PIN_WIDGET_CONTROL verbs:
> patch_analog.c
> patch_realtek.c (LOTS)
> patch_via.c (LOTS)
> 
> The raw hex ones SHOULD be replaced, but the rest of them I'm not fussed either
> way. Takashi can make a decision as to which variant is preferred, I don't mind
> doing the cleanup.

Robin's patch is a right improvement wrt readability for whom are not
familiar with HD-audio codec spec, so I applied it as is.

Well, I also don't care much about the rest, too.  And, I don't think
it's worth to work on this further so seriously, although I don't mind
patches that can be really convincing (if any).

FWIW, AC_* and PIN_* have somewhat different fields: the former is the
definition of each bit, and the latter is the values used in the real
world.


thanks,

Takashi


More information about the Alsa-devel mailing list