[alsa-devel] [alsa-cvslog] alsa-kmirror: ALSA kernel mirror repository branch, master now at v1.0.21-334-g6739046

[Takashi Iwai]

At Thu, 12 Nov 2009 12:02:40 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> 
> > At Thu, 12 Nov 2009 10:54:02 +0100 (CET),
> > noreply-git at alsa-project.org wrote:
> >>
> >> commit 6739046df36c7adf80c961bcba4870270e66dbf6
> >> Author:     Jaroslav Kysela <perex at perex.cz>
> >> AuthorDate: Thu Nov 12 10:15:48 2009 +0100
> >> Commit:     Jaroslav Kysela <perex at perex.cz>
> >> CommitDate: Thu Nov 12 10:51:48 2009 +0100
> >>
> >>     ALSA: hda - proc - add support for dynamic controls to mixer<->NID mapping
> >>
> >>     This patch adds support for dynamically created controls to proc codec file
> >>     (Control: lines).
> >>
> >>     Signed-off-by: Jaroslav Kysela <perex at perex.cz>
> >>
> >> commit c45b73bf328cd8ace53cf39994328cf9d6548c4f
> >> Author:     Jaroslav Kysela <perex at perex.cz>
> >> AuthorDate: Wed Nov 11 13:43:01 2009 +0100
> >> Commit:     Jaroslav Kysela <perex at perex.cz>
> >> CommitDate: Thu Nov 12 10:51:16 2009 +0100
> >>
> >>     ALSA: hda - proc - introduce Control: lines to show mixer<->NID assignment
> >>
> >>     This is an initial patch to show universal control<->NID assigment in
> >>     proc codec file. The change helps to debug codec related problems.
> >>
> >>     Signed-off-by: Jaroslav Kysela <perex at perex.cz>
> >
> > I find the second one is the nice hack, but the first (newer) one
> > isn't good since it abuses the subdevice field of the ctl id.
> > It is a part of API/ABI, and if we do any changes the semantics, we
> > should define the changed behavior *beforehand* publicly.
> 
> The subdevice member IS NOT used (it's always zero) for mixer elements and 
> the value is not PASSED to the midlevel kernel API. Also, the 31. bit 
> checking ensures that subdevice contains right value.

Ah, OK, it's again zero-cleared.  Hrm, it's hackish but works.
But then please document this somewhere clearly.  Otherwise it can lead
to misunderstanding pretty easily like me.

> My idea was to keep the changes according the nid tracking at minimum (at 
> least it would be difficult to do major changes for static 
> snd_kcontrol_new arrays).

Speaking of patch size: the changes in patch_via.c can be reduced by
retrieving nid from the composed private_value.  I suppose all (or
almost all) callers of via_add_control() passe the value of
HDA_COMPOSE_AMP_VAL().

Similarly patch_realtek.c changes can be reduced, I guess.

> 
> > Also, both commits give many warnings via checkpatch.pl...
> > So I postpone the merge so far.
> 
> I'll fix that, if we agree how to go...

As long as the changes are local, I'm fine.  But let's try to reduce
the changes and a bit more clean up.


thanks,

Takashi