[alsa-devel] [PATCH] snd-hda-intel: Jack Mode changes for Sigmatel boards

[Nickolas Lloyd]

Takashi Iwai wrote:
> Hi,
>
> sorry for the late review.  Some comments below.
>   
No worries.  I know you're busy, and I appreciate your time and help :)

>> +        i = 3;
>> +    else
>> +        i = 2;
>> +
>> +    uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
>> +    uinfo->value.enumerated.items = i;
>> +    uinfo->count = 1;
>> +    if (uinfo->value.enumerated.item >= i)
>> +        uinfo->value.enumerated.item = i-1;
>>     
>
> Didn't checkpatch.pl complain?  :)
>   
It didn't actually.  What should it have complained about, so I can fix it?
> Well, these two functions, stac92xx_vref_get() and stac92xx_get_vref()
> are too confusing.  We have to rename one of them, at least, or at best
> clean up a bit more.
I agree.  stac92xx_vref_get() and stac92xx_vref_set() are a pair, so
perhaps they should stay as they are, and stac92xx_get_vref() be renamed
to stac92xx_get_default_vref() or something?

>
> Hmm, this code flow is too complex and hackish to follow.
> There must be a better way.  For example, the complicated neted if
> checks can be put out to a function.  The compiler is clever enough to
> inline, and here is no critical code path for speed.
>
>
> Could you fix and repost please?
>
>
> thanks,
>
> Takashi
>
>   
I've added a function stac92xx_add_mic_jack_control() to take care of
the if's and add the control.  The code is much cleaner now IMO.  Is
this function name ok, and do you have any other thoughts?

I'll be posting the new patch soon so that you can actually look at that
as well.

Thanks,
Nick