[Gta04-owner] Reliable headset detection with raw values

[Dr. H. Nikolaus Schaller]

Am 24.05.2014 um 16:58 schrieb Paul Kocialkowski:

> Le samedi 24 mai 2014 à 16:28 +0200, Dr. H. Nikolaus Schaller a écrit :
>> Am 24.05.2014 um 16:16 schrieb Paul Kocialkowski:
>> 
>>> Le samedi 24 mai 2014 à 14:55 +0200, Lukas Maerdian a écrit :
>>>> On 24.05.2014 14:14 UTC+0200, Paul Kocialkowski wrote:
>>>>>>> This doesn't make much sense to me.
>>>>>>> Rather than guessing I looked at the code, and the effect of setting 'raw'
>>>>>>> is to bypass
>>>>>>> 
>>>>>>> 			buf[i] = (buf[i] * 3 * 1000 *
>>>>>>> 				 twl4030_divider_ratios[i].denominator)
>>>>>>> 				/ (2 * 1023 *
>>>>>>> 				twl4030_divider_ratios[i].numerator);
>>>>>>> 
>>>>>>> where 'i' is 7, and twl4030_divider_ratios[7] is
>>>>>>> 
>>>>>>> 	{6, 10},	/* CHANNEL 7 */
>>>>>>> 
>>>>>>> So this becomes
>>>>>>>  buf[i] * 30000 / 12276
>>>>>>> 
>>>>>>> so it rough multiplies the numbers be 2.5.  That shouldn't affect stability
>>>>>>> at all.
>>>>>> 
>>>>>> Yes, you're perfectly right, I didn't read twl4030_madc_read_channels
>>>>>> completely and just assumed it was changing the range of values in a
>>>>>> dynamic way (for instance, using calibration), but obviously, I was
>>>>>> wrong. It really did seem like it was improving stability (for instance,
>>>>>> I had values that drastically changed from one reboot to the other, like
>>>>>> 800 to 400, without raw), but then it was late when I did this and I
>>>>>> jumped to conclusions too fast, without learning about the details.
>>>>>> 
>>>>>> I'll revert the change in the Replicant 4.2 tree and use thresholds
>>>>>> based on the non-raw values.
>>>>> 
>>>>> Turns out I wasn't so crazy after all! My patch did introduce better
>>>>> stability because the raw element of the request structure wasn't
>>>>> initialized previously, which lead to randomly either raw or processed
>>>>> values, hence the different ranges of values depending on the day, the
>>>>> device, etc.
>>>>> 
>>>>> I've just pushed a patch to fix this in the Replicant 4.2 kernel[0]:
>>>>> commit 2421efd8de974fee6a06e45ca31d4ed6f4805573
>>>>> Author: Paul Kocialkowski <contact at paulk.fr>
>>>>> Date:   Sat May 24 13:08:29 2014 +0200
>>>>> 
>>>>>   twl4030-madc: Request processed values in twl4030_get_madc_conversion
>>>>> 
>>>>>   Not setting the raw parameter in the request causes it to be randomly
>>>>>   initialized to a value that might be different from zero or zero. This leads to
>>>>>   values that are randomly either raw or processed, making it very difficult to
>>>>>   make reliable use of the values.
>>>>> 
>>>>>   Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
>>>>> 
>>>>> I suggest you include that patch in the other kernel trees. I've also
>>>>> proposed a similar patch for mainline, so that this is not an issue for
>>>>> anyone in the future!
>>>>> 
>>>>> By the way, the Replicant kernel is not using raw values anymore, and
>>>>> I've adapted the thresholds[1]:
>>>>> 
>>>>> commit 47d0bd23e540b5fd8dcf4d722a8309d24ce9ee56
>>>>> Author: Paul Kocialkowski <contact at paulk.fr>
>>>>> Date:   Sat May 24 13:15:14 2014 +0200
>>>>> 
>>>>>   gta04-audio: Jack detection thresholds for processed values
>>>>> 
>>>>>   Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
>>>>> 
>>>>> [0]:
>>>>> https://gitorious.org/replicant/kernel_goldelico_gta04/commit/2421efd8de974fee6a06e45ca31d4ed6f4805573
>>>>> [1]:
>>>>> https://gitorious.org/replicant/kernel_goldelico_gta04/commit/47d0bd23e540b5fd8dcf4d722a8309d24ce9ee56
>>>> 
>>>> Thanks for the heads-up!
>>>> I've included the two patches into the Goldelico 3.12-replicant4 kernel
>>>> and Nikolaus might eventually cherry-pick your raw value fix
>>>> (2421efd8de974fee6a06e45ca31d4ed6f4805573) into 3.12.7 and 3.15-rc
>>> 
>>> Note that the patch is different for 3.15-rc, since the driver was moved
>>> to some other place. The correct one is:
>>> http://marc.info/?l=linux-iio&m=140093393003482&w=2
>> 
>> Yes, it was moved around in March.
>>> 
>>> Perhaps it'll even get integrated in mainline soon, so Nikolaus won't
>>> have to cherry-pick!
>> 
>> Many thanks for spotting this!. Finding such single-line bugs is more challenging
>> (and rewarding) than writing 100 lines of new code...
>> 
>> For easiest cherry-picking, could you please do a git format-patch?
> 
> Of course, please find the patch attached to this message.

Thanks! I have applied it to our 3.15-rc6.

It looks as if we are the first board which really makes use of this function:

http://lxr.free-electrons.com/ident?v=3.14;i=twl4030_get_madc_conversion

BR,
Nikolaus