[PATCH v4 1/3] dt-bindings: input: adc-keys bindings documentation

[Heinrich Schuchardt]

On 22.12.20 12:28, Heinrich Schuchardt wrote:
> On 12/22/20 11:12 AM, Heinrich Schuchardt wrote:
>> On 12/22/20 9:56 AM, Marek Szyprowski wrote:
>>> Dump adc-keys bindings documentation from Linux kernel source tree
>>> v5.10.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>>> ---
>>>   doc/device-tree-bindings/input/adc-keys.txt | 49 +++++++++++++++++++++
>>>   1 file changed, 49 insertions(+)
>>>   create mode 100644 doc/device-tree-bindings/input/adc-keys.txt
>>>
>>> diff --git a/doc/device-tree-bindings/input/adc-keys.txt
>>> b/doc/device-tree-bindings/input/adc-keys.txt
>>> new file mode 100644
>>> index 0000000000..e551814629
>>> --- /dev/null
>>> +++ b/doc/device-tree-bindings/input/adc-keys.txt
>>> @@ -0,0 +1,49 @@
>>> +ADC attached resistor ladder buttons
>>> +------------------------------------
>>> +
>>> +Required properties:
>>> + - compatible: "adc-keys"
>>> + - io-channels: Phandle to an ADC channel
>>> + - io-channel-names = "buttons";
>>> + - keyup-threshold-microvolt: Voltage at which all the keys are
>>> considered up.
>>> +
>>> +Optional properties:
>>> +    - poll-interval: Poll interval time in milliseconds
>>> +    - autorepeat: Boolean, Enable auto repeat feature of Linux input
>>> +      subsystem.
>>> +
>>> +Each button (key) is represented as a sub-node of "adc-keys":
>>> +
>>> +Required subnode-properties:
>>> +    - label: Descriptive name of the key.
>>> +    - linux,code: Keycode to emit.
>>> +    - press-threshold-microvolt: Voltage ADC input when this key is
>>> pressed.
>>
>> https://www.merriam-webster.com/dictionary/threshold
>> defines threshold as "a level, point, or value above which something is
>> true or will take place and below which it is not or will not"
>>
>> "when this key is pressed" leaves it completely open if a key is
>> considered pressed below or above the threshold. Please, replace the
>> word 'when' by either 'above which' or 'below which'.
>>
>> In the example keyup-threshold-microvolt is larger than
>> keyup-threshold-microvolt all values of press-threshold-microvolt. So
>> one might assume that 'above' is the intended meaning and the
>> interpretation of the example might be:
>>
>> 2.000.000 <= value: no key pressed
>> 1.500.000 <= value < 2.000.000: KEY_VOLUMEUP pressed
>> 1.000.000 <= value < 1.500.000: KEY_VOLUMEDOWN pressed
>> 500.000 <= value < 1.000.000: KEY_ENTER pressed
>> value < 500.000: no key pressed
>>
>> Both directions 'above' and 'below' make sense. So maybe if
>> keyup-threshold-microvolt is lower than all press-threshold-microvolt
>> you want to invert the logic?
>>
>> The binding lacks a hysteresis which is needed for a reliable function.
>>
>> If you look into drivers/input/keyboard/adc-keys.c in the Linux source,
>> you can see that it is not using the threshold value as a threshold at
>> all. Instead is uses abs(st->map[i].voltage - value) to find the nearest
>> "threshold" voltage level.
>>
>> Could you, please, try to bring this text into a form that cannot be
>> misinterpreted and reconcile with upstream. This should include
>>
>> * a results table for the example
>> * a definition if keyup-threshold-microvolt must be higher than all
>>    press-threshold-microvolt or may be lower than all
>>    press-threshold-microvolt
>> * a sentence forbidding keyup-threshold-microvolt to be between two
>>    press-threshold-microvolt
>> * a definition if or when any of the thresholds is a lower or upper
>>    boundary
>
> Cf. [PATCH 1/1] dt-bindings: adc-keys.txt: clarify description
> https://lore.kernel.org/linux-input/20201222110815.24121-1-xypron.glpk@gmx.de/T/#u

My patch has been accepted into Linux next.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20210118&id=698dc0cf944772a79a9aa417e647c0f7587e51df

Best regards

Heinrich

>>> +
>>> +Example:
>>> +
>>> +#include <dt-bindings/input/input.h>
>>> +
>>> +    adc-keys {
>>> +        compatible = "adc-keys";
>>> +        io-channels = <&lradc 0>;
>>> +        io-channel-names = "buttons";
>>> +        keyup-threshold-microvolt = <2000000>;
>>> +
>>> +        button-up {
>>> +            label = "Volume Up";
>>> +            linux,code = <KEY_VOLUMEUP>;
>>> +            press-threshold-microvolt = <1500000>;
>>> +        };
>>> +
>>> +        button-down {
>>> +            label = "Volume Down";
>>> +            linux,code = <KEY_VOLUMEDOWN>;
>>> +            press-threshold-microvolt = <1000000>;
>>> +        };
>>> +
>>> +        button-enter {
>>> +            label = "Enter";
>>> +            linux,code = <KEY_ENTER>;
>>> +            press-threshold-microvolt = <500000>;
>>> +        };
>>> +    };
>>>
>>
>