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