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