From: Heinrich Schuchardt <xypron.glpk@gmx.de> To: Marek Szyprowski <m.szyprowski@samsung.com>, u-boot@lists.denx.de, u-boot-amlogic@groups.io Cc: Neil Armstrong <narmstrong@baylibre.com>, Lukasz Majewski <lukma@denx.de>, Philippe Reynes <philippe.reynes@softathome.com>, Simon Glass <sjg@chromium.org>, Jaehoon Chung <jh80.chung@samsung.com>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Rob Herring <robh@kernel.org>, Alexandre Belloni <alexandre.belloni@bootlin.com>, devicetree@vger.kernel.org, Dmitry Torokhov <dmitry.torokhov@gmail.com>, lkml <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v4 1/3] dt-bindings: input: adc-keys bindings documentation Date: Tue, 22 Dec 2020 12:28:06 +0100 [thread overview] Message-ID: <f8930b5a-e1eb-7c41-19ad-6ba539c59322@gmx.de> (raw) In-Reply-To: <1b82e9ef-e8af-87e1-ea68-d71d65c2aa8a@gmx.de> 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@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 Best regards Heinrich > > 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>; >> + }; >> + }; >> >
WARNING: multiple messages have this Message-ID (diff)
From: Heinrich Schuchardt <xypron.glpk@gmx.de> To: u-boot@lists.denx.de Subject: [PATCH v4 1/3] dt-bindings: input: adc-keys bindings documentation Date: Tue, 22 Dec 2020 12:28:06 +0100 [thread overview] Message-ID: <f8930b5a-e1eb-7c41-19ad-6ba539c59322@gmx.de> (raw) In-Reply-To: <1b82e9ef-e8af-87e1-ea68-d71d65c2aa8a@gmx.de> 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@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 at gmx.de/T/#u Best regards Heinrich > > 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>; >> +??????? }; >> +??? }; >> >
next prev parent reply other threads:[~2020-12-22 11:29 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20201222085638eucas1p158d91ce4a22d13623b19706b26374078@eucas1p1.samsung.com> 2020-12-22 8:56 ` [PATCH v4 0/3] VIM3: add support for checking 'Function' button state Marek Szyprowski 2020-12-22 8:56 ` Marek Szyprowski [not found] ` <CGME20201222085638eucas1p14d9d9da136593a12fea0140c403095c4@eucas1p1.samsung.com> 2020-12-22 8:56 ` [PATCH v4 1/3] dt-bindings: input: adc-keys bindings documentation Marek Szyprowski 2020-12-22 8:56 ` Marek Szyprowski 2020-12-22 10:12 ` Heinrich Schuchardt 2020-12-22 10:12 ` Heinrich Schuchardt 2020-12-22 11:28 ` Heinrich Schuchardt [this message] 2020-12-22 11:28 ` Heinrich Schuchardt 2021-01-18 12:45 ` Heinrich Schuchardt 2021-01-18 12:45 ` Heinrich Schuchardt [not found] ` <CGME20201222085639eucas1p1db16b6bc2ae790ed711a09bcc5f176e5@eucas1p1.samsung.com> 2020-12-22 8:56 ` [PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver Marek Szyprowski 2020-12-22 8:56 ` Marek Szyprowski 2020-12-22 9:45 ` Heinrich Schuchardt 2020-12-22 9:45 ` Heinrich Schuchardt 2020-12-29 3:31 ` Simon Glass 2020-12-29 3:31 ` Simon Glass [not found] ` <CGME20201222085640eucas1p295da269be60f5193ebd2ebc027a668d2@eucas1p2.samsung.com> 2020-12-22 8:56 ` [PATCH v4 3/3] configs: khadas-vim3(l): enable Function button support Marek Szyprowski 2020-12-22 8:56 ` Marek Szyprowski 2021-01-18 10:24 ` [PATCH v4 0/3] VIM3: add support for checking 'Function' button state Neil Armstrong 2021-01-18 10:24 ` Neil Armstrong 2021-01-18 12:48 ` Heinrich Schuchardt 2021-01-18 12:48 ` Heinrich Schuchardt 2021-01-18 12:55 ` Neil Armstrong 2021-01-18 12:55 ` Neil Armstrong 2021-01-22 12:42 ` Marek Szyprowski 2021-01-22 12:42 ` Marek Szyprowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=f8930b5a-e1eb-7c41-19ad-6ba539c59322@gmx.de \ --to=xypron.glpk@gmx.de \ --cc=alexandre.belloni@bootlin.com \ --cc=b.zolnierkie@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=dmitry.torokhov@gmail.com \ --cc=jh80.chung@samsung.com \ --cc=linux-kernel@vger.kernel.org \ --cc=lukma@denx.de \ --cc=m.szyprowski@samsung.com \ --cc=narmstrong@baylibre.com \ --cc=philippe.reynes@softathome.com \ --cc=robh@kernel.org \ --cc=sjg@chromium.org \ --cc=u-boot-amlogic@groups.io \ --cc=u-boot@lists.denx.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.