[Gta04-owner] Reliable headset detection with raw values

[Dr. H. Nikolaus Schaller]

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?

BR,
Nikolaus