Re: [RFC PATCH] HDA: Generic input jack handling

[Takashi Iwai <tiwai@suse.de>]

At Sun, 09 Oct 2011 10:38:57 +0200,
David Henningsson wrote:
> 
> On 10/08/2011 09:11 AM, Takashi Iwai wrote:
> > At Fri, 07 Oct 2011 18:04:37 +0200,
> > David Henningsson wrote:
> >>
> >> On 10/07/2011 05:27 PM, Takashi Iwai wrote:
> >>> That being said, I'm inclined to delay it for 3.3.  Now is so close to
> >>> the merge window and we don't need to rush for a refactoring patch.
> >>> It can be done more safely in the early development cycle.
> >>> Sorry if it disappoints you.
> >>
> >> This is much more than a refactoring patch - Realtek has no input jacks
> >> for everything but headphones (and mics only if autoswitch is enabled),
> >> so this is a good new feature. And my planned next step was to add it to
> >> patch_via, which currently does not have input jacks at all.
> >
> > OK.
> >
> >> To give you some background here. As you might know, I've written
> >> patches for PulseAudio to use these input jacks, that's why they're
> >> suddenly so much more important. Also, it is not certain whether the
> >> next release of Ubuntu (12.04, released April next year) will use kernel
> >> 3.2 or 3.3. Should it be 3.3, this is not a problem, but if it is 3.2,
> >> it would be better if all this was working upstream. Should it not be
> >> working, I'll need to apply it for Ubuntu only instead.
> >
> > Well, then really you shouldn't be rushing too much.
> > As mentioned, the API function will be needed to be changed because
> > it's wrong to call with jack_type.  So, if you merge it in 3.2, you'll
> > get anyway API/ABI incompatibility for further changes.
> >
> > That is, if it were a self-contained patch only in patch_realtek.c, I
> > wouldn't mind too much.  But it's an addition of API in HD-audio code
> > code, so I do care about it.
> 
> I don't like copy-pasting the same code into several vendor's patch 
> files when the function should really be generic. Therefore I don't want 
> to make several self-contained patches.

I understand it well.  But, when you think of the generic code, first
think of porting the existing portions to the generic API.  Then
you'll notice that the code in patch_sigmatel.c doesn't fit with your
design as it is.  It'll need modifications in both sides.

> So if you feel uncomfortable with having the generic function in 3.2, 
> queue it up for 3.3 (after I've changed the jack_type stuff to your 
> preference) and I'll try to backport it into Ubuntu's 3.2 kernel, should 
> it become necessary.

Don't get me wrong.  If the addition were a rock-solid API that can
cover all existing cases, I'm willing to add it for 3.2 even now.
But the API design and the implementation look still having some rooms
for reconsideration.  That's why I hesitate for 3.2 merge.

Actually, I like the idea to let the driver parsing autocfg for all
pins.  But, as mentioned, for the proper implementation, we'll need a
certain mapping among the NID, the unsol tag and the event
(jack-reporting).

Then, thinking of the fact that other unsol handlers also need more or
less such a mapping, an idea like a generic unsol-event dispatcher
comes to mind.  For example, imagine a table of NID containing the
corresponding tag id, and the list of function pointers to call (and
closures).  The normal unsol event would be registered here, and any
input-jack events would be additionally registered to the same
NID/tag.  Then, upon unsol events, the driver simply calls the
dispatcher with the tag, and the dispatcher calls the functions
assigned to the tag.

Of course, there can be a better implementation for such a purpose.
My point is that, when you look a bit forward, you'll see the
direction of the further implementations.  If the current
implementation matches well with such a forecast, we can merge the
stuff ASAP, yes.  But, if it isn't clear, better to stand and look
around first.


thanks,

Takashi