[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Marek Szyprowski]

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

-- 
2.17.1

[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Marek Szyprowski]

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

-- 
2.17.1

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

[Marek Szyprowski]

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>;
+		};
+	};
-- 
2.17.1

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

[Marek Szyprowski]

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>;
+		};
+	};
-- 
2.17.1

[PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver

[Marek Szyprowski]

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,
+};
-- 
2.17.1

[PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver

[Marek Szyprowski]

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,
+};
-- 
2.17.1

[PATCH v4 3/3] configs: khadas-vim3(l): enable Function button support

[Marek Szyprowski]

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
-- 
2.17.1

[PATCH v4 3/3] configs: khadas-vim3(l): enable Function button support

[Marek Szyprowski]

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
-- 
2.17.1

[PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver

[Heinrich Schuchardt]

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 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);

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,
> +};
>

Re: [PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver

[Heinrich Schuchardt]

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 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);

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,
> +};
>

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

[Heinrich Schuchardt]

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>;
> +		};
> +	};
>

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

[Heinrich Schuchardt]

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>;
> +		};
> +	};
>

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

[Heinrich Schuchardt]

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>;
>> +        };
>> +    };
>>
>

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

[Heinrich Schuchardt]

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>;
>> +??????? };
>> +??? };
>>
>

[PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver

[Simon Glass]

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 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);
>
> 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

Re: [PATCH v4 2/3] button: add a simple Analog to Digital Converter device based button driver

[Simon Glass]

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 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);
>
> 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

[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Neil Armstrong]

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
> 

Re: [PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Neil Armstrong]

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
> 

[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@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

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>;
>>> +??????? };
>>> +??? };
>>>
>>
>

Re: [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@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>;
>>> +        };
>>> +    };
>>>
>>
>

[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Heinrich Schuchardt]

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
>>
>

Re: [PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Heinrich Schuchardt]

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
>>
>

[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Neil Armstrong]

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
>>>
>>
> 

Re: [PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Neil Armstrong]

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
>>>
>>
> 

[PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Marek Szyprowski]

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-0cc47a3356b2-eb4ba315795a3f17&q=1&e=b660ed0e-7d37-47e8-877d-c8391317e3e0&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2020-December%2F435072.html
>>>> - removed 'button' env variable
>>>> - extended kconfig and patch descriptions
>>>>
>>>> v2: https://protect2.fireeye.com/v1/url?k=11f0ed69-4e6bd5b0-11f16626-0cc47a3356b2-f341d16b01448534&q=1&e=b660ed0e-7d37-47e8-877d-c8391317e3e0&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2020-December%2F434991.html
>>>> - removed Change-Id tags
>>>> - split defconfig changes into ADC and button related
>>>>
>>>> v1: https://protect2.fireeye.com/v1/url?k=7c601119-23fb29c0-7c619a56-0cc47a3356b2-55caa8afe3085a65&q=1&e=b660ed0e-7d37-47e8-877d-c8391317e3e0&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2020-December%2F434875.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"

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v4 0/3] VIM3: add support for checking 'Function' button state

[Marek Szyprowski]

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-0cc47a3356b2-eb4ba315795a3f17&q=1&e=b660ed0e-7d37-47e8-877d-c8391317e3e0&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2020-December%2F435072.html
>>>> - removed 'button' env variable
>>>> - extended kconfig and patch descriptions
>>>>
>>>> v2: https://protect2.fireeye.com/v1/url?k=11f0ed69-4e6bd5b0-11f16626-0cc47a3356b2-f341d16b01448534&q=1&e=b660ed0e-7d37-47e8-877d-c8391317e3e0&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2020-December%2F434991.html
>>>> - removed Change-Id tags
>>>> - split defconfig changes into ADC and button related
>>>>
>>>> v1: https://protect2.fireeye.com/v1/url?k=7c601119-23fb29c0-7c619a56-0cc47a3356b2-55caa8afe3085a65&q=1&e=b660ed0e-7d37-47e8-877d-c8391317e3e0&u=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2020-December%2F434875.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"

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland