[PATCH v5 0/4] VIM3: add support for checking 'Function' button state

[PATCH v5 0/4] 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:
v5:
- rebased onto latest uboot-amlogic/u-boot-amlogic-next branch
- synchronized adc-keys binding with the recent version from the Linux
  kernel
- updated adc-keys driver to match behavior from dt-bindings
- added a patch for meson-saradc driver to register vdd reference supply
  to the ADC framework

v4: https://lists.denx.de/pipermail/u-boot/2020-December/435641.html
- 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 (4):
  dt-bindings: input: adc-keys bindings documentation
  button: add a simple Analog to Digital Converter device based button
    driver
  adc: meson-saradc: add support for getting reference voltage value
  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 |  67 +++++++++
 drivers/adc/meson-saradc.c                  |  21 +++
 drivers/button/Kconfig                      |   8 +
 drivers/button/Makefile                     |   1 +
 drivers/button/button-adc.c                 | 156 ++++++++++++++++++++
 7 files changed, 257 insertions(+)
 create mode 100644 doc/device-tree-bindings/input/adc-keys.txt
 create mode 100644 drivers/button/button-adc.c

-- 
2.17.1

[PATCH v5 0/4] 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:
v5:
- rebased onto latest uboot-amlogic/u-boot-amlogic-next branch
- synchronized adc-keys binding with the recent version from the Linux
  kernel
- updated adc-keys driver to match behavior from dt-bindings
- added a patch for meson-saradc driver to register vdd reference supply
  to the ADC framework

v4: https://lists.denx.de/pipermail/u-boot/2020-December/435641.html
- 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 (4):
  dt-bindings: input: adc-keys bindings documentation
  button: add a simple Analog to Digital Converter device based button
    driver
  adc: meson-saradc: add support for getting reference voltage value
  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 |  67 +++++++++
 drivers/adc/meson-saradc.c                  |  21 +++
 drivers/button/Kconfig                      |   8 +
 drivers/button/Makefile                     |   1 +
 drivers/button/button-adc.c                 | 156 ++++++++++++++++++++
 7 files changed, 257 insertions(+)
 create mode 100644 doc/device-tree-bindings/input/adc-keys.txt
 create mode 100644 drivers/button/button-adc.c

-- 
2.17.1

[PATCH v5 1/4] dt-bindings: input: adc-keys bindings documentation

[Marek Szyprowski]

