Re: [PATCH] ALSA: hda - Enable mute/mic-mute LEDs for more Thinkpads with Conexant codec

[Hui Wang <hui.wang@canonical.com>]

On 11/27/2013 03:23 PM, Takashi Iwai wrote:
> At Wed, 27 Nov 2013 15:11:41 +0800,
> Hui Wang wrote:
>> On 11/27/2013 02:44 PM, Takashi Iwai wrote:
>>> At Wed, 27 Nov 2013 06:17:16 +0100,
>>> David Henningsson wrote:
>>>> On 11/27/2013 04:26 AM, Hui Wang wrote:
>>>>> Most Thinkpad Edge series laptops use conexant codec, so far although
>>>>> the codecs have different minor Vendor Id and minor Subsystem Id, but
>>>>> they all belong to the cxt_5066 family, this change can make the
>>>>> mute/mic-mute LEDs support more generic among cxt_5066 family.
>>>> Hi Hui and thanks for your patch!
>>>>
>>>> It's also nice that you have tested the patch on several machines and
>>>> found it to be working.
>>>>
>>>> As for the actual implementation - as the rest of the thinkpad-acpi code
>>>> is copy-pasted between patch_realtek and patch_conexant, I think this
>>>> would also apply here, i e, no code in hda_local.h, instead copy from
>>>> patch_realtek to patch_conexant as necessary.
>>>>
>>>> While copy-pasting code is not the most elegant of solutions, I got the
>>>> impression from Takashi that he preferred that for now.
>>> If you'd like it to be included in 3.13, please just copy the small
>>> code snippet.
>> Yes we hope it can be merged to 3.13, so as your request, i just copy
>> the small code snippet in the V2 patch.
>>> For 3.14, we can move the thinkpad-related code into another file, but
>>> I have some bad feeling to put it into hda_local.h or whatever the
>>> common code base.
>>> Maybe just create hda_thinkpad.c or such and include from both
>>> patch_realtek.c and patch_conexant.c.  As long as the code size is
>>> small enough and they are all static, it should be fine.
>> Good suggestion, if we have more machines to test and the testing expose
>> more problems,
>> we should think about moving the common functions to the hda_thinkpad.c.
> Hrm, this comment makes me worry a bit to include into 3.13.
> Hasn't been the patch tested fully?  The merge window was already
> closed, and it means that we really need only fixes, the ones that
> won't regress obviously...
Sorry to make you misunderstand, this patch definitely works well on the 
machines with Conexant codec (cxt5066 family).  I have tested this patch 
on  9 laptops whose codecs all belong to cxt5066 family, they all worked 
well.

The problem i pointed to was that, assuming we meet a machine with 
conexant codec in future, but this codec doesn't belong to cxt5066 
family, this patch will not work on this machine, but this patch will 
not introduce regress on this machine as well.

Regards,
Hui.
>
> Takashi
>
>
>>