[PATCH v4 0/3] VIM3: add support for checking 'Function' button state
Hi All,
This patchset adds all building blocks needed for checking the 'Function' button state in the boot script on Amlogic A311D based VIM3 board. This button is connected to the ADC lines of the SoC, so it required to enable meson SARADC, the clocks needed for it and a simple button-adc drivers.
Once applied, one can use following commands in the boot scripts: -->8--- echo Checking Func button state: \c if button Function then echo Selected alternative boot ... fi --->8---
Best regards Marek Szyprowski Samsung R&D Institute Poland
Changelog: v4: - rebased onto uboot-amlogic/u-boot-amlogic-next and dropped merged patches - added adc-keys bindings docs (copied from Linux kernel) - minor code adjustments pointed by Simon - enabled driver also in khadas-vim3l_defconfig
v3: https://lists.denx.de/pipermail/u-boot/2020-December/435072.html - removed 'button' env variable - extended kconfig and patch descriptions
v2: https://lists.denx.de/pipermail/u-boot/2020-December/434991.html - removed Change-Id tags - split defconfig changes into ADC and button related
v1: https://lists.denx.de/pipermail/u-boot/2020-December/434875.html - initial submission
Patch summary:
Marek Szyprowski (3): dt-bindings: input: adc-keys bindings documentation button: add a simple Analog to Digital Converter device based button driver configs: khadas-vim3(l): enable Function button support
configs/khadas-vim3_defconfig | 2 + configs/khadas-vim3l_defconfig | 2 + doc/device-tree-bindings/input/adc-keys.txt | 49 ++++++++ drivers/button/Kconfig | 8 ++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++ 6 files changed, 183 insertions(+) create mode 100644 doc/device-tree-bindings/input/adc-keys.txt create mode 100644 drivers/button/button-adc.c
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. + +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>; + }; + };
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
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>;};- };
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.d...
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>; + }; + };
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@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.d...
My patch has been accepted into Linux next.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?...
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>; + }; + };
Add a simple Analog to Digital Converter device based button driver. This driver binds to the 'adc-keys' device tree node.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/button/Kconfig | 8 +++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 drivers/button/button-adc.c
diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig index 6b3ec7e55d..6db3c5e93a 100644 --- a/drivers/button/Kconfig +++ b/drivers/button/Kconfig @@ -9,6 +9,14 @@ config BUTTON can provide access to board-specific buttons. Use of the device tree for configuration is encouraged.
+config BUTTON_ADC + bool "Button adc" + depends on BUTTON + help + Enable support for buttons which are connected to Analog to Digital + Converter device. The ADC driver must use driver model. Buttons are + configured using the device tree. + config BUTTON_GPIO bool "Button gpio" depends on BUTTON diff --git a/drivers/button/Makefile b/drivers/button/Makefile index fcc10ebe8d..bbd18af149 100644 --- a/drivers/button/Makefile +++ b/drivers/button/Makefile @@ -3,4 +3,5 @@ # Copyright (C) 2020 Philippe Reynes philippe.reynes@softathome.com
obj-$(CONFIG_BUTTON) += button-uclass.o +obj-$(CONFIG_BUTTON_ADC) += button-adc.o obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c new file mode 100644 index 0000000000..bf99dd8b43 --- /dev/null +++ b/drivers/button/button-adc.c @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * Author: Marek Szyprowski m.szyprowski@samsung.com + */ + +#include <common.h> +#include <adc.h> +#include <button.h> +#include <dm.h> +#include <dm/lists.h> +#include <dm/uclass-internal.h> + +/** + * struct button_adc_priv - private data for button-adc driver. + * + * @adc: Analog to Digital Converter device to which button is connected. + * @channel: channel of the ADC device to probe the button state. + */ +struct button_adc_priv { + struct udevice *adc; + int channel; +}; + +static enum button_state_t button_adc_get_state(struct udevice *dev) +{ + struct button_adc_priv *priv = dev_get_priv(dev); + unsigned int val, mask; + int ret; + + ret = adc_start_channel(priv->adc, priv->channel); + if (ret) + return ret; + + ret = adc_channel_data(priv->adc, priv->channel, &val); + if (ret) + return ret; + + ret = adc_data_mask(priv->adc, &mask); + if (ret) + return ret; + + /* getting state is simplified a bit */ + if (ret == 0) + return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF; + + return ret; +} + +static int button_adc_probe(struct udevice *dev) +{ + struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev); + struct button_adc_priv *priv = dev_get_priv(dev); + struct ofnode_phandle_args args; + int ret; + + /* Ignore the top-level button node */ + if (!uc_plat->label) + return 0; + + ret = dev_read_phandle_with_args(dev->parent, "io-channels", + "#io-channel-cells", 0, 0, &args); + if (ret) + return ret; + + ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, &priv->adc); + if (ret) + return ret; + + priv->channel = args.args[0]; + + return ret; +} + +static int button_adc_bind(struct udevice *parent) +{ + struct udevice *dev; + ofnode node; + int ret; + + dev_for_each_subnode(node, parent) { + struct button_uc_plat *uc_plat; + const char *label; + + label = ofnode_read_string(node, "label"); + if (!label) { + debug("%s: node %s has no label\n", __func__, + ofnode_get_name(node)); + return -EINVAL; + } + ret = device_bind_driver_to_node(parent, "button_adc", + ofnode_get_name(node), + node, &dev); + if (ret) + return ret; + uc_plat = dev_get_uclass_platdata(dev); + uc_plat->label = label; + } + + return 0; +} + +static const struct button_ops button_adc_ops = { + .get_state = button_adc_get_state, +}; + +static const struct udevice_id button_adc_ids[] = { + { .compatible = "adc-keys" }, + { } +}; + +U_BOOT_DRIVER(button_adc) = { + .name = "button_adc", + .id = UCLASS_BUTTON, + .of_match = button_adc_ids, + .ops = &button_adc_ops, + .priv_auto_alloc_size = sizeof(struct button_adc_priv), + .bind = button_adc_bind, + .probe = button_adc_probe, +};
On 12/22/20 9:56 AM, Marek Szyprowski wrote:
Add a simple Analog to Digital Converter device based button driver. This driver binds to the 'adc-keys' device tree node.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/button/Kconfig | 8 +++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 drivers/button/button-adc.c
diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig index 6b3ec7e55d..6db3c5e93a 100644 --- a/drivers/button/Kconfig +++ b/drivers/button/Kconfig @@ -9,6 +9,14 @@ config BUTTON can provide access to board-specific buttons. Use of the device tree for configuration is encouraged.
+config BUTTON_ADC
- bool "Button adc"
- depends on BUTTON
- help
Enable support for buttons which are connected to Analog to DigitalConverter device. The ADC driver must use driver model. Buttons areconfigured using the device tree.- config BUTTON_GPIO bool "Button gpio" depends on BUTTON
diff --git a/drivers/button/Makefile b/drivers/button/Makefile index fcc10ebe8d..bbd18af149 100644 --- a/drivers/button/Makefile +++ b/drivers/button/Makefile @@ -3,4 +3,5 @@ # Copyright (C) 2020 Philippe Reynes philippe.reynes@softathome.com
obj-$(CONFIG_BUTTON) += button-uclass.o +obj-$(CONFIG_BUTTON_ADC) += button-adc.o obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c new file mode 100644 index 0000000000..bf99dd8b43 --- /dev/null +++ b/drivers/button/button-adc.c @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2020 Samsung Electronics Co., Ltd.
http://www.samsung.com
- Author: Marek Szyprowski m.szyprowski@samsung.com
- */
+#include <common.h> +#include <adc.h> +#include <button.h> +#include <dm.h> +#include <dm/lists.h> +#include <dm/uclass-internal.h>
+/**
- struct button_adc_priv - private data for button-adc driver.
- @adc: Analog to Digital Converter device to which button is connected.
- @channel: channel of the ADC device to probe the button state.
- */
+struct button_adc_priv {
- struct udevice *adc;
- int channel;
+};
+static enum button_state_t button_adc_get_state(struct udevice *dev) +{
- struct button_adc_priv *priv = dev_get_priv(dev);
- unsigned int val, mask;
- int ret;
- ret = adc_start_channel(priv->adc, priv->channel);
- if (ret)
return ret;- ret = adc_channel_data(priv->adc, priv->channel, &val);
- if (ret)
return ret;- ret = adc_data_mask(priv->adc, &mask);
- if (ret)
return ret;- /* getting state is simplified a bit */
- if (ret == 0)
return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;- return ret;
+}
+static int button_adc_probe(struct udevice *dev) +{
- struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
- struct button_adc_priv *priv = dev_get_priv(dev);
- struct ofnode_phandle_args args;
- int ret;
- /* Ignore the top-level button node */
- if (!uc_plat->label)
return 0;- ret = dev_read_phandle_with_args(dev->parent, "io-channels",
"#io-channel-cells", 0, 0, &args);- if (ret)
return ret;- ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, &priv->adc);
- if (ret)
return ret;- priv->channel = args.args[0];
- return ret;
+}
+static int button_adc_bind(struct udevice *parent) +{
- struct udevice *dev;
- ofnode node;
- int ret;
- dev_for_each_subnode(node, parent) {
struct button_uc_plat *uc_plat;const char *label;label = ofnode_read_string(node, "label");if (!label) {debug("%s: node %s has no label\n", __func__,ofnode_get_name(node));return -EINVAL;}ret = device_bind_driver_to_node(parent, "button_adc",ofnode_get_name(node),node, &dev);
This code does not match the binding. The binding defines a voltage ladder where you have multiple voltage ranges and multiple buttons.
You need to consider the threshold voltages.
Have a look at Linux' drivers/input/keyboard/adc-keys.c. Beware that code is also incorrect as the author ignores the meaning of "threshold" and uses it it as closest voltage.
Best regards
Heinrich
if (ret)return ret;uc_plat = dev_get_uclass_platdata(dev);uc_plat->label = label;- }
- return 0;
+}
+static const struct button_ops button_adc_ops = {
- .get_state = button_adc_get_state,
+};
+static const struct udevice_id button_adc_ids[] = {
- { .compatible = "adc-keys" },
- { }
+};
+U_BOOT_DRIVER(button_adc) = {
- .name = "button_adc",
- .id = UCLASS_BUTTON,
- .of_match = button_adc_ids,
- .ops = &button_adc_ops,
- .priv_auto_alloc_size = sizeof(struct button_adc_priv),
- .bind = button_adc_bind,
- .probe = button_adc_probe,
+};
Hi,
On Tue, 22 Dec 2020 at 02:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/22/20 9:56 AM, Marek Szyprowski wrote:
Add a simple Analog to Digital Converter device based button driver. This driver binds to the 'adc-keys' device tree node.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/button/Kconfig | 8 +++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 drivers/button/button-adc.c
diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig index 6b3ec7e55d..6db3c5e93a 100644 --- a/drivers/button/Kconfig +++ b/drivers/button/Kconfig @@ -9,6 +9,14 @@ config BUTTON can provide access to board-specific buttons. Use of the device tree for configuration is encouraged.
+config BUTTON_ADC
bool "Button adc"depends on BUTTONhelpEnable support for buttons which are connected to Analog to DigitalConverter device. The ADC driver must use driver model. Buttons areconfigured using the device tree.- config BUTTON_GPIO bool "Button gpio" depends on BUTTON
diff --git a/drivers/button/Makefile b/drivers/button/Makefile index fcc10ebe8d..bbd18af149 100644 --- a/drivers/button/Makefile +++ b/drivers/button/Makefile @@ -3,4 +3,5 @@ # Copyright (C) 2020 Philippe Reynes philippe.reynes@softathome.com
obj-$(CONFIG_BUTTON) += button-uclass.o +obj-$(CONFIG_BUTTON_ADC) += button-adc.o obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c new file mode 100644 index 0000000000..bf99dd8b43 --- /dev/null +++ b/drivers/button/button-adc.c @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2020 Samsung Electronics Co., Ltd.
http://www.samsung.com
- Author: Marek Szyprowski m.szyprowski@samsung.com
- */
+#include <common.h> +#include <adc.h> +#include <button.h> +#include <dm.h> +#include <dm/lists.h> +#include <dm/uclass-internal.h>
+/**
- struct button_adc_priv - private data for button-adc driver.
- @adc: Analog to Digital Converter device to which button is connected.
- @channel: channel of the ADC device to probe the button state.
- */
+struct button_adc_priv {
struct udevice *adc;int channel;+};
+static enum button_state_t button_adc_get_state(struct udevice *dev) +{
struct button_adc_priv *priv = dev_get_priv(dev);unsigned int val, mask;int ret;ret = adc_start_channel(priv->adc, priv->channel);if (ret)return ret;ret = adc_channel_data(priv->adc, priv->channel, &val);if (ret)return ret;ret = adc_data_mask(priv->adc, &mask);if (ret)return ret;/* getting state is simplified a bit */if (ret == 0)return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;return ret;+}
+static int button_adc_probe(struct udevice *dev) +{
struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);struct button_adc_priv *priv = dev_get_priv(dev);struct ofnode_phandle_args args;int ret;/* Ignore the top-level button node */if (!uc_plat->label)return 0;ret = dev_read_phandle_with_args(dev->parent, "io-channels","#io-channel-cells", 0, 0, &args);if (ret)return ret;ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, &priv->adc);if (ret)return ret;priv->channel = args.args[0];return ret;+}
+static int button_adc_bind(struct udevice *parent) +{
struct udevice *dev;ofnode node;int ret;dev_for_each_subnode(node, parent) {struct button_uc_plat *uc_plat;const char *label;label = ofnode_read_string(node, "label");if (!label) {debug("%s: node %s has no label\n", __func__,ofnode_get_name(node));return -EINVAL;}ret = device_bind_driver_to_node(parent, "button_adc",ofnode_get_name(node),node, &dev);This code does not match the binding. The binding defines a voltage ladder where you have multiple voltage ranges and multiple buttons.
You need to consider the threshold voltages.
Have a look at Linux' drivers/input/keyboard/adc-keys.c. Beware that code is also incorrect as the author ignores the meaning of "threshold" and uses it it as closest voltage.
Apart from that, from a DM point of view, this looks good.
Regards, Simon
Add options required to check the 'Function' button state.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- configs/khadas-vim3_defconfig | 2 ++ configs/khadas-vim3l_defconfig | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig index 5d16652fd6..bc17430569 100644 --- a/configs/khadas-vim3_defconfig +++ b/configs/khadas-vim3_defconfig @@ -31,6 +31,8 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_ADC=y CONFIG_SARADC_MESON=y +CONFIG_BUTTON=y +CONFIG_BUTTON_ADC=y CONFIG_DM_I2C=y CONFIG_SYS_I2C_MESON=y CONFIG_DM_MMC=y diff --git a/configs/khadas-vim3l_defconfig b/configs/khadas-vim3l_defconfig index 6b13ce045c..c1877922c7 100644 --- a/configs/khadas-vim3l_defconfig +++ b/configs/khadas-vim3l_defconfig @@ -31,6 +31,8 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_ADC=y CONFIG_SARADC_MESON=y +CONFIG_BUTTON=y +CONFIG_BUTTON_ADC=y CONFIG_DM_I2C=y CONFIG_SYS_I2C_MESON=y CONFIG_DM_MMC=y
Hi,
On 22/12/2020 09:56, Marek Szyprowski wrote:
Hi All,
This patchset adds all building blocks needed for checking the 'Function' button state in the boot script on Amlogic A311D based VIM3 board. This button is connected to the ADC lines of the SoC, so it required to enable meson SARADC, the clocks needed for it and a simple button-adc drivers.
Once applied, one can use following commands in the boot scripts: -->8--- echo Checking Func button state: \c if button Function then echo Selected alternative boot ... fi --->8---
Best regards Marek Szyprowski Samsung R&D Institute Poland
Changelog: v4:
- rebased onto uboot-amlogic/u-boot-amlogic-next and dropped merged patches
- added adc-keys bindings docs (copied from Linux kernel)
- minor code adjustments pointed by Simon
- enabled driver also in khadas-vim3l_defconfig
v3: https://lists.denx.de/pipermail/u-boot/2020-December/435072.html
- removed 'button' env variable
- extended kconfig and patch descriptions
v2: https://lists.denx.de/pipermail/u-boot/2020-December/434991.html
- removed Change-Id tags
- split defconfig changes into ADC and button related
v1: https://lists.denx.de/pipermail/u-boot/2020-December/434875.html
- initial submission
What's the state of the patchset ?
Neil
Patch summary:
Marek Szyprowski (3): dt-bindings: input: adc-keys bindings documentation button: add a simple Analog to Digital Converter device based button driver configs: khadas-vim3(l): enable Function button support
configs/khadas-vim3_defconfig | 2 + configs/khadas-vim3l_defconfig | 2 + doc/device-tree-bindings/input/adc-keys.txt | 49 ++++++++ drivers/button/Kconfig | 8 ++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++ 6 files changed, 183 insertions(+) create mode 100644 doc/device-tree-bindings/input/adc-keys.txt create mode 100644 drivers/button/button-adc.c
On 18.01.21 11:24, Neil Armstrong wrote:
Hi,
On 22/12/2020 09:56, Marek Szyprowski wrote:
Hi All,
This patchset adds all building blocks needed for checking the 'Function' button state in the boot script on Amlogic A311D based VIM3 board. This button is connected to the ADC lines of the SoC, so it required to enable meson SARADC, the clocks needed for it and a simple button-adc drivers.
Once applied, one can use following commands in the boot scripts: -->8--- echo Checking Func button state: \c if button Function then echo Selected alternative boot ... fi --->8---
Best regards Marek Szyprowski Samsung R&D Institute Poland
Changelog: v4:
- rebased onto uboot-amlogic/u-boot-amlogic-next and dropped merged patches
- added adc-keys bindings docs (copied from Linux kernel)
- minor code adjustments pointed by Simon
- enabled driver also in khadas-vim3l_defconfig
v3: https://lists.denx.de/pipermail/u-boot/2020-December/435072.html
- removed 'button' env variable
- extended kconfig and patch descriptions
v2: https://lists.denx.de/pipermail/u-boot/2020-December/434991.html
- removed Change-Id tags
- split defconfig changes into ADC and button related
v1: https://lists.denx.de/pipermail/u-boot/2020-December/434875.html
- initial submission
What's the state of the patchset ?
Hello Neil,
the series is assigned to you. Patch 2 is incorrect. Patch 1 can be updated to match Linux. So I suggest to set the status for the series to "changes requested".
Best regards
Heinrich
Neil
Patch summary:
Marek Szyprowski (3): dt-bindings: input: adc-keys bindings documentation button: add a simple Analog to Digital Converter device based button driver configs: khadas-vim3(l): enable Function button support
configs/khadas-vim3_defconfig | 2 + configs/khadas-vim3l_defconfig | 2 + doc/device-tree-bindings/input/adc-keys.txt | 49 ++++++++ drivers/button/Kconfig | 8 ++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++ 6 files changed, 183 insertions(+) create mode 100644 doc/device-tree-bindings/input/adc-keys.txt create mode 100644 drivers/button/button-adc.c
Hi,
On 18/01/2021 13:48, Heinrich Schuchardt wrote:
On 18.01.21 11:24, Neil Armstrong wrote:
Hi,
On 22/12/2020 09:56, Marek Szyprowski wrote:
Hi All,
This patchset adds all building blocks needed for checking the 'Function' button state in the boot script on Amlogic A311D based VIM3 board. This button is connected to the ADC lines of the SoC, so it required to enable meson SARADC, the clocks needed for it and a simple button-adc drivers.
Once applied, one can use following commands in the boot scripts: -->8--- echo Checking Func button state: \c if button Function then echo Selected alternative boot ... fi --->8---
Best regards Marek Szyprowski Samsung R&D Institute Poland
Changelog: v4:
- rebased onto uboot-amlogic/u-boot-amlogic-next and dropped merged patches
- added adc-keys bindings docs (copied from Linux kernel)
- minor code adjustments pointed by Simon
- enabled driver also in khadas-vim3l_defconfig
v3: https://lists.denx.de/pipermail/u-boot/2020-December/435072.html
- removed 'button' env variable
- extended kconfig and patch descriptions
v2: https://lists.denx.de/pipermail/u-boot/2020-December/434991.html
- removed Change-Id tags
- split defconfig changes into ADC and button related
v1: https://lists.denx.de/pipermail/u-boot/2020-December/434875.html
- initial submission
What's the state of the patchset ?
Hello Neil,
the series is assigned to you. Patch 2 is incorrect. Patch 1 can be updated to match Linux. So I suggest to set the status for the series to "changes requested".
Sure, it was more a question to Marek, but yeah I'll tag them "changes requested"
Neil
Best regards
Heinrich
Neil
Patch summary:
Marek Szyprowski (3): dt-bindings: input: adc-keys bindings documentation button: add a simple Analog to Digital Converter device based button driver configs: khadas-vim3(l): enable Function button support
configs/khadas-vim3_defconfig | 2 + configs/khadas-vim3l_defconfig | 2 + doc/device-tree-bindings/input/adc-keys.txt | 49 ++++++++ drivers/button/Kconfig | 8 ++ drivers/button/Makefile | 1 + drivers/button/button-adc.c | 121 ++++++++++++++++++++ 6 files changed, 183 insertions(+) create mode 100644 doc/device-tree-bindings/input/adc-keys.txt create mode 100644 drivers/button/button-adc.c
Hi Neil,
On 18.01.2021 13:55, Neil Armstrong wrote:
On 18/01/2021 13:48, Heinrich Schuchardt wrote:
On 18.01.21 11:24, Neil Armstrong wrote:
On 22/12/2020 09:56, Marek Szyprowski wrote:
Hi All,
This patchset adds all building blocks needed for checking the 'Function' button state in the boot script on Amlogic A311D based VIM3 board. This button is connected to the ADC lines of the SoC, so it required to enable meson SARADC, the clocks needed for it and a simple button-adc drivers.
Once applied, one can use following commands in the boot scripts: -->8--- echo Checking Func button state: \c if button Function then echo Selected alternative boot ... fi --->8---
Best regards Marek Szyprowski Samsung R&D Institute Poland
Changelog: v4:
- rebased onto uboot-amlogic/u-boot-amlogic-next and dropped merged patches
- added adc-keys bindings docs (copied from Linux kernel)
- minor code adjustments pointed by Simon
- enabled driver also in khadas-vim3l_defconfig
v3: https://protect2.fireeye.com/v1/url?k=fc4a4b97-a3d1734e-fc4bc0d8-0cc47a3356b...
- removed 'button' env variable
- extended kconfig and patch descriptions
v2: https://protect2.fireeye.com/v1/url?k=11f0ed69-4e6bd5b0-11f16626-0cc47a3356b...
- removed Change-Id tags
- split defconfig changes into ADC and button related
v1: https://protect2.fireeye.com/v1/url?k=7c601119-23fb29c0-7c619a56-0cc47a3356b...
- initial submission
What's the state of the patchset ?
Hello Neil,
the series is assigned to you. Patch 2 is incorrect. Patch 1 can be updated to match Linux. So I suggest to set the status for the series to "changes requested".
Sure, it was more a question to Marek, but yeah I'll tag them "changes requested"
I'm sorry for the late reply, I've been busy with other stuff. I will try to update the adc-keys driver to use the threshold values and add the updated device tree bindings soon.
Best regards
participants (4)
-
Heinrich Schuchardt -
Marek Szyprowski -
Neil Armstrong -
Simon Glass