Dump adc-keys bindings documentation from Linux kernel source tree from
commit 698dc0cf9447 ("dt-bindings: input: adc-keys: clarify
description").

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 doc/device-tree-bindings/input/adc-keys.txt | 67 +++++++++++++++++++++
 1 file changed, 67 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..6c8be6a9ac
--- /dev/null
+++ b/doc/device-tree-bindings/input/adc-keys.txt
@@ -0,0 +1,67 @@
+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 above or equal to 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 above or equal to which this key is
+				     considered pressed.
+
+No two values of press-threshold-microvolt may be the same.
+All values of press-threshold-microvolt must be less than
+keyup-threshold-microvolt.
+
+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.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         |
++--------------------------------+------------------------+
-- 
2.17.1

[PATCH v5 1/4] dt-bindings: input: adc-keys bindings documentation

[Marek Szyprowski]

Dump adc-keys bindings documentation from Linux kernel source tree from
commit 698dc0cf9447 ("dt-bindings: input: adc-keys: clarify
description").

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 doc/device-tree-bindings/input/adc-keys.txt | 67 +++++++++++++++++++++
 1 file changed, 67 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..6c8be6a9ac
--- /dev/null
+++ b/doc/device-tree-bindings/input/adc-keys.txt
@@ -0,0 +1,67 @@
+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 above or equal to 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 above or equal to which this key is
+				     considered pressed.
+
+No two values of press-threshold-microvolt may be the same.
+All values of press-threshold-microvolt must be less than
+keyup-threshold-microvolt.
+
+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.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         |
++--------------------------------+------------------------+
-- 
2.17.1

[PATCH v5 2/4] 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 | 156 ++++++++++++++++++++++++++++++++++++
 3 files changed, 165 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..1901d59a0e
--- /dev/null
+++ b/drivers/button/button-adc.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 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 <log.h>
+#include <dm.h>
+#include <dm/lists.h>
+#include <dm/of_access.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.
+ * @min: minimal raw ADC sample value to consider button as pressed.
+ * @max: maximal raw ADC sample value to consider button as pressed.
+ */
+struct button_adc_priv {
+	struct udevice *adc;
+	int channel;
+	int min;
+	int max;
+};
+
+static enum button_state_t button_adc_get_state(struct udevice *dev)
+{
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	unsigned int val;
+	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;
+
+	if (ret == 0)
+		return (val >= priv->min && val < priv->max) ?
+			BUTTON_ON : BUTTON_OFF;
+
+	return ret;
+}
+
+static int button_adc_probe(struct udevice *dev)
+{
+	struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	struct ofnode_phandle_args args;
+	u32 treshold, up_treshold, t;
+	unsigned int mask;
+	ofnode node;
+	int ret, vdd;
+
+	/* 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;
+
+	ret = ofnode_read_u32(dev_ofnode(dev->parent),
+			      "keyup-threshold-microvolt", &up_treshold);
+	if (ret)
+		return ret;
+
+	ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
+			      &treshold);
+	if (ret)
+		return ret;
+
+	dev_for_each_subnode(node, dev->parent) {
+		ret = ofnode_read_u32(dev_ofnode(dev),
+				      "press-threshold-microvolt", &t);
+		if (ret)
+			return ret;
+
+		if (t > treshold)
+			up_treshold = t;
+	}
+
+	ret = adc_vdd_value(priv->adc, &vdd);
+	if (ret)
+		return ret;
+
+	ret = adc_data_mask(priv->adc, &mask);
+	if (ret)
+		return ret;
+
+	priv->channel = args.args[0];
+	priv->min = mask * (treshold / 1000) / (vdd / 1000);
+	priv->max = mask * (up_treshold / 1000) / (vdd / 1000);
+
+	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_plat(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	= sizeof(struct button_adc_priv),
+	.bind		= button_adc_bind,
+	.probe		= button_adc_probe,
+};
-- 
2.17.1

[PATCH v5 2/4] 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 | 156 ++++++++++++++++++++++++++++++++++++
 3 files changed, 165 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..1901d59a0e
--- /dev/null
+++ b/drivers/button/button-adc.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 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 <log.h>
+#include <dm.h>
+#include <dm/lists.h>
+#include <dm/of_access.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.
+ * @min: minimal raw ADC sample value to consider button as pressed.
+ * @max: maximal raw ADC sample value to consider button as pressed.
+ */
+struct button_adc_priv {
+	struct udevice *adc;
+	int channel;
+	int min;
+	int max;
+};
+
+static enum button_state_t button_adc_get_state(struct udevice *dev)
+{
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	unsigned int val;
+	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;
+
+	if (ret == 0)
+		return (val >= priv->min && val < priv->max) ?
+			BUTTON_ON : BUTTON_OFF;
+
+	return ret;
+}
+
+static int button_adc_probe(struct udevice *dev)
+{
+	struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	struct ofnode_phandle_args args;
+	u32 treshold, up_treshold, t;
+	unsigned int mask;
+	ofnode node;
+	int ret, vdd;
+
+	/* 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;
+
+	ret = ofnode_read_u32(dev_ofnode(dev->parent),
+			      "keyup-threshold-microvolt", &up_treshold);
+	if (ret)
+		return ret;
+
+	ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
+			      &treshold);
+	if (ret)
+		return ret;
+
+	dev_for_each_subnode(node, dev->parent) {
+		ret = ofnode_read_u32(dev_ofnode(dev),
+				      "press-threshold-microvolt", &t);
+		if (ret)
+			return ret;
+
+		if (t > treshold)
+			up_treshold = t;
+	}
+
+	ret = adc_vdd_value(priv->adc, &vdd);
+	if (ret)
+		return ret;
+
+	ret = adc_data_mask(priv->adc, &mask);
+	if (ret)
+		return ret;
+
+	priv->channel = args.args[0];
+	priv->min = mask * (treshold / 1000) / (vdd / 1000);
+	priv->max = mask * (up_treshold / 1000) / (vdd / 1000);
+
+	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_plat(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	= sizeof(struct button_adc_priv),
+	.bind		= button_adc_bind,
+	.probe		= button_adc_probe,
+};
-- 
2.17.1

[PATCH v5 3/4] adc: meson-saradc: add support for getting reference voltage value

[Marek Szyprowski]

Add support for getting the 'vref-supply' regulator and register it as
ADC's reference voltage regulator, so clients can translate sampled ADC
values to the voltage.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/adc/meson-saradc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
index 21db55831d..1a45a3a265 100644
--- a/drivers/adc/meson-saradc.c
+++ b/drivers/adc/meson-saradc.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/math64.h>
 #include <linux/bitfield.h>
+#include <power/regulator.h>
 
 #define MESON_SAR_ADC_REG0					0x00
 	#define MESON_SAR_ADC_REG0_PANEL_DETECT			BIT(31)
@@ -656,7 +657,10 @@ static int meson_saradc_stop(struct udevice *dev)
 
 static int meson_saradc_probe(struct udevice *dev)
 {
+	struct adc_uclass_plat *uc_pdata = dev_get_uclass_plat(dev);
 	struct meson_saradc_priv *priv = dev_get_priv(dev);
+	struct udevice *vref;
+	int vref_uv;
 	int ret;
 
 	ret = regmap_init_mem(dev_ofnode(dev), &priv->regmap);
@@ -675,6 +679,23 @@ static int meson_saradc_probe(struct udevice *dev)
 
 	priv->active_channel = -1;
 
+	ret = device_get_supply_regulator(dev, "vref-supply", &vref);
+	if (ret) {
+		printf("can't get vref-supply: %d\n", ret);
+		return ret;
+	}
+
+	vref_uv = regulator_get_value(vref);
+	if (vref_uv < 0) {
+		printf("can't get vref-supply value: %d\n", vref_uv);
+		return vref_uv;
+	}
+
+	/* VDD supplied by common vref pin */
+	uc_pdata->vdd_supply = vref;
+	uc_pdata->vdd_microvolts = vref_uv;
+	uc_pdata->vss_microvolts = 0;
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH v5 3/4] adc: meson-saradc: add support for getting reference voltage value

[Marek Szyprowski]

Add support for getting the 'vref-supply' regulator and register it as
ADC's reference voltage regulator, so clients can translate sampled ADC
values to the voltage.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/adc/meson-saradc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
index 21db55831d..1a45a3a265 100644
--- a/drivers/adc/meson-saradc.c
+++ b/drivers/adc/meson-saradc.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/math64.h>
 #include <linux/bitfield.h>
+#include <power/regulator.h>
 
 #define MESON_SAR_ADC_REG0					0x00
 	#define MESON_SAR_ADC_REG0_PANEL_DETECT			BIT(31)
@@ -656,7 +657,10 @@ static int meson_saradc_stop(struct udevice *dev)
 
 static int meson_saradc_probe(struct udevice *dev)
 {
+	struct adc_uclass_plat *uc_pdata = dev_get_uclass_plat(dev);
 	struct meson_saradc_priv *priv = dev_get_priv(dev);
+	struct udevice *vref;
+	int vref_uv;
 	int ret;
 
 	ret = regmap_init_mem(dev_ofnode(dev), &priv->regmap);
@@ -675,6 +679,23 @@ static int meson_saradc_probe(struct udevice *dev)
 
 	priv->active_channel = -1;
 
+	ret = device_get_supply_regulator(dev, "vref-supply", &vref);
+	if (ret) {
+		printf("can't get vref-supply: %d\n", ret);
+		return ret;
+	}
+
+	vref_uv = regulator_get_value(vref);
+	if (vref_uv < 0) {
+		printf("can't get vref-supply value: %d\n", vref_uv);
+		return vref_uv;
+	}
+
+	/* VDD supplied by common vref pin */
+	uc_pdata->vdd_supply = vref;
+	uc_pdata->vdd_microvolts = vref_uv;
+	uc_pdata->vss_microvolts = 0;
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH v5 4/4] 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 v5 4/4] 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 v5 2/4] button: add a simple Analog to Digital Converter device based button driver

[Heinrich Schuchardt]

On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 165 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..1901d59a0e
> --- /dev/null
> +++ b/drivers/button/button-adc.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 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 <log.h>
> +#include <dm.h>
> +#include <dm/lists.h>
> +#include <dm/of_access.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.
> + * @min: minimal raw ADC sample value to consider button as pressed.
> + * @max: maximal raw ADC sample value to consider button as pressed.
> + */
> +struct button_adc_priv {
> +	struct udevice *adc;
> +	int channel;
> +	int min;
> +	int max;
> +};
> +
> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> +{
> +	struct button_adc_priv *priv = dev_get_priv(dev);
> +	unsigned int val;
> +	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;
> +
> +	if (ret == 0)
> +		return (val >= priv->min && val < priv->max) ?
> +			BUTTON_ON : BUTTON_OFF;
> +
> +	return ret;
> +}
> +
> +static int button_adc_probe(struct udevice *dev)
> +{
> +	struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> +	struct button_adc_priv *priv = dev_get_priv(dev);
> +	struct ofnode_phandle_args args;
> +	u32 treshold, up_treshold, t;
> +	unsigned int mask;
> +	ofnode node;
> +	int ret, vdd;
> +
> +	/* 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;
> +
> +	ret = ofnode_read_u32(dev_ofnode(dev->parent),
> +			      "keyup-threshold-microvolt", &up_treshold);
> +	if (ret)
> +		return ret;
> +
> +	ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> +			      &treshold);
> +	if (ret)
> +		return ret;
> +
> +	dev_for_each_subnode(node, dev->parent) {
> +		ret = ofnode_read_u32(dev_ofnode(dev),
> +				      "press-threshold-microvolt", &t);
> +		if (ret)
> +			return ret;
> +
> +		if (t > treshold)
> +			up_treshold = t;

Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
that one virtual key is created per sub-node.

If I read your code correctly, this is not what you are implementing.
Instead you only define a single key per adc-keys node.

Why are your deviating from the bindings document?

Best regards

Heinrich

> +	}
> +
> +	ret = adc_vdd_value(priv->adc, &vdd);
> +	if (ret)
> +		return ret;
> +
> +	ret = adc_data_mask(priv->adc, &mask);
> +	if (ret)
> +		return ret;
> +
> +	priv->channel = args.args[0];
> +	priv->min = mask * (treshold / 1000) / (vdd / 1000);
> +	priv->max = mask * (up_treshold / 1000) / (vdd / 1000);
> +
> +	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_plat(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	= sizeof(struct button_adc_priv),
> +	.bind		= button_adc_bind,
> +	.probe		= button_adc_probe,
> +};
>

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

[Heinrich Schuchardt]

On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 165 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..1901d59a0e
> --- /dev/null
> +++ b/drivers/button/button-adc.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 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 <log.h>
> +#include <dm.h>
> +#include <dm/lists.h>
> +#include <dm/of_access.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.
> + * @min: minimal raw ADC sample value to consider button as pressed.
> + * @max: maximal raw ADC sample value to consider button as pressed.
> + */
> +struct button_adc_priv {
> +	struct udevice *adc;
> +	int channel;
> +	int min;
> +	int max;
> +};
> +
> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> +{
> +	struct button_adc_priv *priv = dev_get_priv(dev);
> +	unsigned int val;
> +	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;
> +
> +	if (ret == 0)
> +		return (val >= priv->min && val < priv->max) ?
> +			BUTTON_ON : BUTTON_OFF;
> +
> +	return ret;
> +}
> +
> +static int button_adc_probe(struct udevice *dev)
> +{
> +	struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> +	struct button_adc_priv *priv = dev_get_priv(dev);
> +	struct ofnode_phandle_args args;
> +	u32 treshold, up_treshold, t;
> +	unsigned int mask;
> +	ofnode node;
> +	int ret, vdd;
> +
> +	/* 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;
> +
> +	ret = ofnode_read_u32(dev_ofnode(dev->parent),
> +			      "keyup-threshold-microvolt", &up_treshold);
> +	if (ret)
> +		return ret;
> +
> +	ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> +			      &treshold);
> +	if (ret)
> +		return ret;
> +
> +	dev_for_each_subnode(node, dev->parent) {
> +		ret = ofnode_read_u32(dev_ofnode(dev),
> +				      "press-threshold-microvolt", &t);
> +		if (ret)
> +			return ret;
> +
> +		if (t > treshold)
> +			up_treshold = t;

Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
that one virtual key is created per sub-node.

If I read your code correctly, this is not what you are implementing.
Instead you only define a single key per adc-keys node.

Why are your deviating from the bindings document?

Best regards

Heinrich

> +	}
> +
> +	ret = adc_vdd_value(priv->adc, &vdd);
> +	if (ret)
> +		return ret;
> +
> +	ret = adc_data_mask(priv->adc, &mask);
> +	if (ret)
> +		return ret;
> +
> +	priv->channel = args.args[0];
> +	priv->min = mask * (treshold / 1000) / (vdd / 1000);
> +	priv->max = mask * (up_treshold / 1000) / (vdd / 1000);
> +
> +	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_plat(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	= sizeof(struct button_adc_priv),
> +	.bind		= button_adc_bind,
> +	.probe		= button_adc_probe,
> +};
>

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

[Marek Szyprowski]

Hi Heinrich,

On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>> ? 3 files changed, 165 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..1901d59a0e
>> --- /dev/null
>> +++ b/drivers/button/button-adc.c
>> @@ -0,0 +1,156 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2021 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 <log.h>
>> +#include <dm.h>
>> +#include <dm/lists.h>
>> +#include <dm/of_access.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.
>> + * @min: minimal raw ADC sample value to consider button as pressed.
>> + * @max: maximal raw ADC sample value to consider button as pressed.
>> + */
>> +struct button_adc_priv {
>> +??? struct udevice *adc;
>> +??? int channel;
>> +??? int min;
>> +??? int max;
>> +};
>> +
>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>> +{
>> +??? struct button_adc_priv *priv = dev_get_priv(dev);
>> +??? unsigned int val;
>> +??? 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;
>> +
>> +??? if (ret == 0)
>> +??????? return (val >= priv->min && val < priv->max) ?
>> +??????????? BUTTON_ON : BUTTON_OFF;
>> +
>> +??? return ret;
>> +}
>> +
>> +static int button_adc_probe(struct udevice *dev)
>> +{
>> +??? struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> +??? struct button_adc_priv *priv = dev_get_priv(dev);
>> +??? struct ofnode_phandle_args args;
>> +??? u32 treshold, up_treshold, t;
>> +??? unsigned int mask;
>> +??? ofnode node;
>> +??? int ret, vdd;
>> +
>> +??? /* 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;
>> +
>> +??? ret = ofnode_read_u32(dev_ofnode(dev->parent),
>> +????????????????? "keyup-threshold-microvolt", &up_treshold);
>> +??? if (ret)
>> +??????? return ret;
>> +
>> +??? ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>> +????????????????? &treshold);
>> +??? if (ret)
>> +??????? return ret;
>> +
>> +??? dev_for_each_subnode(node, dev->parent) {
>> +??????? ret = ofnode_read_u32(dev_ofnode(dev),
>> +????????????????????? "press-threshold-microvolt", &t);
>> +??????? if (ret)
>> +??????????? return ret;
>> +
>> +??????? if (t > treshold)
>> +??????????? up_treshold = t;
>
> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
> that one virtual key is created per sub-node.
>
> If I read your code correctly, this is not what you are implementing.
> Instead you only define a single key per adc-keys node.
>
> Why are your deviating from the bindings document?

No I don't. button_adc_bind() binds to the root node with 'adc-keys' 
compatible, while the dev_for_each_subnode() loop instantiates driver 
for each subnode, so the button_adc_probe() is called for each defined 
key. I've copied this pattern from gpio-keys driver.

 >> ...

Here is the related code:

>> +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_plat(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??? = sizeof(struct button_adc_priv),
>> +??? .bind??????? = button_adc_bind,
>> +??? .probe??????? = button_adc_probe,
>> +};
>>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Marek Szyprowski]

Hi Heinrich,

On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 165 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..1901d59a0e
>> --- /dev/null
>> +++ b/drivers/button/button-adc.c
>> @@ -0,0 +1,156 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2021 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 <log.h>
>> +#include <dm.h>
>> +#include <dm/lists.h>
>> +#include <dm/of_access.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.
>> + * @min: minimal raw ADC sample value to consider button as pressed.
>> + * @max: maximal raw ADC sample value to consider button as pressed.
>> + */
>> +struct button_adc_priv {
>> +    struct udevice *adc;
>> +    int channel;
>> +    int min;
>> +    int max;
>> +};
>> +
>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>> +{
>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>> +    unsigned int val;
>> +    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;
>> +
>> +    if (ret == 0)
>> +        return (val >= priv->min && val < priv->max) ?
>> +            BUTTON_ON : BUTTON_OFF;
>> +
>> +    return ret;
>> +}
>> +
>> +static int button_adc_probe(struct udevice *dev)
>> +{
>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>> +    struct ofnode_phandle_args args;
>> +    u32 treshold, up_treshold, t;
>> +    unsigned int mask;
>> +    ofnode node;
>> +    int ret, vdd;
>> +
>> +    /* 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;
>> +
>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
>> +                  "keyup-threshold-microvolt", &up_treshold);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>> +                  &treshold);
>> +    if (ret)
>> +        return ret;
>> +
>> +    dev_for_each_subnode(node, dev->parent) {
>> +        ret = ofnode_read_u32(dev_ofnode(dev),
>> +                      "press-threshold-microvolt", &t);
>> +        if (ret)
>> +            return ret;
>> +
>> +        if (t > treshold)
>> +            up_treshold = t;
>
> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
> that one virtual key is created per sub-node.
>
> If I read your code correctly, this is not what you are implementing.
> Instead you only define a single key per adc-keys node.
>
> Why are your deviating from the bindings document?

No I don't. button_adc_bind() binds to the root node with 'adc-keys' 
compatible, while the dev_for_each_subnode() loop instantiates driver 
for each subnode, so the button_adc_probe() is called for each defined 
key. I've copied this pattern from gpio-keys driver.

 >> ...

Here is the related code:

>> +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_plat(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    = sizeof(struct button_adc_priv),
>> +    .bind        = button_adc_bind,
>> +    .probe        = button_adc_probe,
>> +};
>>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Heinrich Schuchardt]

On 26.01.21 12:25, Marek Szyprowski wrote:
> Hi Heinrich,
>
> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
>> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>>> ? 3 files changed, 165 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..1901d59a0e
>>> --- /dev/null
>>> +++ b/drivers/button/button-adc.c
>>> @@ -0,0 +1,156 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2021 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 <log.h>
>>> +#include <dm.h>
>>> +#include <dm/lists.h>
>>> +#include <dm/of_access.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.
>>> + * @min: minimal raw ADC sample value to consider button as pressed.
>>> + * @max: maximal raw ADC sample value to consider button as pressed.
>>> + */
>>> +struct button_adc_priv {
>>> +??? struct udevice *adc;
>>> +??? int channel;
>>> +??? int min;
>>> +??? int max;
>>> +};
>>> +
>>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>>> +{
>>> +??? struct button_adc_priv *priv = dev_get_priv(dev);
>>> +??? unsigned int val;
>>> +??? 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;
>>> +
>>> +??? if (ret == 0)
>>> +??????? return (val >= priv->min && val < priv->max) ?
>>> +??????????? BUTTON_ON : BUTTON_OFF;
>>> +
>>> +??? return ret;
>>> +}
>>> +
>>> +static int button_adc_probe(struct udevice *dev)
>>> +{
>>> +??? struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>>> +??? struct button_adc_priv *priv = dev_get_priv(dev);
>>> +??? struct ofnode_phandle_args args;
>>> +??? u32 treshold, up_treshold, t;
>>> +??? unsigned int mask;
>>> +??? ofnode node;
>>> +??? int ret, vdd;
>>> +
>>> +??? /* 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;
>>> +
>>> +??? ret = ofnode_read_u32(dev_ofnode(dev->parent),
>>> +????????????????? "keyup-threshold-microvolt", &up_treshold);
>>> +??? if (ret)
>>> +??????? return ret;
>>> +
>>> +??? ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>>> +????????????????? &treshold);
>>> +??? if (ret)
>>> +??????? return ret;
>>> +
>>> +??? dev_for_each_subnode(node, dev->parent) {
>>> +??????? ret = ofnode_read_u32(dev_ofnode(dev),
>>> +????????????????????? "press-threshold-microvolt", &t);
>>> +??????? if (ret)
>>> +??????????? return ret;
>>> +
>>> +??????? if (t > treshold)
>>> +??????????? up_treshold = t;
>>
>> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
>> that one virtual key is created per sub-node.
>>
>> If I read your code correctly, this is not what you are implementing.
>> Instead you only define a single key per adc-keys node.
>>
>> Why are your deviating from the bindings document?
>
> No I don't. button_adc_bind() binds to the root node with 'adc-keys'
> compatible, while the dev_for_each_subnode() loop instantiates driver
> for each subnode, so the button_adc_probe() is called for each defined
> key. I've copied this pattern from gpio-keys driver.
>
>  >> ...
>
> Here is the related code:

Thanks for pointing this out.

To really test the driver we would need an emulated device on the
sandbox where we can set the voltage and see which button is activated.

I assume this can be added to test/dm/adc.c.

Best regards

Heinrich

>
>>> +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_plat(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??? = sizeof(struct button_adc_priv),
>>> +??? .bind??????? = button_adc_bind,
>>> +??? .probe??????? = button_adc_probe,
>>> +};
>>>
>>
>>
> Best regards
>

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

[Heinrich Schuchardt]

On 26.01.21 12:25, Marek Szyprowski wrote:
> Hi Heinrich,
>
> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
>> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 165 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..1901d59a0e
>>> --- /dev/null
>>> +++ b/drivers/button/button-adc.c
>>> @@ -0,0 +1,156 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2021 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 <log.h>
>>> +#include <dm.h>
>>> +#include <dm/lists.h>
>>> +#include <dm/of_access.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.
>>> + * @min: minimal raw ADC sample value to consider button as pressed.
>>> + * @max: maximal raw ADC sample value to consider button as pressed.
>>> + */
>>> +struct button_adc_priv {
>>> +    struct udevice *adc;
>>> +    int channel;
>>> +    int min;
>>> +    int max;
>>> +};
>>> +
>>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>>> +{
>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>>> +    unsigned int val;
>>> +    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;
>>> +
>>> +    if (ret == 0)
>>> +        return (val >= priv->min && val < priv->max) ?
>>> +            BUTTON_ON : BUTTON_OFF;
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int button_adc_probe(struct udevice *dev)
>>> +{
>>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>>> +    struct ofnode_phandle_args args;
>>> +    u32 treshold, up_treshold, t;
>>> +    unsigned int mask;
>>> +    ofnode node;
>>> +    int ret, vdd;
>>> +
>>> +    /* 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;
>>> +
>>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
>>> +                  "keyup-threshold-microvolt", &up_treshold);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>>> +                  &treshold);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    dev_for_each_subnode(node, dev->parent) {
>>> +        ret = ofnode_read_u32(dev_ofnode(dev),
>>> +                      "press-threshold-microvolt", &t);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        if (t > treshold)
>>> +            up_treshold = t;
>>
>> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
>> that one virtual key is created per sub-node.
>>
>> If I read your code correctly, this is not what you are implementing.
>> Instead you only define a single key per adc-keys node.
>>
>> Why are your deviating from the bindings document?
>
> No I don't. button_adc_bind() binds to the root node with 'adc-keys'
> compatible, while the dev_for_each_subnode() loop instantiates driver
> for each subnode, so the button_adc_probe() is called for each defined
> key. I've copied this pattern from gpio-keys driver.
>
>  >> ...
>
> Here is the related code:

Thanks for pointing this out.

To really test the driver we would need an emulated device on the
sandbox where we can set the voltage and see which button is activated.

I assume this can be added to test/dm/adc.c.

Best regards

Heinrich

>
>>> +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_plat(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    = sizeof(struct button_adc_priv),
>>> +    .bind        = button_adc_bind,
>>> +    .probe        = button_adc_probe,
>>> +};
>>>
>>
>>
> Best regards
>

[PATCH v5 1/4] dt-bindings: input: adc-keys bindings documentation

[Simon Glass]

On Tue, 26 Jan 2021 at 02:51, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Dump adc-keys bindings documentation from Linux kernel source tree from
> commit 698dc0cf9447 ("dt-bindings: input: adc-keys: clarify
> description").
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  doc/device-tree-bindings/input/adc-keys.txt | 67 +++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 doc/device-tree-bindings/input/adc-keys.txt

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v5 3/4] adc: meson-saradc: add support for getting reference voltage value

[Simon Glass]

On Tue, 26 Jan 2021 at 02:51, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Add support for getting the 'vref-supply' regulator and register it as
> ADC's reference voltage regulator, so clients can translate sampled ADC
> values to the voltage.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/adc/meson-saradc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Simon Glass]

On Tue, 26 Jan 2021 at 02:51, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> 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(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Simon Glass]

Hi,

On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 26.01.21 12:25, Marek Szyprowski wrote:
> > Hi Heinrich,
> >
> > On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> >> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
> >>>   3 files changed, 165 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..1901d59a0e
> >>> --- /dev/null
> >>> +++ b/drivers/button/button-adc.c
> >>> @@ -0,0 +1,156 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright (C) 2021 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 <log.h>
> >>> +#include <dm.h>
> >>> +#include <dm/lists.h>
> >>> +#include <dm/of_access.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.
> >>> + * @min: minimal raw ADC sample value to consider button as pressed.
> >>> + * @max: maximal raw ADC sample value to consider button as pressed.
> >>> + */
> >>> +struct button_adc_priv {
> >>> +    struct udevice *adc;
> >>> +    int channel;
> >>> +    int min;
> >>> +    int max;
> >>> +};
> >>> +
> >>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >>> +{
> >>> +    struct button_adc_priv *priv = dev_get_priv(dev);
> >>> +    unsigned int val;
> >>> +    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;
> >>> +
> >>> +    if (ret == 0)
> >>> +        return (val >= priv->min && val < priv->max) ?
> >>> +            BUTTON_ON : BUTTON_OFF;
> >>> +
> >>> +    return ret;
> >>> +}
> >>> +
> >>> +static int button_adc_probe(struct udevice *dev)
> >>> +{
> >>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >>> +    struct button_adc_priv *priv = dev_get_priv(dev);
> >>> +    struct ofnode_phandle_args args;
> >>> +    u32 treshold, up_treshold, t;
> >>> +    unsigned int mask;
> >>> +    ofnode node;
> >>> +    int ret, vdd;
> >>> +
> >>> +    /* 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;
> >>> +
> >>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
> >>> +                  "keyup-threshold-microvolt", &up_treshold);
> >>> +    if (ret)
> >>> +        return ret;
> >>> +
> >>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> >>> +                  &treshold);
> >>> +    if (ret)
> >>> +        return ret;
> >>> +
> >>> +    dev_for_each_subnode(node, dev->parent) {
> >>> +        ret = ofnode_read_u32(dev_ofnode(dev),
> >>> +                      "press-threshold-microvolt", &t);
> >>> +        if (ret)
> >>> +            return ret;
> >>> +
> >>> +        if (t > treshold)
> >>> +            up_treshold = t;
> >>
> >> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
> >> that one virtual key is created per sub-node.
> >>
> >> If I read your code correctly, this is not what you are implementing.
> >> Instead you only define a single key per adc-keys node.
> >>
> >> Why are your deviating from the bindings document?
> >
> > No I don't. button_adc_bind() binds to the root node with 'adc-keys'
> > compatible, while the dev_for_each_subnode() loop instantiates driver
> > for each subnode, so the button_adc_probe() is called for each defined
> > key. I've copied this pattern from gpio-keys driver.
> >
> >  >> ...
> >
> > Here is the related code:
>
> Thanks for pointing this out.
>
> To really test the driver we would need an emulated device on the
> sandbox where we can set the voltage and see which button is activated.
>
> I assume this can be added to test/dm/adc.c.

Yes please!

Regards,
Simon

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

[Marek Szyprowski]

Hi Simon,

On 01.02.2021 21:38, Simon Glass wrote:
> On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 26.01.21 12:25, Marek Szyprowski wrote:
>>> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
>>>> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 165 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..1901d59a0e
>>>>> --- /dev/null
>>>>> +++ b/drivers/button/button-adc.c
>>>>> @@ -0,0 +1,156 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * Copyright (C) 2021 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 <log.h>
>>>>> +#include <dm.h>
>>>>> +#include <dm/lists.h>
>>>>> +#include <dm/of_access.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.
>>>>> + * @min: minimal raw ADC sample value to consider button as pressed.
>>>>> + * @max: maximal raw ADC sample value to consider button as pressed.
>>>>> + */
>>>>> +struct button_adc_priv {
>>>>> +    struct udevice *adc;
>>>>> +    int channel;
>>>>> +    int min;
>>>>> +    int max;
>>>>> +};
>>>>> +
>>>>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>>>>> +{
>>>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>>>>> +    unsigned int val;
>>>>> +    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;
>>>>> +
>>>>> +    if (ret == 0)
>>>>> +        return (val >= priv->min && val < priv->max) ?
>>>>> +            BUTTON_ON : BUTTON_OFF;
>>>>> +
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static int button_adc_probe(struct udevice *dev)
>>>>> +{
>>>>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>>>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
>>>>> +    struct ofnode_phandle_args args;
>>>>> +    u32 treshold, up_treshold, t;
>>>>> +    unsigned int mask;
>>>>> +    ofnode node;
>>>>> +    int ret, vdd;
>>>>> +
>>>>> +    /* 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;
>>>>> +
>>>>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
>>>>> +                  "keyup-threshold-microvolt", &up_treshold);
>>>>> +    if (ret)
>>>>> +        return ret;
>>>>> +
>>>>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
>>>>> +                  &treshold);
>>>>> +    if (ret)
>>>>> +        return ret;
>>>>> +
>>>>> +    dev_for_each_subnode(node, dev->parent) {
>>>>> +        ret = ofnode_read_u32(dev_ofnode(dev),
>>>>> +                      "press-threshold-microvolt", &t);
>>>>> +        if (ret)
>>>>> +            return ret;
>>>>> +
>>>>> +        if (t > treshold)
>>>>> +            up_treshold = t;
>>>> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
>>>> that one virtual key is created per sub-node.
>>>>
>>>> If I read your code correctly, this is not what you are implementing.
>>>> Instead you only define a single key per adc-keys node.
>>>>
>>>> Why are your deviating from the bindings document?
>>> No I don't. button_adc_bind() binds to the root node with 'adc-keys'
>>> compatible, while the dev_for_each_subnode() loop instantiates driver
>>> for each subnode, so the button_adc_probe() is called for each defined
>>> key. I've copied this pattern from gpio-keys driver.
>>>
>>>   >> ...
>>>
>>> Here is the related code:
>> Thanks for pointing this out.
>>
>> To really test the driver we would need an emulated device on the
>> sandbox where we can set the voltage and see which button is activated.
>>
>> I assume this can be added to test/dm/adc.c.
> Yes please!

Could you give me a bit more hints or point where to start? I've tried 
to build sandbox, but it fails for v2021.01 release (I've did make 
sandbox_defconfig && make all). I assume I would need to add adc and 
adc-keys devices to some sandbox dts and some code triggering and 
checking the key values, but that's all I know now.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Simon Glass]

Hi Marek,

On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Simon,
>
> On 01.02.2021 21:38, Simon Glass wrote:
> > On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> On 26.01.21 12:25, Marek Szyprowski wrote:
> >>> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> >>>> On 1/26/21 10:50 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 | 156 ++++++++++++++++++++++++++++++++++++
> >>>>>    3 files changed, 165 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..1901d59a0e
> >>>>> --- /dev/null
> >>>>> +++ b/drivers/button/button-adc.c
> >>>>> @@ -0,0 +1,156 @@
> >>>>> +// SPDX-License-Identifier: GPL-2.0
> >>>>> +/*
> >>>>> + * Copyright (C) 2021 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 <log.h>
> >>>>> +#include <dm.h>
> >>>>> +#include <dm/lists.h>
> >>>>> +#include <dm/of_access.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.
> >>>>> + * @min: minimal raw ADC sample value to consider button as pressed.
> >>>>> + * @max: maximal raw ADC sample value to consider button as pressed.
> >>>>> + */
> >>>>> +struct button_adc_priv {
> >>>>> +    struct udevice *adc;
> >>>>> +    int channel;
> >>>>> +    int min;
> >>>>> +    int max;
> >>>>> +};
> >>>>> +
> >>>>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >>>>> +{
> >>>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
> >>>>> +    unsigned int val;
> >>>>> +    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;
> >>>>> +
> >>>>> +    if (ret == 0)
> >>>>> +        return (val >= priv->min && val < priv->max) ?
> >>>>> +            BUTTON_ON : BUTTON_OFF;
> >>>>> +
> >>>>> +    return ret;
> >>>>> +}
> >>>>> +
> >>>>> +static int button_adc_probe(struct udevice *dev)
> >>>>> +{
> >>>>> +    struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >>>>> +    struct button_adc_priv *priv = dev_get_priv(dev);
> >>>>> +    struct ofnode_phandle_args args;
> >>>>> +    u32 treshold, up_treshold, t;
> >>>>> +    unsigned int mask;
> >>>>> +    ofnode node;
> >>>>> +    int ret, vdd;
> >>>>> +
> >>>>> +    /* 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;
> >>>>> +
> >>>>> +    ret = ofnode_read_u32(dev_ofnode(dev->parent),
> >>>>> +                  "keyup-threshold-microvolt", &up_treshold);
> >>>>> +    if (ret)
> >>>>> +        return ret;
> >>>>> +
> >>>>> +    ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> >>>>> +                  &treshold);
> >>>>> +    if (ret)
> >>>>> +        return ret;
> >>>>> +
> >>>>> +    dev_for_each_subnode(node, dev->parent) {
> >>>>> +        ret = ofnode_read_u32(dev_ofnode(dev),
> >>>>> +                      "press-threshold-microvolt", &t);
> >>>>> +        if (ret)
> >>>>> +            return ret;
> >>>>> +
> >>>>> +        if (t > treshold)
> >>>>> +            up_treshold = t;
> >>>> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
> >>>> that one virtual key is created per sub-node.
> >>>>
> >>>> If I read your code correctly, this is not what you are implementing.
> >>>> Instead you only define a single key per adc-keys node.
> >>>>
> >>>> Why are your deviating from the bindings document?
> >>> No I don't. button_adc_bind() binds to the root node with 'adc-keys'
> >>> compatible, while the dev_for_each_subnode() loop instantiates driver
> >>> for each subnode, so the button_adc_probe() is called for each defined
> >>> key. I've copied this pattern from gpio-keys driver.
> >>>
> >>>   >> ...
> >>>
> >>> Here is the related code:
> >> Thanks for pointing this out.
> >>
> >> To really test the driver we would need an emulated device on the
> >> sandbox where we can set the voltage and see which button is activated.
> >>
> >> I assume this can be added to test/dm/adc.c.
> > Yes please!
>
> Could you give me a bit more hints or point where to start? I've tried
> to build sandbox, but it fails for v2021.01 release (I've did make
> sandbox_defconfig && make all). I assume I would need to add adc and
> adc-keys devices to some sandbox dts and some code triggering and
> checking the key values, but that's all I know now.

Well you do need to be able to build sandbox or you will get
nowhere...what error did you get? Once we understand what went wrong
we can update the docs. Maybe it is missing a dependency.

BTW for testing there is a series with more docs at
u-boot-dm/test-working that might be useful.

You can add your node to sandbox.dtsi and then run U-Boot with -T to
get the test devicetree. Something like:

$ ./u-boot -T
(U-Boot starts)
> ut dm adc_multi_channel_shot
(test runs)

You just need to probe your device and then button_get_state() on it.

BTW I think all of the code in your probe() method should move to
of_to_plat(). It has nothing to do with probing the device and is all
about reading from the devicetree.

Regards,
Simon

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

[Marek Szyprowski]

Hi Simon,

On 06.02.2021 17:21, Simon Glass wrote:
> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> ...
>> Could you give me a bit more hints or point where to start? I've tried
>> to build sandbox, but it fails for v2021.01 release (I've did make
>> sandbox_defconfig && make all). I assume I would need to add adc and
>> adc-keys devices to some sandbox dts and some code triggering and
>> checking the key values, but that's all I know now.
> Well you do need to be able to build sandbox or you will get
> nowhere...what error did you get? Once we understand what went wrong
> we can update the docs. Maybe it is missing a dependency.

$ gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ git checkout v2021.01

$ make sandbox_defconfig
#
# configuration written to .config
#

$ make
scripts/kconfig/conf? --syncconfig Kconfig
 ? CFG???? u-boot.cfg
 ? GEN???? include/autoconf.mk
 ? GEN???? include/autoconf.mk.dep
 ? CFGCHK? u-boot.cfg
 ? UPD???? include/generated/timestamp_autogenerated.h
 ? HOSTCC? tools/mkenvimage.o
 ? HOSTLD? tools/mkenvimage
 ? HOSTCC? tools/fit_image.o
 ? HOSTCC? tools/image-host.o
 ? HOSTCC? tools/dumpimage.o
 ? HOSTLD? tools/dumpimage
 ? HOSTCC? tools/mkimage.o
 ? HOSTLD? tools/mkimage
 ? HOSTLD? tools/fit_info
 ? HOSTLD? tools/fit_check_sign

...

 ? CC????? arch/sandbox/cpu/cpu.o
In file included from include/common.h:26:0,
 ???????????????? from arch/sandbox/cpu/cpu.c:6:
include/asm/global_data.h:112:58: warning: call-clobbered register used 
for global register variable
 ?#define DECLARE_GLOBAL_DATA_PTR? register volatile gd_t *gd asm ("r9")
 ????????????????????????????????????????????????????????? ^
include/dm/of.h:86:1: note: in expansion of macro ?DECLARE_GLOBAL_DATA_PTR?
 ?DECLARE_GLOBAL_DATA_PTR;
 ?^~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/sandbox/cpu/cpu.c:18:0:
./arch/sandbox/include/asm/state.h:98:30: error: 
?CONFIG_SANDBOX_SPI_MAX_BUS? undeclared here (not in a function); did 
you mean ?CONFIG_SANDBOX_SPI??
 ? struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
 ????????????????????????????? ^~~~~~~~~~~~~~~~~~~~~~~~~~
 ????????????????????????????? CONFIG_SANDBOX_SPI
./arch/sandbox/include/asm/state.h:99:7: error: 
?CONFIG_SANDBOX_SPI_MAX_CS? undeclared here (not in a function); did you 
mean ?CONFIG_SANDBOX_SPI_MAX_BUS??
 ????? [CONFIG_SANDBOX_SPI_MAX_CS];
 ?????? ^~~~~~~~~~~~~~~~~~~~~~~~~
 ?????? CONFIG_SANDBOX_SPI_MAX_BUS
arch/sandbox/cpu/cpu.c: In function ?is_in_sandbox_mem?:
arch/sandbox/cpu/cpu.c:83:41: error: ?volatile struct arch_global_data? 
has no member named ?ram_buf?
 ? return (const uint8_t *)ptr >= gd->arch.ram_buf &&
 ???????????????????????????????????????? ^
arch/sandbox/cpu/cpu.c:84:34: error: ?volatile struct arch_global_data? 
has no member named ?ram_buf?
 ?? (const uint8_t *)ptr < gd->arch.ram_buf + gd->ram_size;
 ????????????????????????????????? ^
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:97:7: error: redefinition of ?phys_to_virt?
 ?void *phys_to_virt(phys_addr_t paddr)
 ?????? ^~~~~~~~~~~~
In file included from include/asm/io.h:495:0,
 ???????????????? from arch/sandbox/cpu/cpu.c:15:
include/asm-generic/io.h:30:21: note: previous definition of 
?phys_to_virt? was here
 ?static inline void *phys_to_virt(phys_addr_t paddr)
 ???????????????????? ^~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?phys_to_virt?:
arch/sandbox/cpu/cpu.c:104:27: error: ?volatile struct arch_global_data? 
has no member named ?ram_buf?
 ?? return (void *)(gd->arch.ram_buf + paddr);
 ?????????????????????????? ^
In file included from include/linux/posix_types.h:4:0,
 ???????????????? from include/linux/types.h:4,
 ???????????????? from include/time.h:7,
 ???????????????? from include/common.h:18,
 ???????????????? from arch/sandbox/cpu/cpu.c:6:
include/linux/stddef.h:17:33: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
 ?#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 ???????????????????????????????? ^
include/linux/kernel.h:274:29: note: in expansion of macro ?offsetof?
 ? (type *)( (char *)__mptr - offsetof(type,member) );})
 ???????????????????????????? ^~~~~~~~
include/linux/list.h:327:2: note: in expansion of macro ?container_of?
 ? container_of(ptr, type, member)
 ? ^~~~~~~~~~~~
include/linux/list.h:424:13: note: in expansion of macro ?list_entry?
 ? for (pos = list_entry((head)->next, typeof(*pos), member); \
 ???????????? ^~~~~~~~~~
arch/sandbox/cpu/cpu.c:111:2: note: in expansion of macro 
?list_for_each_entry?
 ? list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
 ? ^~~~~~~~~~~~~~~~~~~
include/linux/stddef.h:17:33: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
 ?#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 ???????????????????????????????? ^
include/linux/kernel.h:274:29: note: in expansion of macro ?offsetof?
 ? (type *)( (char *)__mptr - offsetof(type,member) );})
 ???????????????????????????? ^~~~~~~~
include/linux/list.h:327:2: note: in expansion of macro ?container_of?
 ? container_of(ptr, type, member)
 ? ^~~~~~~~~~~~
include/linux/list.h:426:13: note: in expansion of macro ?list_entry?
 ?????? pos = list_entry(pos->member.next, typeof(*pos), member))
 ???????????? ^~~~~~~~~~
arch/sandbox/cpu/cpu.c:111:2: note: in expansion of macro 
?list_for_each_entry?
 ? list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
 ? ^~~~~~~~~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?find_tag?:
include/linux/stddef.h:17:33: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
 ?#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 ???????????????????????????????? ^
include/linux/kernel.h:274:29: note: in expansion of macro ?offsetof?
 ? (type *)( (char *)__mptr - offsetof(type,member) );})
 ???????????????????????????? ^~~~~~~~
include/linux/list.h:327:2: note: in expansion of macro ?container_of?
 ? container_of(ptr, type, member)
 ? ^~~~~~~~~~~~
include/linux/list.h:424:13: note: in expansion of macro ?list_entry?
 ? for (pos = list_entry((head)->next, typeof(*pos), member); \
 ???????????? ^~~~~~~~~~
arch/sandbox/cpu/cpu.c:132:2: note: in expansion of macro 
?list_for_each_entry?
 ? list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
 ? ^~~~~~~~~~~~~~~~~~~
include/linux/stddef.h:17:33: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
 ?#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 ???????????????????????????????? ^
include/linux/kernel.h:274:29: note: in expansion of macro ?offsetof?
 ? (type *)( (char *)__mptr - offsetof(type,member) );})
 ???????????????????????????? ^~~~~~~~
include/linux/list.h:327:2: note: in expansion of macro ?container_of?
 ? container_of(ptr, type, member)
 ? ^~~~~~~~~~~~
include/linux/list.h:426:13: note: in expansion of macro ?list_entry?
 ?????? pos = list_entry(pos->member.next, typeof(*pos), member))
 ???????????? ^~~~~~~~~~
arch/sandbox/cpu/cpu.c:132:2: note: in expansion of macro 
?list_for_each_entry?
 ? list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
 ? ^~~~~~~~~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:142:13: error: redefinition of ?virt_to_phys?
 ?phys_addr_t virt_to_phys(void *ptr)
 ???????????? ^~~~~~~~~~~~
In file included from include/asm/io.h:495:0,
 ???????????????? from arch/sandbox/cpu/cpu.c:15:
include/asm-generic/io.h:46:27: note: previous definition of 
?virt_to_phys? was here
 ?static inline phys_addr_t virt_to_phys(void *vaddr)
 ?????????????????????????? ^~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?virt_to_phys?:
arch/sandbox/cpu/cpu.c:151:49: error: ?volatile struct arch_global_data? 
has no member named ?ram_buf?
 ?? return (phys_addr_t)((uint8_t *)ptr - gd->arch.ram_buf);
 ???????????????????????????????????????????????? ^
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:165:7: error: redefinition of ?map_physmem?
 ?void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long 
flags)
 ?????? ^~~~~~~~~~~
In file included from include/asm/io.h:495:0,
 ???????????????? from arch/sandbox/cpu/cpu.c:15:
include/asm-generic/io.h:86:21: note: previous definition of 
?map_physmem? was here
 ?static inline void *map_physmem(phys_addr_t paddr, unsigned long len,
 ???????????????????? ^~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?map_physmem?:
arch/sandbox/cpu/cpu.c:172:25: warning: implicit declaration of function 
?pci_map_physmem?; did you mean ?map_physmem?? 
[-Wimplicit-function-declaration]
 ? if (enable_pci_map && !pci_map_physmem(paddr, &len, &map_dev, &ptr)) {
 ???????????????????????? ^~~~~~~~~~~~~~~
 ???????????????????????? map_physmem
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:185:6: error: conflicting types for ?unmap_physmem?
 ?void unmap_physmem(const void *ptr, unsigned long flags)
 ????? ^~~~~~~~~~~~~
In file included from include/asm/io.h:495:0,
 ???????????????? from arch/sandbox/cpu/cpu.c:15:
include/asm-generic/io.h:103:20: note: previous definition of 
?unmap_physmem? was here
 ?static inline void unmap_physmem(void *vaddr, unsigned long flags)
 ??????????????????? ^~~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?unmap_physmem?:
arch/sandbox/cpu/cpu.c:189:3: warning: implicit declaration of function 
?pci_unmap_physmem?; did you mean ?unmap_physmem?? 
[-Wimplicit-function-declaration]
 ?? pci_unmap_physmem(ptr, map_len, map_dev);
 ?? ^~~~~~~~~~~~~~~~~
 ?? unmap_physmem
arch/sandbox/cpu/cpu.c: In function ?map_to_sysmem?:
arch/sandbox/cpu/cpu.c:204:30: error: ?volatile struct arch_global_data? 
has no member named ?ram_buf?
 ?? return (u8 *)ptr - gd->arch.ram_buf;
 ????????????????????????????? ^
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:233:50: warning: ?enum sandboxio_size_t? declared 
inside parameter list will not be visible outside of this definition or 
declaration
 ?unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size)
 ????????????????????????????????????????????????? ^~~~~~~~~~~~~~~~
arch/sandbox/cpu/cpu.c:233:67: error: parameter 2 (?size?) has 
incomplete type
 ?unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size)
^~~~
arch/sandbox/cpu/cpu.c:233:14: warning: function declaration isn?t a 
prototype [-Wstrict-prototypes]
 ?unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size)
 ????????????? ^~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?sandbox_read?:
arch/sandbox/cpu/cpu.c:241:7: error: ?SB_SIZE_8? undeclared (first use 
in this function); did you mean ?PCI_SIZE_8??
 ? case SB_SIZE_8:
 ?????? ^~~~~~~~~
 ?????? PCI_SIZE_8
arch/sandbox/cpu/cpu.c:241:7: note: each undeclared identifier is 
reported only once for each function it appears in
arch/sandbox/cpu/cpu.c:243:7: error: ?SB_SIZE_16? undeclared (first use 
in this function); did you mean ?SB_SIZE_8??
 ? case SB_SIZE_16:
 ?????? ^~~~~~~~~~
 ?????? SB_SIZE_8
arch/sandbox/cpu/cpu.c:245:7: error: ?SB_SIZE_32? undeclared (first use 
in this function); did you mean ?SB_SIZE_16??
 ? case SB_SIZE_32:
 ?????? ^~~~~~~~~~
 ?????? SB_SIZE_16
arch/sandbox/cpu/cpu.c:247:7: error: ?SB_SIZE_64? undeclared (first use 
in this function); did you mean ?SB_SIZE_32??
 ? case SB_SIZE_64:
 ?????? ^~~~~~~~~~
 ?????? SB_SIZE_32
arch/sandbox/cpu/cpu.c: At top level:
arch/sandbox/cpu/cpu.c:254:55: warning: ?enum sandboxio_size_t? declared 
inside parameter list will not be visible outside of this definition or 
declaration
 ?void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t 
size)
^~~~~~~~~~~~~~~~
arch/sandbox/cpu/cpu.c:254:72: error: parameter 3 (?size?) has 
incomplete type
 ?void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t 
size)
^~~~
arch/sandbox/cpu/cpu.c:254:6: warning: function declaration isn?t a 
prototype [-Wstrict-prototypes]
 ?void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t 
size)
 ????? ^~~~~~~~~~~~~
arch/sandbox/cpu/cpu.c: In function ?sandbox_write?:
arch/sandbox/cpu/cpu.c:262:7: error: ?SB_SIZE_8? undeclared (first use 
in this function); did you mean ?PCI_SIZE_8??
 ? case SB_SIZE_8:
 ?????? ^~~~~~~~~
 ?????? PCI_SIZE_8
arch/sandbox/cpu/cpu.c:265:7: error: ?SB_SIZE_16? undeclared (first use 
in this function); did you mean ?SB_SIZE_8??
 ? case SB_SIZE_16:
 ?????? ^~~~~~~~~~
 ?????? SB_SIZE_8
arch/sandbox/cpu/cpu.c:268:7: error: ?SB_SIZE_32? undeclared (first use 
in this function); did you mean ?SB_SIZE_16??
 ? case SB_SIZE_32:
 ?????? ^~~~~~~~~~
 ?????? SB_SIZE_16
arch/sandbox/cpu/cpu.c:271:7: error: ?SB_SIZE_64? undeclared (first use 
in this function); did you mean ?SB_SIZE_32??
 ? case SB_SIZE_64:
 ?????? ^~~~~~~~~~
 ?????? SB_SIZE_32
arch/sandbox/cpu/cpu.c: In function ?sandbox_read_fdt_from_file?:
arch/sandbox/cpu/cpu.c:306:9: warning: implicit declaration of function 
?map_sysmem?; did you mean ?map_physmem?? [-Wimplicit-function-declaration]
 ? blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0);
 ???????? ^~~~~~~~~~
 ???????? map_physmem
arch/sandbox/cpu/cpu.c:306:7: warning: assignment makes pointer from 
integer without a cast [-Wint-conversion]
 ? blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0);
 ?????? ^
arch/sandbox/cpu/cpu.c: In function ?is_in_sandbox_mem?:
arch/sandbox/cpu/cpu.c:85:1: warning: control reaches end of non-void 
function [-Wreturn-type]
 ?}
 ?^
scripts/Makefile.build:265: recipe for target 'arch/sandbox/cpu/cpu.o' 
failed
make[1]: *** [arch/sandbox/cpu/cpu.o] Error 1
Makefile:1784: recipe for target 'arch/sandbox/cpu' failed
make: *** [arch/sandbox/cpu] Error 2

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Simon Glass]

HI Marek,

On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Simon,
>
> On 06.02.2021 17:21, Simon Glass wrote:
> > On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> ...
> >> Could you give me a bit more hints or point where to start? I've tried
> >> to build sandbox, but it fails for v2021.01 release (I've did make
> >> sandbox_defconfig && make all). I assume I would need to add adc and
> >> adc-keys devices to some sandbox dts and some code triggering and
> >> checking the key values, but that's all I know now.
> > Well you do need to be able to build sandbox or you will get
> > nowhere...what error did you get? Once we understand what went wrong
> > we can update the docs. Maybe it is missing a dependency.
>
> $ gcc --version
> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> Copyright (C) 2017 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> $ git checkout v2021.01
>
> $ make sandbox_defconfig
> #
> # configuration written to .config
> #
>
> $ make
> scripts/kconfig/conf  --syncconfig Kconfig
>    CFG     u-boot.cfg
>    GEN     include/autoconf.mk
>    GEN     include/autoconf.mk.dep
>    CFGCHK  u-boot.cfg
>    UPD     include/generated/timestamp_autogenerated.h
>    HOSTCC  tools/mkenvimage.o
>    HOSTLD  tools/mkenvimage
>    HOSTCC  tools/fit_image.o
>    HOSTCC  tools/image-host.o
>    HOSTCC  tools/dumpimage.o
>    HOSTLD  tools/dumpimage
>    HOSTCC  tools/mkimage.o
>    HOSTLD  tools/mkimage
>    HOSTLD  tools/fit_info
>    HOSTLD  tools/fit_check_sign
>
> ...
>
>    CC      arch/sandbox/cpu/cpu.o
> In file included from include/common.h:26:0,
>                   from arch/sandbox/cpu/cpu.c:6:
> include/asm/global_data.h:112:58: warning: call-clobbered register used
> for global register variable
>   #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
>                                                            ^
> include/dm/of.h:86:1: note: in expansion of macro ?DECLARE_GLOBAL_DATA_PTR?
>   DECLARE_GLOBAL_DATA_PTR;

This is pretty mysterious. Are you sure you are using an x86_64 machine?

Regards,
Simon

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

[Heinrich Schuchardt]

On 2/8/21 6:08 PM, Simon Glass wrote:
> HI Marek,
>
> On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>
>> Hi Simon,
>>
>> On 06.02.2021 17:21, Simon Glass wrote:
>>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>>> ...
>>>> Could you give me a bit more hints or point where to start? I've tried
>>>> to build sandbox, but it fails for v2021.01 release (I've did make
>>>> sandbox_defconfig && make all). I assume I would need to add adc and
>>>> adc-keys devices to some sandbox dts and some code triggering and
>>>> checking the key values, but that's all I know now.
>>> Well you do need to be able to build sandbox or you will get
>>> nowhere...what error did you get? Once we understand what went wrong
>>> we can update the docs. Maybe it is missing a dependency.
>>
>> $ gcc --version
>> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
>> Copyright (C) 2017 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions. There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
>> $ git checkout v2021.01
>>
>> $ make sandbox_defconfig
>> #
>> # configuration written to .config
>> #
>>
>> $ make
>> scripts/kconfig/conf  --syncconfig Kconfig
>>     CFG     u-boot.cfg
>>     GEN     include/autoconf.mk
>>     GEN     include/autoconf.mk.dep
>>     CFGCHK  u-boot.cfg
>>     UPD     include/generated/timestamp_autogenerated.h
>>     HOSTCC  tools/mkenvimage.o
>>     HOSTLD  tools/mkenvimage
>>     HOSTCC  tools/fit_image.o
>>     HOSTCC  tools/image-host.o
>>     HOSTCC  tools/dumpimage.o
>>     HOSTLD  tools/dumpimage
>>     HOSTCC  tools/mkimage.o
>>     HOSTLD  tools/mkimage
>>     HOSTLD  tools/fit_info
>>     HOSTLD  tools/fit_check_sign
>>
>> ...
>>
>>     CC      arch/sandbox/cpu/cpu.o
>> In file included from include/common.h:26:0,
>>                    from arch/sandbox/cpu/cpu.c:6:
>> include/asm/global_data.h:112:58: warning: call-clobbered register used
>> for global register variable
>>    #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
>>                                                             ^
>> include/dm/of.h:86:1: note: in expansion of macro ?DECLARE_GLOBAL_DATA_PTR?
>>    DECLARE_GLOBAL_DATA_PTR;
>
> This is pretty mysterious. Are you sure you are using an x86_64 machine?

r9 relates to ARMv7.

You have to unset CROSS_COMPILE when you build the sandbox.

The sandbox runs fine on aarch64.

There are a bunch of errors currently when building on 32bit
architectures. Simon has a lot of patches pending.

Best regards

Heinrich

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

[Heinrich Schuchardt]

On 2/8/21 6:56 PM, Heinrich Schuchardt wrote:
> On 2/8/21 6:08 PM, Simon Glass wrote:
>> HI Marek,
>>
>> On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On 06.02.2021 17:21, Simon Glass wrote:
>>>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski
>>>> <m.szyprowski@samsung.com> wrote:
>>>>> ...
>>>>> Could you give me a bit more hints or point where to start? I've tried
>>>>> to build sandbox, but it fails for v2021.01 release (I've did make
>>>>> sandbox_defconfig && make all). I assume I would need to add adc and
>>>>> adc-keys devices to some sandbox dts and some code triggering and
>>>>> checking the key values, but that's all I know now.
>>>> Well you do need to be able to build sandbox or you will get
>>>> nowhere...what error did you get? Once we understand what went wrong
>>>> we can update the docs. Maybe it is missing a dependency.
>>>
>>> $ gcc --version
>>> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
>>> Copyright (C) 2017 Free Software Foundation, Inc.
>>> This is free software; see the source for copying conditions. There
>>> is NO
>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>>> PURPOSE.
>>>
>>> $ git checkout v2021.01
>>>
>>> $ make sandbox_defconfig
>>> #
>>> # configuration written to .config
>>> #
>>>
>>> $ make
>>> scripts/kconfig/conf? --syncconfig Kconfig
>>> ??? CFG???? u-boot.cfg
>>> ??? GEN???? include/autoconf.mk
>>> ??? GEN???? include/autoconf.mk.dep
>>> ??? CFGCHK? u-boot.cfg
>>> ??? UPD???? include/generated/timestamp_autogenerated.h
>>> ??? HOSTCC? tools/mkenvimage.o
>>> ??? HOSTLD? tools/mkenvimage
>>> ??? HOSTCC? tools/fit_image.o
>>> ??? HOSTCC? tools/image-host.o
>>> ??? HOSTCC? tools/dumpimage.o
>>> ??? HOSTLD? tools/dumpimage
>>> ??? HOSTCC? tools/mkimage.o
>>> ??? HOSTLD? tools/mkimage
>>> ??? HOSTLD? tools/fit_info
>>> ??? HOSTLD? tools/fit_check_sign
>>>
>>> ...
>>>
>>> ??? CC????? arch/sandbox/cpu/cpu.o
>>> In file included from include/common.h:26:0,
>>> ?????????????????? from arch/sandbox/cpu/cpu.c:6:
>>> include/asm/global_data.h:112:58: warning: call-clobbered register used
>>> for global register variable
>>> ?? #define DECLARE_GLOBAL_DATA_PTR? register volatile gd_t *gd asm
>>> ("r9")
>>> ??????????????????????????????????????????????????????????? ^
>>> include/dm/of.h:86:1: note: in expansion of macro
>>> ?DECLARE_GLOBAL_DATA_PTR?
>>> ?? DECLARE_GLOBAL_DATA_PTR;
>>
>> This is pretty mysterious. Are you sure you are using an x86_64 machine?
>
> r9 relates to ARMv7.
>
> You have to unset CROSS_COMPILE when you build the sandbox.
>
> The sandbox runs fine on aarch64.
>
> There are a bunch of errors currently when building on 32bit
> architectures. Simon has a lot of patches pending.

Hello Simon,

it was your patch

091401131085 ("command: Remove the cmd_tbl_t typedef") 2020-05-10

that broke building the sandbox on ARMv7.

Once your 32bit corrections are merged we should try to add
cross-compiling the sandbox for aarch64 and armv7 to Gitlab CI.

Best regards

Heinrich

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

[Simon Glass]

Hi Heinrich,

On Mon, 8 Feb 2021 at 15:18, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 2/8/21 6:56 PM, Heinrich Schuchardt wrote:
> > On 2/8/21 6:08 PM, Simon Glass wrote:
> >> HI Marek,
> >>
> >> On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski
> >> <m.szyprowski@samsung.com> wrote:
> >>>
> >>> Hi Simon,
> >>>
> >>> On 06.02.2021 17:21, Simon Glass wrote:
> >>>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski
> >>>> <m.szyprowski@samsung.com> wrote:
> >>>>> ...
> >>>>> Could you give me a bit more hints or point where to start? I've tried
> >>>>> to build sandbox, but it fails for v2021.01 release (I've did make
> >>>>> sandbox_defconfig && make all). I assume I would need to add adc and
> >>>>> adc-keys devices to some sandbox dts and some code triggering and
> >>>>> checking the key values, but that's all I know now.
> >>>> Well you do need to be able to build sandbox or you will get
> >>>> nowhere...what error did you get? Once we understand what went wrong
> >>>> we can update the docs. Maybe it is missing a dependency.
> >>>
> >>> $ gcc --version
> >>> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> >>> Copyright (C) 2017 Free Software Foundation, Inc.
> >>> This is free software; see the source for copying conditions. There
> >>> is NO
> >>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> >>> PURPOSE.
> >>>
> >>> $ git checkout v2021.01
> >>>
> >>> $ make sandbox_defconfig
> >>> #
> >>> # configuration written to .config
> >>> #
> >>>
> >>> $ make
> >>> scripts/kconfig/conf  --syncconfig Kconfig
> >>>     CFG     u-boot.cfg
> >>>     GEN     include/autoconf.mk
> >>>     GEN     include/autoconf.mk.dep
> >>>     CFGCHK  u-boot.cfg
> >>>     UPD     include/generated/timestamp_autogenerated.h
> >>>     HOSTCC  tools/mkenvimage.o
> >>>     HOSTLD  tools/mkenvimage
> >>>     HOSTCC  tools/fit_image.o
> >>>     HOSTCC  tools/image-host.o
> >>>     HOSTCC  tools/dumpimage.o
> >>>     HOSTLD  tools/dumpimage
> >>>     HOSTCC  tools/mkimage.o
> >>>     HOSTLD  tools/mkimage
> >>>     HOSTLD  tools/fit_info
> >>>     HOSTLD  tools/fit_check_sign
> >>>
> >>> ...
> >>>
> >>>     CC      arch/sandbox/cpu/cpu.o
> >>> In file included from include/common.h:26:0,
> >>>                    from arch/sandbox/cpu/cpu.c:6:
> >>> include/asm/global_data.h:112:58: warning: call-clobbered register used
> >>> for global register variable
> >>>    #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm
> >>> ("r9")
> >>>                                                             ^
> >>> include/dm/of.h:86:1: note: in expansion of macro
> >>> ?DECLARE_GLOBAL_DATA_PTR?
> >>>    DECLARE_GLOBAL_DATA_PTR;
> >>
> >> This is pretty mysterious. Are you sure you are using an x86_64 machine?
> >
> > r9 relates to ARMv7.
> >
> > You have to unset CROSS_COMPILE when you build the sandbox.
> >
> > The sandbox runs fine on aarch64.
> >
> > There are a bunch of errors currently when building on 32bit
> > architectures. Simon has a lot of patches pending.
>
> Hello Simon,
>
> it was your patch
>
> 091401131085 ("command: Remove the cmd_tbl_t typedef") 2020-05-10
>
> that broke building the sandbox on ARMv7.

Oh dear.

>
> Once your 32bit corrections are merged we should try to add
> cross-compiling the sandbox for aarch64 and armv7 to Gitlab CI.

Yes, what we don't test doesn't work :-)

Regards,
Simon

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

[Marek Szyprowski]

Hi Simon,

On 08.02.2021 18:08, Simon Glass wrote:
> On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 06.02.2021 17:21, Simon Glass wrote:
>>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>>> ...
>>>> Could you give me a bit more hints or point where to start? I've tried
>>>> to build sandbox, but it fails for v2021.01 release (I've did make
>>>> sandbox_defconfig && make all). I assume I would need to add adc and
>>>> adc-keys devices to some sandbox dts and some code triggering and
>>>> checking the key values, but that's all I know now.
>>> Well you do need to be able to build sandbox or you will get
>>> nowhere...what error did you get? Once we understand what went wrong
>>> we can update the docs. Maybe it is missing a dependency.
>> $ gcc --version
>> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
>> Copyright (C) 2017 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions. There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
>> $ git checkout v2021.01
>>
>> $ make sandbox_defconfig
>> #
>> # configuration written to .config
>> #
>>
>> $ make
>> scripts/kconfig/conf  --syncconfig Kconfig
>>     CFG     u-boot.cfg
>>     GEN     include/autoconf.mk
>>     GEN     include/autoconf.mk.dep
>>     CFGCHK  u-boot.cfg
>>     UPD     include/generated/timestamp_autogenerated.h
>>     HOSTCC  tools/mkenvimage.o
>>     HOSTLD  tools/mkenvimage
>>     HOSTCC  tools/fit_image.o
>>     HOSTCC  tools/image-host.o
>>     HOSTCC  tools/dumpimage.o
>>     HOSTLD  tools/dumpimage
>>     HOSTCC  tools/mkimage.o
>>     HOSTLD  tools/mkimage
>>     HOSTLD  tools/fit_info
>>     HOSTLD  tools/fit_check_sign
>>
>> ...
>>
>>     CC      arch/sandbox/cpu/cpu.o
>> In file included from include/common.h:26:0,
>>                    from arch/sandbox/cpu/cpu.c:6:
>> include/asm/global_data.h:112:58: warning: call-clobbered register used
>> for global register variable
>>    #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
>>                                                             ^
>> include/dm/of.h:86:1: note: in expansion of macro ?DECLARE_GLOBAL_DATA_PTR?
>>    DECLARE_GLOBAL_DATA_PTR;
> This is pretty mysterious. Are you sure you are using an x86_64 machine?

I've finally found what caused the issue on my build system. It is 
x86_64 machine, but after some old cross-builds I had an 'asm' symlink 
in u-boot/include directory pointing to arch/arm directory. I'm quite 
surprised that it has not been removed by make clean/distclean/mrproper 
combo.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Simon Glass]

Hi Marek,

On Tue, 9 Feb 2021 at 01:43, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Simon,
>
> On 08.02.2021 18:08, Simon Glass wrote:
> > On Mon, 8 Feb 2021 at 09:10, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> On 06.02.2021 17:21, Simon Glass wrote:
> >>> On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >>>> ...
> >>>> Could you give me a bit more hints or point where to start? I've tried
> >>>> to build sandbox, but it fails for v2021.01 release (I've did make
> >>>> sandbox_defconfig && make all). I assume I would need to add adc and
> >>>> adc-keys devices to some sandbox dts and some code triggering and
> >>>> checking the key values, but that's all I know now.
> >>> Well you do need to be able to build sandbox or you will get
> >>> nowhere...what error did you get? Once we understand what went wrong
> >>> we can update the docs. Maybe it is missing a dependency.
> >> $ gcc --version
> >> gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> >> Copyright (C) 2017 Free Software Foundation, Inc.
> >> This is free software; see the source for copying conditions. There is NO
> >> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >>
> >> $ git checkout v2021.01
> >>
> >> $ make sandbox_defconfig
> >> #
> >> # configuration written to .config
> >> #
> >>
> >> $ make
> >> scripts/kconfig/conf  --syncconfig Kconfig
> >>     CFG     u-boot.cfg
> >>     GEN     include/autoconf.mk
> >>     GEN     include/autoconf.mk.dep
> >>     CFGCHK  u-boot.cfg
> >>     UPD     include/generated/timestamp_autogenerated.h
> >>     HOSTCC  tools/mkenvimage.o
> >>     HOSTLD  tools/mkenvimage
> >>     HOSTCC  tools/fit_image.o
> >>     HOSTCC  tools/image-host.o
> >>     HOSTCC  tools/dumpimage.o
> >>     HOSTLD  tools/dumpimage
> >>     HOSTCC  tools/mkimage.o
> >>     HOSTLD  tools/mkimage
> >>     HOSTLD  tools/fit_info
> >>     HOSTLD  tools/fit_check_sign
> >>
> >> ...
> >>
> >>     CC      arch/sandbox/cpu/cpu.o
> >> In file included from include/common.h:26:0,
> >>                    from arch/sandbox/cpu/cpu.c:6:
> >> include/asm/global_data.h:112:58: warning: call-clobbered register used
> >> for global register variable
> >>    #define DECLARE_GLOBAL_DATA_PTR  register volatile gd_t *gd asm ("r9")
> >>                                                             ^
> >> include/dm/of.h:86:1: note: in expansion of macro ?DECLARE_GLOBAL_DATA_PTR?
> >>    DECLARE_GLOBAL_DATA_PTR;
> > This is pretty mysterious. Are you sure you are using an x86_64 machine?
>
> I've finally found what caused the issue on my build system. It is
> x86_64 machine, but after some old cross-builds I had an 'asm' symlink
> in u-boot/include directory pointing to arch/arm directory. I'm quite
> surprised that it has not been removed by make clean/distclean/mrproper
> combo.

OK. I wonder if this is after building a U-Boot from 2013? I will send a patch.

Regards,
Simon