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

[PATCH 0/6] 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
button Function
if test ${button} = on
then
	echo Selected alternative boot
	...
fi
--->8---

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Marek Szyprowski (6):
  clk: meson: add minimal driver for g12a-ao clocks
  adc: meson-saradc: add G12A variant
  adc: meson-saradc: skip hardware init only if ADC is enabled
  button: add a simple ADC-based button driver
  cmd: button: store button state in the 'button' env
  configs: khadas-vim3: enable Function button support

 cmd/button.c                  |   4 +-
 configs/khadas-vim3_defconfig |   4 ++
 drivers/adc/meson-saradc.c    |   9 ++-
 drivers/button/Kconfig        |   8 +++
 drivers/button/Makefile       |   1 +
 drivers/button/button-adc.c   | 117 ++++++++++++++++++++++++++++++++++
 drivers/clk/meson/Makefile    |   1 +
 drivers/clk/meson/g12a-ao.c   |  83 ++++++++++++++++++++++++
 8 files changed, 224 insertions(+), 3 deletions(-)
 create mode 100644 drivers/button/button-adc.c
 create mode 100644 drivers/clk/meson/g12a-ao.c

-- 
2.17.1

[PATCH 0/6] 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
button Function
if test ${button} = on
then
	echo Selected alternative boot
	...
fi
--->8---

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Marek Szyprowski (6):
  clk: meson: add minimal driver for g12a-ao clocks
  adc: meson-saradc: add G12A variant
  adc: meson-saradc: skip hardware init only if ADC is enabled
  button: add a simple ADC-based button driver
  cmd: button: store button state in the 'button' env
  configs: khadas-vim3: enable Function button support

 cmd/button.c                  |   4 +-
 configs/khadas-vim3_defconfig |   4 ++
 drivers/adc/meson-saradc.c    |   9 ++-
 drivers/button/Kconfig        |   8 +++
 drivers/button/Makefile       |   1 +
 drivers/button/button-adc.c   | 117 ++++++++++++++++++++++++++++++++++
 drivers/clk/meson/Makefile    |   1 +
 drivers/clk/meson/g12a-ao.c   |  83 ++++++++++++++++++++++++
 8 files changed, 224 insertions(+), 3 deletions(-)
 create mode 100644 drivers/button/button-adc.c
 create mode 100644 drivers/clk/meson/g12a-ao.c

-- 
2.17.1

[PATCH 1/6] clk: meson: add minimal driver for g12a-ao clocks

[Marek Szyprowski]

Add minimal driver AO clocks on meson G12A family. Only ADC related clocks
are supported.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I0c91848bc55493e19570db333e7da6020d687907
---
 drivers/clk/meson/Makefile  |  1 +
 drivers/clk/meson/g12a-ao.c | 83 +++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)
 create mode 100644 drivers/clk/meson/g12a-ao.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index c873d6976fa..7204383e173 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -6,4 +6,5 @@
 obj-$(CONFIG_CLK_MESON_GX) += gxbb.o
 obj-$(CONFIG_CLK_MESON_AXG) += axg.o
 obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
+obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
 
diff --git a/drivers/clk/meson/g12a-ao.c b/drivers/clk/meson/g12a-ao.c
new file mode 100644
index 00000000000..7a0abea77c4
--- /dev/null
+++ b/drivers/clk/meson/g12a-ao.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <common.h>
+#include <log.h>
+#include <asm/io.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <dt-bindings/clock/g12a-aoclkc.h>
+
+#include "clk_meson.h"
+
+struct meson_clk {
+	struct regmap *map;
+};
+
+#define AO_CLK_GATE0		0x4c
+#define AO_SAR_CLK		0x90
+
+static struct meson_gate gates[] = {
+	MESON_GATE(CLKID_AO_SAR_ADC, AO_CLK_GATE0, 8),
+	MESON_GATE(CLKID_AO_SAR_ADC_CLK, AO_SAR_CLK, 8),
+};
+
+static int meson_set_gate(struct clk *clk, bool on)
+{
+	struct meson_clk *priv = dev_get_priv(clk->dev);
+	struct meson_gate *gate;
+
+	if (clk->id >= ARRAY_SIZE(gates))
+		return -ENOENT;
+
+	gate = &gates[clk->id];
+
+	if (gate->reg == 0)
+		return 0;
+
+	regmap_update_bits(priv->map, gate->reg,
+			   BIT(gate->bit), on ? BIT(gate->bit) : 0);
+
+	return 0;
+}
+
+static int meson_clk_enable(struct clk *clk)
+{
+	return meson_set_gate(clk, true);
+}
+
+static int meson_clk_disable(struct clk *clk)
+{
+	return meson_set_gate(clk, false);
+}
+
+static int meson_clk_probe(struct udevice *dev)
+{
+	struct meson_clk *priv = dev_get_priv(dev);
+
+	priv->map = syscon_node_to_regmap(dev_get_parent(dev)->node);
+	if (IS_ERR(priv->map))
+		return PTR_ERR(priv->map);
+
+	return 0;
+}
+
+static struct clk_ops meson_clk_ops = {
+	.disable	= meson_clk_disable,
+	.enable		= meson_clk_enable,
+};
+
+static const struct udevice_id meson_clk_ids[] = {
+	{ .compatible = "amlogic,meson-g12a-aoclkc" },
+	{ }
+};
+
+U_BOOT_DRIVER(meson_clk_axg) = {
+	.name		= "meson_clk_g12a_ao",
+	.id		= UCLASS_CLK,
+	.of_match	= meson_clk_ids,
+	.priv_auto_alloc_size = sizeof(struct meson_clk),
+	.ops		= &meson_clk_ops,
+	.probe		= meson_clk_probe,
+};
-- 
2.17.1

[PATCH 1/6] clk: meson: add minimal driver for g12a-ao clocks

[Marek Szyprowski]

Add minimal driver AO clocks on meson G12A family. Only ADC related clocks
are supported.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I0c91848bc55493e19570db333e7da6020d687907
---
 drivers/clk/meson/Makefile  |  1 +
 drivers/clk/meson/g12a-ao.c | 83 +++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)
 create mode 100644 drivers/clk/meson/g12a-ao.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index c873d6976fa..7204383e173 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -6,4 +6,5 @@
 obj-$(CONFIG_CLK_MESON_GX) += gxbb.o
 obj-$(CONFIG_CLK_MESON_AXG) += axg.o
 obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
+obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
 
diff --git a/drivers/clk/meson/g12a-ao.c b/drivers/clk/meson/g12a-ao.c
new file mode 100644
index 00000000000..7a0abea77c4
--- /dev/null
+++ b/drivers/clk/meson/g12a-ao.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <common.h>
+#include <log.h>
+#include <asm/io.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <dt-bindings/clock/g12a-aoclkc.h>
+
+#include "clk_meson.h"
+
+struct meson_clk {
+	struct regmap *map;
+};
+
+#define AO_CLK_GATE0		0x4c
+#define AO_SAR_CLK		0x90
+
+static struct meson_gate gates[] = {
+	MESON_GATE(CLKID_AO_SAR_ADC, AO_CLK_GATE0, 8),
+	MESON_GATE(CLKID_AO_SAR_ADC_CLK, AO_SAR_CLK, 8),
+};
+
+static int meson_set_gate(struct clk *clk, bool on)
+{
+	struct meson_clk *priv = dev_get_priv(clk->dev);
+	struct meson_gate *gate;
+
+	if (clk->id >= ARRAY_SIZE(gates))
+		return -ENOENT;
+
+	gate = &gates[clk->id];
+
+	if (gate->reg == 0)
+		return 0;
+
+	regmap_update_bits(priv->map, gate->reg,
+			   BIT(gate->bit), on ? BIT(gate->bit) : 0);
+
+	return 0;
+}
+
+static int meson_clk_enable(struct clk *clk)
+{
+	return meson_set_gate(clk, true);
+}
+
+static int meson_clk_disable(struct clk *clk)
+{
+	return meson_set_gate(clk, false);
+}
+
+static int meson_clk_probe(struct udevice *dev)
+{
+	struct meson_clk *priv = dev_get_priv(dev);
+
+	priv->map = syscon_node_to_regmap(dev_get_parent(dev)->node);
+	if (IS_ERR(priv->map))
+		return PTR_ERR(priv->map);
+
+	return 0;
+}
+
+static struct clk_ops meson_clk_ops = {
+	.disable	= meson_clk_disable,
+	.enable		= meson_clk_enable,
+};
+
+static const struct udevice_id meson_clk_ids[] = {
+	{ .compatible = "amlogic,meson-g12a-aoclkc" },
+	{ }
+};
+
+U_BOOT_DRIVER(meson_clk_axg) = {
+	.name		= "meson_clk_g12a_ao",
+	.id		= UCLASS_CLK,
+	.of_match	= meson_clk_ids,
+	.priv_auto_alloc_size = sizeof(struct meson_clk),
+	.ops		= &meson_clk_ops,
+	.probe		= meson_clk_probe,
+};
-- 
2.17.1

[PATCH 2/6] adc: meson-saradc: add G12A variant

[Marek Szyprowski]

Add support for the SARADC variant found on the G12A SoCs family.

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

diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
index 72b0cc4e5bd..998cef24d88 100644
--- a/drivers/adc/meson-saradc.c
+++ b/drivers/adc/meson-saradc.c
@@ -711,6 +711,8 @@ static const struct udevice_id meson_saradc_ids[] = {
 	  .data = (ulong)&gxl_saradc_data },
 	{ .compatible = "amlogic,meson-gxm-saradc",
 	  .data = (ulong)&gxl_saradc_data },
+	{ .compatible = "amlogic,meson-g12a-saradc",
+	  .data = (ulong)&gxl_saradc_data },
 	{ }
 };
 
-- 
2.17.1

[PATCH 2/6] adc: meson-saradc: add G12A variant

[Marek Szyprowski]

Add support for the SARADC variant found on the G12A SoCs family.

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

diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
index 72b0cc4e5bd..998cef24d88 100644
--- a/drivers/adc/meson-saradc.c
+++ b/drivers/adc/meson-saradc.c
@@ -711,6 +711,8 @@ static const struct udevice_id meson_saradc_ids[] = {
 	  .data = (ulong)&gxl_saradc_data },
 	{ .compatible = "amlogic,meson-gxm-saradc",
 	  .data = (ulong)&gxl_saradc_data },
+	{ .compatible = "amlogic,meson-g12a-saradc",
+	  .data = (ulong)&gxl_saradc_data },
 	{ }
 };
 
-- 
2.17.1

[PATCH 3/6] adc: meson-saradc: skip hardware init only if ADC is enabled

[Marek Szyprowski]

The driver skips hardware initialization if it is already configured by
the earlier bootloader stage (BL30). Skip the initialization only if the
hardware is really initialized and enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I3e2e2d270ad3e7e7f38e2bc3ce2a924a47b164af
---
 drivers/adc/meson-saradc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
index 998cef24d88..ce7ae990ad0 100644
--- a/drivers/adc/meson-saradc.c
+++ b/drivers/adc/meson-saradc.c
@@ -512,8 +512,11 @@ static int meson_saradc_init(struct meson_saradc_priv *priv)
 	 * reading the temperature sensor.
 	 */
 	regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
-	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
-		return 0;
+	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED) {
+		regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
+		if (regval & MESON_SAR_ADC_REG3_ADC_EN)
+			return 0;
+	}
 
 	meson_saradc_stop_sample_engine(priv);
 
-- 
2.17.1

[PATCH 3/6] adc: meson-saradc: skip hardware init only if ADC is enabled

[Marek Szyprowski]

The driver skips hardware initialization if it is already configured by
the earlier bootloader stage (BL30). Skip the initialization only if the
hardware is really initialized and enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I3e2e2d270ad3e7e7f38e2bc3ce2a924a47b164af
---
 drivers/adc/meson-saradc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
index 998cef24d88..ce7ae990ad0 100644
--- a/drivers/adc/meson-saradc.c
+++ b/drivers/adc/meson-saradc.c
@@ -512,8 +512,11 @@ static int meson_saradc_init(struct meson_saradc_priv *priv)
 	 * reading the temperature sensor.
 	 */
 	regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
-	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
-		return 0;
+	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED) {
+		regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
+		if (regval & MESON_SAR_ADC_REG3_ADC_EN)
+			return 0;
+	}
 
 	meson_saradc_stop_sample_engine(priv);
 
-- 
2.17.1

[PATCH 4/6] button: add a simple ADC-based button driver

[Marek Szyprowski]

Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
device tree node.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
---
 drivers/button/Kconfig      |   8 +++
 drivers/button/Makefile     |   1 +
 drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)
 create mode 100644 drivers/button/button-adc.c

diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
--- /dev/null
+++ b/drivers/button/button-adc.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ */
+
+#include <common.h>
+#include <button.h>
+#include <dm.h>
+#include <dm/lists.h>
+#include <dm/uclass-internal.h>
+#include <log.h>
+#include <adc.h>
+
+struct button_adc_priv {
+	struct udevice *adc;
+	int channel;
+};
+
+static enum button_state_t button_adc_get_state(struct udevice *dev)
+{
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	unsigned int val, mask;
+	int ret;
+
+	ret = adc_start_channel(priv->adc, priv->channel);
+	if (ret)
+		return ret;
+
+	ret = adc_channel_data(priv->adc, priv->channel, &val);
+	if (ret)
+		return ret;
+
+	ret = adc_data_mask(priv->adc, &mask);
+	if (ret)
+		return ret;
+
+	/* getting state is simplified a bit */
+	if (ret == 0)
+		return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
+
+	return ret;
+}
+
+static int button_adc_probe(struct udevice *dev)
+{
+	struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	struct ofnode_phandle_args args;
+	int ret;
+
+	/* Ignore the top-level button node */
+	if (!uc_plat->label)
+		return 0;
+
+	ret = dev_read_phandle_with_args(dev->parent, "io-channels",
+					 "#io-channel-cells", 0, 0, &args);
+	if (ret)
+		return ret;
+
+	ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
+					&priv->adc);
+	if (ret)
+		return ret;
+
+	priv->channel = args.args[0];
+
+	return ret;
+}
+
+static int button_adc_bind(struct udevice *parent)
+{
+	struct udevice *dev;
+	ofnode node;
+	int ret;
+
+	dev_for_each_subnode(node, parent) {
+		struct button_uc_plat *uc_plat;
+		const char *label;
+
+		label = ofnode_read_string(node, "label");
+		if (!label) {
+			debug("%s: node %s has no label\n", __func__,
+			      ofnode_get_name(node));
+			return -EINVAL;
+		}
+		ret = device_bind_driver_to_node(parent, "button_adc",
+						 ofnode_get_name(node),
+						 node, &dev);
+		if (ret)
+			return ret;
+		uc_plat = dev_get_uclass_platdata(dev);
+		uc_plat->label = label;
+	}
+
+	return 0;
+}
+
+static const struct button_ops button_adc_ops = {
+	.get_state	= button_adc_get_state,
+};
+
+static const struct udevice_id button_adc_ids[] = {
+	{ .compatible = "adc-keys" },
+	{ }
+};
+
+U_BOOT_DRIVER(button_adc) = {
+	.name		= "button_adc",
+	.id		= UCLASS_BUTTON,
+	.of_match	= button_adc_ids,
+	.ops		= &button_adc_ops,
+	.priv_auto_alloc_size = sizeof(struct button_adc_priv),
+	.bind		= button_adc_bind,
+	.probe		= button_adc_probe,
+};
-- 
2.17.1

[PATCH 4/6] button: add a simple ADC-based button driver

[Marek Szyprowski]

Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
device tree node.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
---
 drivers/button/Kconfig      |   8 +++
 drivers/button/Makefile     |   1 +
 drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)
 create mode 100644 drivers/button/button-adc.c

diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
--- /dev/null
+++ b/drivers/button/button-adc.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ */
+
+#include <common.h>
+#include <button.h>
+#include <dm.h>
+#include <dm/lists.h>
+#include <dm/uclass-internal.h>
+#include <log.h>
+#include <adc.h>
+
+struct button_adc_priv {
+	struct udevice *adc;
+	int channel;
+};
+
+static enum button_state_t button_adc_get_state(struct udevice *dev)
+{
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	unsigned int val, mask;
+	int ret;
+
+	ret = adc_start_channel(priv->adc, priv->channel);
+	if (ret)
+		return ret;
+
+	ret = adc_channel_data(priv->adc, priv->channel, &val);
+	if (ret)
+		return ret;
+
+	ret = adc_data_mask(priv->adc, &mask);
+	if (ret)
+		return ret;
+
+	/* getting state is simplified a bit */
+	if (ret == 0)
+		return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
+
+	return ret;
+}
+
+static int button_adc_probe(struct udevice *dev)
+{
+	struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
+	struct button_adc_priv *priv = dev_get_priv(dev);
+	struct ofnode_phandle_args args;
+	int ret;
+
+	/* Ignore the top-level button node */
+	if (!uc_plat->label)
+		return 0;
+
+	ret = dev_read_phandle_with_args(dev->parent, "io-channels",
+					 "#io-channel-cells", 0, 0, &args);
+	if (ret)
+		return ret;
+
+	ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
+					&priv->adc);
+	if (ret)
+		return ret;
+
+	priv->channel = args.args[0];
+
+	return ret;
+}
+
+static int button_adc_bind(struct udevice *parent)
+{
+	struct udevice *dev;
+	ofnode node;
+	int ret;
+
+	dev_for_each_subnode(node, parent) {
+		struct button_uc_plat *uc_plat;
+		const char *label;
+
+		label = ofnode_read_string(node, "label");
+		if (!label) {
+			debug("%s: node %s has no label\n", __func__,
+			      ofnode_get_name(node));
+			return -EINVAL;
+		}
+		ret = device_bind_driver_to_node(parent, "button_adc",
+						 ofnode_get_name(node),
+						 node, &dev);
+		if (ret)
+			return ret;
+		uc_plat = dev_get_uclass_platdata(dev);
+		uc_plat->label = label;
+	}
+
+	return 0;
+}
+
+static const struct button_ops button_adc_ops = {
+	.get_state	= button_adc_get_state,
+};
+
+static const struct udevice_id button_adc_ids[] = {
+	{ .compatible = "adc-keys" },
+	{ }
+};
+
+U_BOOT_DRIVER(button_adc) = {
+	.name		= "button_adc",
+	.id		= UCLASS_BUTTON,
+	.of_match	= button_adc_ids,
+	.ops		= &button_adc_ops,
+	.priv_auto_alloc_size = sizeof(struct button_adc_priv),
+	.bind		= button_adc_bind,
+	.probe		= button_adc_probe,
+};
-- 
2.17.1

[PATCH 5/6] cmd: button: store button state in the 'button' env

[Marek Szyprowski]

Save examined button state in 'button' environment variable to enable
checking button state in the scripts.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I78b539e1516573fcfea4401f75469291844daae4
---
 cmd/button.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cmd/button.c b/cmd/button.c
index 64c5a8fa046..8da911068a4 100644
--- a/cmd/button.c
+++ b/cmd/button.c
@@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev)
 	ret = button_get_state(dev);
 	if (ret >= BUTTON_COUNT)
 		ret = -EINVAL;
-	if (ret >= 0)
+	if (ret >= 0) {
 		printf("%s\n", state_label[ret]);
+		env_set("button", state_label[ret]);
+	}
 
 	return ret;
 }
-- 
2.17.1

[PATCH 5/6] cmd: button: store button state in the 'button' env

[Marek Szyprowski]

Save examined button state in 'button' environment variable to enable
checking button state in the scripts.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Change-Id: I78b539e1516573fcfea4401f75469291844daae4
---
 cmd/button.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cmd/button.c b/cmd/button.c
index 64c5a8fa046..8da911068a4 100644
--- a/cmd/button.c
+++ b/cmd/button.c
@@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev)
 	ret = button_get_state(dev);
 	if (ret >= BUTTON_COUNT)
 		ret = -EINVAL;
-	if (ret >= 0)
+	if (ret >= 0) {
 		printf("%s\n", state_label[ret]);
+		env_set("button", state_label[ret]);
+	}
 
 	return ret;
 }
-- 
2.17.1

[PATCH 6/6] configs: khadas-vim3: 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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
index 9d7ba72d440..bc174305692 100644
--- a/configs/khadas-vim3_defconfig
+++ b/configs/khadas-vim3_defconfig
@@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
 CONFIG_OF_CONTROL=y
 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 6/6] configs: khadas-vim3: 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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
index 9d7ba72d440..bc174305692 100644
--- a/configs/khadas-vim3_defconfig
+++ b/configs/khadas-vim3_defconfig
@@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
 CONFIG_OF_CONTROL=y
 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 1/6] clk: meson: add minimal driver for g12a-ao clocks

[Neil Armstrong]

Hi,

On 14/12/2020 12:24, Marek Szyprowski wrote:
> Add minimal driver AO clocks on meson G12A family. Only ADC related clocks
> are supported.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I0c91848bc55493e19570db333e7da6020d687907

Please drop the Change-Ids

> ---
>  drivers/clk/meson/Makefile  |  1 +
>  drivers/clk/meson/g12a-ao.c | 83 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
>  create mode 100644 drivers/clk/meson/g12a-ao.c
> 
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index c873d6976fa..7204383e173 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,4 +6,5 @@
>  obj-$(CONFIG_CLK_MESON_GX) += gxbb.o
>  obj-$(CONFIG_CLK_MESON_AXG) += axg.o
>  obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
> +obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
>  
> diff --git a/drivers/clk/meson/g12a-ao.c b/drivers/clk/meson/g12a-ao.c
> new file mode 100644
> index 00000000000..7a0abea77c4
> --- /dev/null
> +++ b/drivers/clk/meson/g12a-ao.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <common.h>
> +#include <log.h>
> +#include <asm/io.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <dt-bindings/clock/g12a-aoclkc.h>
> +
> +#include "clk_meson.h"
> +
> +struct meson_clk {
> +	struct regmap *map;
> +};
> +
> +#define AO_CLK_GATE0		0x4c
> +#define AO_SAR_CLK		0x90
> +
> +static struct meson_gate gates[] = {
> +	MESON_GATE(CLKID_AO_SAR_ADC, AO_CLK_GATE0, 8),
> +	MESON_GATE(CLKID_AO_SAR_ADC_CLK, AO_SAR_CLK, 8),
> +};
> +
> +static int meson_set_gate(struct clk *clk, bool on)
> +{
> +	struct meson_clk *priv = dev_get_priv(clk->dev);
> +	struct meson_gate *gate;
> +
> +	if (clk->id >= ARRAY_SIZE(gates))
> +		return -ENOENT;
> +
> +	gate = &gates[clk->id];
> +
> +	if (gate->reg == 0)
> +		return 0;
> +
> +	regmap_update_bits(priv->map, gate->reg,
> +			   BIT(gate->bit), on ? BIT(gate->bit) : 0);
> +
> +	return 0;
> +}
> +
> +static int meson_clk_enable(struct clk *clk)
> +{
> +	return meson_set_gate(clk, true);
> +}
> +
> +static int meson_clk_disable(struct clk *clk)
> +{
> +	return meson_set_gate(clk, false);
> +}
> +
> +static int meson_clk_probe(struct udevice *dev)
> +{
> +	struct meson_clk *priv = dev_get_priv(dev);
> +
> +	priv->map = syscon_node_to_regmap(dev_get_parent(dev)->node);
> +	if (IS_ERR(priv->map))
> +		return PTR_ERR(priv->map);
> +
> +	return 0;
> +}
> +
> +static struct clk_ops meson_clk_ops = {
> +	.disable	= meson_clk_disable,
> +	.enable		= meson_clk_enable,
> +};
> +
> +static const struct udevice_id meson_clk_ids[] = {
> +	{ .compatible = "amlogic,meson-g12a-aoclkc" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(meson_clk_axg) = {
> +	.name		= "meson_clk_g12a_ao",
> +	.id		= UCLASS_CLK,
> +	.of_match	= meson_clk_ids,
> +	.priv_auto_alloc_size = sizeof(struct meson_clk),
> +	.ops		= &meson_clk_ops,
> +	.probe		= meson_clk_probe,
> +};
> 

very nice, thanks for addition !

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH 1/6] clk: meson: add minimal driver for g12a-ao clocks

[Neil Armstrong]

Hi,

On 14/12/2020 12:24, Marek Szyprowski wrote:
> Add minimal driver AO clocks on meson G12A family. Only ADC related clocks
> are supported.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I0c91848bc55493e19570db333e7da6020d687907

Please drop the Change-Ids

> ---
>  drivers/clk/meson/Makefile  |  1 +
>  drivers/clk/meson/g12a-ao.c | 83 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
>  create mode 100644 drivers/clk/meson/g12a-ao.c
> 
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index c873d6976fa..7204383e173 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -6,4 +6,5 @@
>  obj-$(CONFIG_CLK_MESON_GX) += gxbb.o
>  obj-$(CONFIG_CLK_MESON_AXG) += axg.o
>  obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
> +obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
>  
> diff --git a/drivers/clk/meson/g12a-ao.c b/drivers/clk/meson/g12a-ao.c
> new file mode 100644
> index 00000000000..7a0abea77c4
> --- /dev/null
> +++ b/drivers/clk/meson/g12a-ao.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <common.h>
> +#include <log.h>
> +#include <asm/io.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <dt-bindings/clock/g12a-aoclkc.h>
> +
> +#include "clk_meson.h"
> +
> +struct meson_clk {
> +	struct regmap *map;
> +};
> +
> +#define AO_CLK_GATE0		0x4c
> +#define AO_SAR_CLK		0x90
> +
> +static struct meson_gate gates[] = {
> +	MESON_GATE(CLKID_AO_SAR_ADC, AO_CLK_GATE0, 8),
> +	MESON_GATE(CLKID_AO_SAR_ADC_CLK, AO_SAR_CLK, 8),
> +};
> +
> +static int meson_set_gate(struct clk *clk, bool on)
> +{
> +	struct meson_clk *priv = dev_get_priv(clk->dev);
> +	struct meson_gate *gate;
> +
> +	if (clk->id >= ARRAY_SIZE(gates))
> +		return -ENOENT;
> +
> +	gate = &gates[clk->id];
> +
> +	if (gate->reg == 0)
> +		return 0;
> +
> +	regmap_update_bits(priv->map, gate->reg,
> +			   BIT(gate->bit), on ? BIT(gate->bit) : 0);
> +
> +	return 0;
> +}
> +
> +static int meson_clk_enable(struct clk *clk)
> +{
> +	return meson_set_gate(clk, true);
> +}
> +
> +static int meson_clk_disable(struct clk *clk)
> +{
> +	return meson_set_gate(clk, false);
> +}
> +
> +static int meson_clk_probe(struct udevice *dev)
> +{
> +	struct meson_clk *priv = dev_get_priv(dev);
> +
> +	priv->map = syscon_node_to_regmap(dev_get_parent(dev)->node);
> +	if (IS_ERR(priv->map))
> +		return PTR_ERR(priv->map);
> +
> +	return 0;
> +}
> +
> +static struct clk_ops meson_clk_ops = {
> +	.disable	= meson_clk_disable,
> +	.enable		= meson_clk_enable,
> +};
> +
> +static const struct udevice_id meson_clk_ids[] = {
> +	{ .compatible = "amlogic,meson-g12a-aoclkc" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(meson_clk_axg) = {
> +	.name		= "meson_clk_g12a_ao",
> +	.id		= UCLASS_CLK,
> +	.of_match	= meson_clk_ids,
> +	.priv_auto_alloc_size = sizeof(struct meson_clk),
> +	.ops		= &meson_clk_ops,
> +	.probe		= meson_clk_probe,
> +};
> 

very nice, thanks for addition !

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[PATCH 2/6] adc: meson-saradc: add G12A variant

[Neil Armstrong]

On 14/12/2020 12:24, Marek Szyprowski wrote:
> Add support for the SARADC variant found on the G12A SoCs family.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: If519d333e9773d089f37a8c7b4ccb144be68925b

Ditto

> ---
>  drivers/adc/meson-saradc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
> index 72b0cc4e5bd..998cef24d88 100644
> --- a/drivers/adc/meson-saradc.c
> +++ b/drivers/adc/meson-saradc.c
> @@ -711,6 +711,8 @@ static const struct udevice_id meson_saradc_ids[] = {
>  	  .data = (ulong)&gxl_saradc_data },
>  	{ .compatible = "amlogic,meson-gxm-saradc",
>  	  .data = (ulong)&gxl_saradc_data },
> +	{ .compatible = "amlogic,meson-g12a-saradc",
> +	  .data = (ulong)&gxl_saradc_data },
>  	{ }
>  };
>  
> 


Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH 2/6] adc: meson-saradc: add G12A variant

[Neil Armstrong]

On 14/12/2020 12:24, Marek Szyprowski wrote:
> Add support for the SARADC variant found on the G12A SoCs family.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: If519d333e9773d089f37a8c7b4ccb144be68925b

Ditto

> ---
>  drivers/adc/meson-saradc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
> index 72b0cc4e5bd..998cef24d88 100644
> --- a/drivers/adc/meson-saradc.c
> +++ b/drivers/adc/meson-saradc.c
> @@ -711,6 +711,8 @@ static const struct udevice_id meson_saradc_ids[] = {
>  	  .data = (ulong)&gxl_saradc_data },
>  	{ .compatible = "amlogic,meson-gxm-saradc",
>  	  .data = (ulong)&gxl_saradc_data },
> +	{ .compatible = "amlogic,meson-g12a-saradc",
> +	  .data = (ulong)&gxl_saradc_data },
>  	{ }
>  };
>  
> 


Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[PATCH 3/6] adc: meson-saradc: skip hardware init only if ADC is enabled

[Neil Armstrong]

On 14/12/2020 12:24, Marek Szyprowski wrote:
> The driver skips hardware initialization if it is already configured by
> the earlier bootloader stage (BL30). Skip the initialization only if the
> hardware is really initialized and enabled.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I3e2e2d270ad3e7e7f38e2bc3ce2a924a47b164af

Ditto

> ---
>  drivers/adc/meson-saradc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
> index 998cef24d88..ce7ae990ad0 100644
> --- a/drivers/adc/meson-saradc.c
> +++ b/drivers/adc/meson-saradc.c
> @@ -512,8 +512,11 @@ static int meson_saradc_init(struct meson_saradc_priv *priv)
>  	 * reading the temperature sensor.
>  	 */
>  	regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
> -	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
> -		return 0;
> +	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED) {
> +		regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
> +		if (regval & MESON_SAR_ADC_REG3_ADC_EN)
> +			return 0;
> +	}
>  
>  	meson_saradc_stop_sample_engine(priv);
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH 3/6] adc: meson-saradc: skip hardware init only if ADC is enabled

[Neil Armstrong]

On 14/12/2020 12:24, Marek Szyprowski wrote:
> The driver skips hardware initialization if it is already configured by
> the earlier bootloader stage (BL30). Skip the initialization only if the
> hardware is really initialized and enabled.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I3e2e2d270ad3e7e7f38e2bc3ce2a924a47b164af

Ditto

> ---
>  drivers/adc/meson-saradc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
> index 998cef24d88..ce7ae990ad0 100644
> --- a/drivers/adc/meson-saradc.c
> +++ b/drivers/adc/meson-saradc.c
> @@ -512,8 +512,11 @@ static int meson_saradc_init(struct meson_saradc_priv *priv)
>  	 * reading the temperature sensor.
>  	 */
>  	regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
> -	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
> -		return 0;
> +	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED) {
> +		regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
> +		if (regval & MESON_SAR_ADC_REG3_ADC_EN)
> +			return 0;
> +	}
>  
>  	meson_saradc_stop_sample_engine(priv);
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[PATCH 6/6] configs: khadas-vim3: enable Function button support

[Neil Armstrong]

Hi,

On 14/12/2020 12:24, Marek Szyprowski wrote:
> Add options required to check the 'Function' button state.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  configs/khadas-vim3_defconfig | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
> index 9d7ba72d440..bc174305692 100644
> --- a/configs/khadas-vim3_defconfig
> +++ b/configs/khadas-vim3_defconfig
> @@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
>  CONFIG_OF_CONTROL=y
>  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
> 

If you separate the ADC configs and Button configs, I can directly apply the clk-ao & adc patches
for next release.

Neil

Re: [PATCH 6/6] configs: khadas-vim3: enable Function button support

[Neil Armstrong]

Hi,

On 14/12/2020 12:24, Marek Szyprowski wrote:
> Add options required to check the 'Function' button state.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  configs/khadas-vim3_defconfig | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
> index 9d7ba72d440..bc174305692 100644
> --- a/configs/khadas-vim3_defconfig
> +++ b/configs/khadas-vim3_defconfig
> @@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
>  CONFIG_OF_CONTROL=y
>  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
> 

If you separate the ADC configs and Button configs, I can directly apply the clk-ao & adc patches
for next release.

Neil

[PATCH 6/6] configs: khadas-vim3: enable Function button support

[Marek Szyprowski]

Hi Neil,

On 14.12.2020 19:55, Neil Armstrong wrote:
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add options required to check the 'Function' button state.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   configs/khadas-vim3_defconfig | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
>> index 9d7ba72d440..bc174305692 100644
>> --- a/configs/khadas-vim3_defconfig
>> +++ b/configs/khadas-vim3_defconfig
>> @@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
>>   CONFIG_OF_CONTROL=y
>>   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
>>
> If you separate the ADC configs and Button configs, I can directly apply the clk-ao & adc patches
> for next release.

Sure, no problem. I'm sorry for the change-id mess, I forgot to clean it 
before submission. I will send v2 soon.

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

Re: [PATCH 6/6] configs: khadas-vim3: enable Function button support

[Marek Szyprowski]

Hi Neil,

On 14.12.2020 19:55, Neil Armstrong wrote:
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add options required to check the 'Function' button state.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   configs/khadas-vim3_defconfig | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
>> index 9d7ba72d440..bc174305692 100644
>> --- a/configs/khadas-vim3_defconfig
>> +++ b/configs/khadas-vim3_defconfig
>> @@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
>>   CONFIG_OF_CONTROL=y
>>   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
>>
> If you separate the ADC configs and Button configs, I can directly apply the clk-ao & adc patches
> for next release.

Sure, no problem. I'm sorry for the change-id mess, I forgot to clean it 
before submission. I will send v2 soon.

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

[PATCH 1/6] clk: meson: add minimal driver for g12a-ao clocks

[Jaehoon Chung]

On 12/15/20 3:53 AM, Neil Armstrong wrote:
> Hi,
> 
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add minimal driver AO clocks on meson G12A family. Only ADC related clocks
>> are supported.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: I0c91848bc55493e19570db333e7da6020d687907
> 
> Please drop the Change-Ids
> 
>> ---
>>  drivers/clk/meson/Makefile  |  1 +
>>  drivers/clk/meson/g12a-ao.c | 83 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 84 insertions(+)
>>  create mode 100644 drivers/clk/meson/g12a-ao.c
>>
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index c873d6976fa..7204383e173 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -6,4 +6,5 @@
>>  obj-$(CONFIG_CLK_MESON_GX) += gxbb.o
>>  obj-$(CONFIG_CLK_MESON_AXG) += axg.o
>>  obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
>> +obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
>>  
>> diff --git a/drivers/clk/meson/g12a-ao.c b/drivers/clk/meson/g12a-ao.c
>> new file mode 100644
>> index 00000000000..7a0abea77c4
>> --- /dev/null
>> +++ b/drivers/clk/meson/g12a-ao.c
>> @@ -0,0 +1,83 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#include <common.h>
>> +#include <log.h>
>> +#include <asm/io.h>
>> +#include <clk-uclass.h>
>> +#include <dm.h>
>> +#include <regmap.h>
>> +#include <syscon.h>
>> +#include <dt-bindings/clock/g12a-aoclkc.h>
>> +
>> +#include "clk_meson.h"
>> +
>> +struct meson_clk {
>> +	struct regmap *map;
>> +};
>> +
>> +#define AO_CLK_GATE0		0x4c
>> +#define AO_SAR_CLK		0x90
>> +
>> +static struct meson_gate gates[] = {
>> +	MESON_GATE(CLKID_AO_SAR_ADC, AO_CLK_GATE0, 8),
>> +	MESON_GATE(CLKID_AO_SAR_ADC_CLK, AO_SAR_CLK, 8),
>> +};
>> +
>> +static int meson_set_gate(struct clk *clk, bool on)
>> +{
>> +	struct meson_clk *priv = dev_get_priv(clk->dev);
>> +	struct meson_gate *gate;
>> +
>> +	if (clk->id >= ARRAY_SIZE(gates))
>> +		return -ENOENT;
>> +
>> +	gate = &gates[clk->id];
>> +
>> +	if (gate->reg == 0)
>> +		return 0;
>> +
>> +	regmap_update_bits(priv->map, gate->reg,
>> +			   BIT(gate->bit), on ? BIT(gate->bit) : 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_clk_enable(struct clk *clk)
>> +{
>> +	return meson_set_gate(clk, true);
>> +}
>> +
>> +static int meson_clk_disable(struct clk *clk)
>> +{
>> +	return meson_set_gate(clk, false);
>> +}
>> +
>> +static int meson_clk_probe(struct udevice *dev)
>> +{
>> +	struct meson_clk *priv = dev_get_priv(dev);
>> +
>> +	priv->map = syscon_node_to_regmap(dev_get_parent(dev)->node);
>> +	if (IS_ERR(priv->map))
>> +		return PTR_ERR(priv->map);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct clk_ops meson_clk_ops = {
>> +	.disable	= meson_clk_disable,
>> +	.enable		= meson_clk_enable,
>> +};
>> +
>> +static const struct udevice_id meson_clk_ids[] = {
>> +	{ .compatible = "amlogic,meson-g12a-aoclkc" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(meson_clk_axg) = {
>> +	.name		= "meson_clk_g12a_ao",
>> +	.id		= UCLASS_CLK,
>> +	.of_match	= meson_clk_ids,
>> +	.priv_auto_alloc_size = sizeof(struct meson_clk),
>> +	.ops		= &meson_clk_ops,
>> +	.probe		= meson_clk_probe,
>> +};
>>
> 
> very nice, thanks for addition !
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

> 

Re: [PATCH 1/6] clk: meson: add minimal driver for g12a-ao clocks

[Jaehoon Chung]

On 12/15/20 3:53 AM, Neil Armstrong wrote:
> Hi,
> 
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add minimal driver AO clocks on meson G12A family. Only ADC related clocks
>> are supported.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: I0c91848bc55493e19570db333e7da6020d687907
> 
> Please drop the Change-Ids
> 
>> ---
>>  drivers/clk/meson/Makefile  |  1 +
>>  drivers/clk/meson/g12a-ao.c | 83 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 84 insertions(+)
>>  create mode 100644 drivers/clk/meson/g12a-ao.c
>>
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index c873d6976fa..7204383e173 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -6,4 +6,5 @@
>>  obj-$(CONFIG_CLK_MESON_GX) += gxbb.o
>>  obj-$(CONFIG_CLK_MESON_AXG) += axg.o
>>  obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
>> +obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
>>  
>> diff --git a/drivers/clk/meson/g12a-ao.c b/drivers/clk/meson/g12a-ao.c
>> new file mode 100644
>> index 00000000000..7a0abea77c4
>> --- /dev/null
>> +++ b/drivers/clk/meson/g12a-ao.c
>> @@ -0,0 +1,83 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#include <common.h>
>> +#include <log.h>
>> +#include <asm/io.h>
>> +#include <clk-uclass.h>
>> +#include <dm.h>
>> +#include <regmap.h>
>> +#include <syscon.h>
>> +#include <dt-bindings/clock/g12a-aoclkc.h>
>> +
>> +#include "clk_meson.h"
>> +
>> +struct meson_clk {
>> +	struct regmap *map;
>> +};
>> +
>> +#define AO_CLK_GATE0		0x4c
>> +#define AO_SAR_CLK		0x90
>> +
>> +static struct meson_gate gates[] = {
>> +	MESON_GATE(CLKID_AO_SAR_ADC, AO_CLK_GATE0, 8),
>> +	MESON_GATE(CLKID_AO_SAR_ADC_CLK, AO_SAR_CLK, 8),
>> +};
>> +
>> +static int meson_set_gate(struct clk *clk, bool on)
>> +{
>> +	struct meson_clk *priv = dev_get_priv(clk->dev);
>> +	struct meson_gate *gate;
>> +
>> +	if (clk->id >= ARRAY_SIZE(gates))
>> +		return -ENOENT;
>> +
>> +	gate = &gates[clk->id];
>> +
>> +	if (gate->reg == 0)
>> +		return 0;
>> +
>> +	regmap_update_bits(priv->map, gate->reg,
>> +			   BIT(gate->bit), on ? BIT(gate->bit) : 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_clk_enable(struct clk *clk)
>> +{
>> +	return meson_set_gate(clk, true);
>> +}
>> +
>> +static int meson_clk_disable(struct clk *clk)
>> +{
>> +	return meson_set_gate(clk, false);
>> +}
>> +
>> +static int meson_clk_probe(struct udevice *dev)
>> +{
>> +	struct meson_clk *priv = dev_get_priv(dev);
>> +
>> +	priv->map = syscon_node_to_regmap(dev_get_parent(dev)->node);
>> +	if (IS_ERR(priv->map))
>> +		return PTR_ERR(priv->map);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct clk_ops meson_clk_ops = {
>> +	.disable	= meson_clk_disable,
>> +	.enable		= meson_clk_enable,
>> +};
>> +
>> +static const struct udevice_id meson_clk_ids[] = {
>> +	{ .compatible = "amlogic,meson-g12a-aoclkc" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(meson_clk_axg) = {
>> +	.name		= "meson_clk_g12a_ao",
>> +	.id		= UCLASS_CLK,
>> +	.of_match	= meson_clk_ids,
>> +	.priv_auto_alloc_size = sizeof(struct meson_clk),
>> +	.ops		= &meson_clk_ops,
>> +	.probe		= meson_clk_probe,
>> +};
>>
> 
> very nice, thanks for addition !
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

> 

[PATCH 2/6] adc: meson-saradc: add G12A variant

[Jaehoon Chung]

On 12/15/20 3:54 AM, Neil Armstrong wrote:
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add support for the SARADC variant found on the G12A SoCs family.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: If519d333e9773d089f37a8c7b4ccb144be68925b
> 
> Ditto
> 
>> ---
>>  drivers/adc/meson-saradc.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
>> index 72b0cc4e5bd..998cef24d88 100644
>> --- a/drivers/adc/meson-saradc.c
>> +++ b/drivers/adc/meson-saradc.c
>> @@ -711,6 +711,8 @@ static const struct udevice_id meson_saradc_ids[] = {
>>  	  .data = (ulong)&gxl_saradc_data },
>>  	{ .compatible = "amlogic,meson-gxm-saradc",
>>  	  .data = (ulong)&gxl_saradc_data },
>> +	{ .compatible = "amlogic,meson-g12a-saradc",
>> +	  .data = (ulong)&gxl_saradc_data },
>>  	{ }
>>  };
>>  
>>
> 
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

> 

Re: [PATCH 2/6] adc: meson-saradc: add G12A variant

[Jaehoon Chung]

On 12/15/20 3:54 AM, Neil Armstrong wrote:
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add support for the SARADC variant found on the G12A SoCs family.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: If519d333e9773d089f37a8c7b4ccb144be68925b
> 
> Ditto
> 
>> ---
>>  drivers/adc/meson-saradc.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
>> index 72b0cc4e5bd..998cef24d88 100644
>> --- a/drivers/adc/meson-saradc.c
>> +++ b/drivers/adc/meson-saradc.c
>> @@ -711,6 +711,8 @@ static const struct udevice_id meson_saradc_ids[] = {
>>  	  .data = (ulong)&gxl_saradc_data },
>>  	{ .compatible = "amlogic,meson-gxm-saradc",
>>  	  .data = (ulong)&gxl_saradc_data },
>> +	{ .compatible = "amlogic,meson-g12a-saradc",
>> +	  .data = (ulong)&gxl_saradc_data },
>>  	{ }
>>  };
>>  
>>
> 
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

> 

[PATCH 3/6] adc: meson-saradc: skip hardware init only if ADC is enabled

[Jaehoon Chung]

On 12/15/20 3:54 AM, Neil Armstrong wrote:
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> The driver skips hardware initialization if it is already configured by
>> the earlier bootloader stage (BL30). Skip the initialization only if the
>> hardware is really initialized and enabled.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: I3e2e2d270ad3e7e7f38e2bc3ce2a924a47b164af
> 
> Ditto
> 
>> ---
>>  drivers/adc/meson-saradc.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
>> index 998cef24d88..ce7ae990ad0 100644
>> --- a/drivers/adc/meson-saradc.c
>> +++ b/drivers/adc/meson-saradc.c
>> @@ -512,8 +512,11 @@ static int meson_saradc_init(struct meson_saradc_priv *priv)
>>  	 * reading the temperature sensor.
>>  	 */
>>  	regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
>> -	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
>> -		return 0;
>> +	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED) {
>> +		regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
>> +		if (regval & MESON_SAR_ADC_REG3_ADC_EN)
>> +			return 0;
>> +	}
>>  
>>  	meson_saradc_stop_sample_engine(priv);
>>  
>>
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

> 

Re: [PATCH 3/6] adc: meson-saradc: skip hardware init only if ADC is enabled

[Jaehoon Chung]

On 12/15/20 3:54 AM, Neil Armstrong wrote:
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> The driver skips hardware initialization if it is already configured by
>> the earlier bootloader stage (BL30). Skip the initialization only if the
>> hardware is really initialized and enabled.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: I3e2e2d270ad3e7e7f38e2bc3ce2a924a47b164af
> 
> Ditto
> 
>> ---
>>  drivers/adc/meson-saradc.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
>> index 998cef24d88..ce7ae990ad0 100644
>> --- a/drivers/adc/meson-saradc.c
>> +++ b/drivers/adc/meson-saradc.c
>> @@ -512,8 +512,11 @@ static int meson_saradc_init(struct meson_saradc_priv *priv)
>>  	 * reading the temperature sensor.
>>  	 */
>>  	regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
>> -	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
>> -		return 0;
>> +	if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED) {
>> +		regmap_read(priv->regmap, MESON_SAR_ADC_REG3, &regval);
>> +		if (regval & MESON_SAR_ADC_REG3_ADC_EN)
>> +			return 0;
>> +	}
>>  
>>  	meson_saradc_stop_sample_engine(priv);
>>  
>>
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

> 

[PATCH 6/6] configs: khadas-vim3: enable Function button support

[Jaehoon Chung]

On 12/15/20 3:55 AM, Neil Armstrong wrote:
> Hi,
> 
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add options required to check the 'Function' button state.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

>> ---
>>  configs/khadas-vim3_defconfig | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
>> index 9d7ba72d440..bc174305692 100644
>> --- a/configs/khadas-vim3_defconfig
>> +++ b/configs/khadas-vim3_defconfig
>> @@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
>>  CONFIG_OF_CONTROL=y
>>  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
>>
> 
> If you separate the ADC configs and Button configs, I can directly apply the clk-ao & adc patches
> for next release.
> 
> Neil
> 

Re: [PATCH 6/6] configs: khadas-vim3: enable Function button support

[Jaehoon Chung]

On 12/15/20 3:55 AM, Neil Armstrong wrote:
> Hi,
> 
> On 14/12/2020 12:24, Marek Szyprowski wrote:
>> Add options required to check the 'Function' button state.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

>> ---
>>  configs/khadas-vim3_defconfig | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/configs/khadas-vim3_defconfig b/configs/khadas-vim3_defconfig
>> index 9d7ba72d440..bc174305692 100644
>> --- a/configs/khadas-vim3_defconfig
>> +++ b/configs/khadas-vim3_defconfig
>> @@ -29,6 +29,10 @@ CONFIG_CMD_REGULATOR=y
>>  CONFIG_OF_CONTROL=y
>>  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
>>
> 
> If you separate the ADC configs and Button configs, I can directly apply the clk-ao & adc patches
> for next release.
> 
> Neil
> 

[PATCH 4/6] button: add a simple ADC-based button driver

[Simon Glass]

Hi Marek,

On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
> device tree node.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
> ---
>  drivers/button/Kconfig      |   8 +++
>  drivers/button/Makefile     |   1 +
>  drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 126 insertions(+)
>  create mode 100644 drivers/button/button-adc.c
>
> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
> --- /dev/null
> +++ b/drivers/button/button-adc.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
> + *             http://www.samsung.com
> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> + */
> +
> +#include <common.h>
> +#include <button.h>
> +#include <dm.h>
> +#include <dm/lists.h>
> +#include <dm/uclass-internal.h>
> +#include <log.h>
> +#include <adc.h>

Please check header order

https://www.denx.de/wiki/U-Boot/CodingStyle

> +
> +struct button_adc_priv {
> +       struct udevice *adc;
> +       int channel;

comments

> +};
> +
> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> +{
> +       struct button_adc_priv *priv = dev_get_priv(dev);
> +       unsigned int val, mask;
> +       int ret;
> +
> +       ret = adc_start_channel(priv->adc, priv->channel);
> +       if (ret)
> +               return ret;
> +
> +       ret = adc_channel_data(priv->adc, priv->channel, &val);
> +       if (ret)
> +               return ret;
> +
> +       ret = adc_data_mask(priv->adc, &mask);
> +       if (ret)
> +               return ret;
> +
> +       /* getting state is simplified a bit */
> +       if (ret == 0)
> +               return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
> +
> +       return ret;
> +}
> +
> +static int button_adc_probe(struct udevice *dev)
> +{
> +       struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +       struct button_adc_priv *priv = dev_get_priv(dev);
> +       struct ofnode_phandle_args args;
> +       int ret;
> +
> +       /* Ignore the top-level button node */
> +       if (!uc_plat->label)
> +               return 0;
> +
> +       ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> +                                        "#io-channel-cells", 0, 0, &args);
> +       if (ret)
> +               return ret;
> +
> +       ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
> +                                       &priv->adc);

How about uclass_get_device_by_ofnode() ?

> +       if (ret)
> +               return ret;
> +
> +       priv->channel = args.args[0];
> +
> +       return ret;
> +}
> +
> +static int button_adc_bind(struct udevice *parent)
> +{
> +       struct udevice *dev;
> +       ofnode node;
> +       int ret;
> +
> +       dev_for_each_subnode(node, parent) {
> +               struct button_uc_plat *uc_plat;
> +               const char *label;
> +
> +               label = ofnode_read_string(node, "label");
> +               if (!label) {
> +                       debug("%s: node %s has no label\n", __func__,
> +                             ofnode_get_name(node));
> +                       return -EINVAL;
> +               }
> +               ret = device_bind_driver_to_node(parent, "button_adc",
> +                                                ofnode_get_name(node),
> +                                                node, &dev);
> +               if (ret)
> +                       return ret;
> +               uc_plat = dev_get_uclass_platdata(dev);
> +               uc_plat->label = label;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct button_ops button_adc_ops = {
> +       .get_state      = button_adc_get_state,
> +};
> +
> +static const struct udevice_id button_adc_ids[] = {
> +       { .compatible = "adc-keys" },

Please add the binding file for this.

> +       { }
> +};
> +
> +U_BOOT_DRIVER(button_adc) = {
> +       .name           = "button_adc",
> +       .id             = UCLASS_BUTTON,
> +       .of_match       = button_adc_ids,
> +       .ops            = &button_adc_ops,
> +       .priv_auto_alloc_size = sizeof(struct button_adc_priv),
> +       .bind           = button_adc_bind,
> +       .probe          = button_adc_probe,
> +};
> --
> 2.17.1
>

Regards,
Simon

Re: [PATCH 4/6] button: add a simple ADC-based button driver

[Simon Glass]

Hi Marek,

On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
> device tree node.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
> ---
>  drivers/button/Kconfig      |   8 +++
>  drivers/button/Makefile     |   1 +
>  drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 126 insertions(+)
>  create mode 100644 drivers/button/button-adc.c
>
> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
> --- /dev/null
> +++ b/drivers/button/button-adc.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
> + *             http://www.samsung.com
> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> + */
> +
> +#include <common.h>
> +#include <button.h>
> +#include <dm.h>
> +#include <dm/lists.h>
> +#include <dm/uclass-internal.h>
> +#include <log.h>
> +#include <adc.h>

Please check header order

https://www.denx.de/wiki/U-Boot/CodingStyle

> +
> +struct button_adc_priv {
> +       struct udevice *adc;
> +       int channel;

comments

> +};
> +
> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> +{
> +       struct button_adc_priv *priv = dev_get_priv(dev);
> +       unsigned int val, mask;
> +       int ret;
> +
> +       ret = adc_start_channel(priv->adc, priv->channel);
> +       if (ret)
> +               return ret;
> +
> +       ret = adc_channel_data(priv->adc, priv->channel, &val);
> +       if (ret)
> +               return ret;
> +
> +       ret = adc_data_mask(priv->adc, &mask);
> +       if (ret)
> +               return ret;
> +
> +       /* getting state is simplified a bit */
> +       if (ret == 0)
> +               return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
> +
> +       return ret;
> +}
> +
> +static int button_adc_probe(struct udevice *dev)
> +{
> +       struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> +       struct button_adc_priv *priv = dev_get_priv(dev);
> +       struct ofnode_phandle_args args;
> +       int ret;
> +
> +       /* Ignore the top-level button node */
> +       if (!uc_plat->label)
> +               return 0;
> +
> +       ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> +                                        "#io-channel-cells", 0, 0, &args);
> +       if (ret)
> +               return ret;
> +
> +       ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
> +                                       &priv->adc);

How about uclass_get_device_by_ofnode() ?

> +       if (ret)
> +               return ret;
> +
> +       priv->channel = args.args[0];
> +
> +       return ret;
> +}
> +
> +static int button_adc_bind(struct udevice *parent)
> +{
> +       struct udevice *dev;
> +       ofnode node;
> +       int ret;
> +
> +       dev_for_each_subnode(node, parent) {
> +               struct button_uc_plat *uc_plat;
> +               const char *label;
> +
> +               label = ofnode_read_string(node, "label");
> +               if (!label) {
> +                       debug("%s: node %s has no label\n", __func__,
> +                             ofnode_get_name(node));
> +                       return -EINVAL;
> +               }
> +               ret = device_bind_driver_to_node(parent, "button_adc",
> +                                                ofnode_get_name(node),
> +                                                node, &dev);
> +               if (ret)
> +                       return ret;
> +               uc_plat = dev_get_uclass_platdata(dev);
> +               uc_plat->label = label;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct button_ops button_adc_ops = {
> +       .get_state      = button_adc_get_state,
> +};
> +
> +static const struct udevice_id button_adc_ids[] = {
> +       { .compatible = "adc-keys" },

Please add the binding file for this.

> +       { }
> +};
> +
> +U_BOOT_DRIVER(button_adc) = {
> +       .name           = "button_adc",
> +       .id             = UCLASS_BUTTON,
> +       .of_match       = button_adc_ids,
> +       .ops            = &button_adc_ops,
> +       .priv_auto_alloc_size = sizeof(struct button_adc_priv),
> +       .bind           = button_adc_bind,
> +       .probe          = button_adc_probe,
> +};
> --
> 2.17.1
>

Regards,
Simon

[PATCH 5/6] cmd: button: store button state in the 'button' env

[Simon Glass]

Hi Marek,

On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Save examined button state in 'button' environment variable to enable
> checking button state in the scripts.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I78b539e1516573fcfea4401f75469291844daae4
> ---
>  cmd/button.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Can you add this to the README /docs where it talks about env vars?

>
> diff --git a/cmd/button.c b/cmd/button.c
> index 64c5a8fa046..8da911068a4 100644
> --- a/cmd/button.c
> +++ b/cmd/button.c
> @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev)
>         ret = button_get_state(dev);
>         if (ret >= BUTTON_COUNT)
>                 ret = -EINVAL;
> -       if (ret >= 0)
> +       if (ret >= 0) {
>                 printf("%s\n", state_label[ret]);
> +               env_set("button", state_label[ret]);
> +       }
>
>         return ret;
>  }
> --
> 2.17.1
>

Regards,
Simon

Re: [PATCH 5/6] cmd: button: store button state in the 'button' env

[Simon Glass]

Hi Marek,

On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Save examined button state in 'button' environment variable to enable
> checking button state in the scripts.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Change-Id: I78b539e1516573fcfea4401f75469291844daae4
> ---
>  cmd/button.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Can you add this to the README /docs where it talks about env vars?

>
> diff --git a/cmd/button.c b/cmd/button.c
> index 64c5a8fa046..8da911068a4 100644
> --- a/cmd/button.c
> +++ b/cmd/button.c
> @@ -23,8 +23,10 @@ static int show_button_state(struct udevice *dev)
>         ret = button_get_state(dev);
>         if (ret >= BUTTON_COUNT)
>                 ret = -EINVAL;
> -       if (ret >= 0)
> +       if (ret >= 0) {
>                 printf("%s\n", state_label[ret]);
> +               env_set("button", state_label[ret]);
> +       }
>
>         return ret;
>  }
> --
> 2.17.1
>

Regards,
Simon

[PATCH 4/6] button: add a simple ADC-based button driver

[Marek Szyprowski]

Hi Simon,

On 19.12.2020 03:28, Simon Glass wrote:
> On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
>> device tree node.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
>> ---
>>   drivers/button/Kconfig      |   8 +++
>>   drivers/button/Makefile     |   1 +
>>   drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 126 insertions(+)
>>   create mode 100644 drivers/button/button-adc.c
>>
>> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
>> index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
>> --- /dev/null
>> +++ b/drivers/button/button-adc.c
>> @@ -0,0 +1,117 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
>> + *             http://www.samsung.com
>> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <button.h>
>> +#include <dm.h>
>> +#include <dm/lists.h>
>> +#include <dm/uclass-internal.h>
>> +#include <log.h>
>> +#include <adc.h>
> Please check header order
>
> https://protect2.fireeye.com/v1/url?k=0827922c-57bcaba3-08261963-0cc47a31307c-c2add94742a708a4&q=1&e=e6ddd072-23a6-4591-9ca7-8d41aa498536&u=https%3A%2F%2Fwww.denx.de%2Fwiki%2FU-Boot%2FCodingStyle
>
>> +
>> +struct button_adc_priv {
>> +       struct udevice *adc;
>> +       int channel;
> comments
>
>> +};
>> +
>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>> +{
>> +       struct button_adc_priv *priv = dev_get_priv(dev);
>> +       unsigned int val, mask;
>> +       int ret;
>> +
>> +       ret = adc_start_channel(priv->adc, priv->channel);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = adc_channel_data(priv->adc, priv->channel, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = adc_data_mask(priv->adc, &mask);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* getting state is simplified a bit */
>> +       if (ret == 0)
>> +               return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
>> +
>> +       return ret;
>> +}
>> +
>> +static int button_adc_probe(struct udevice *dev)
>> +{
>> +       struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
>> +       struct button_adc_priv *priv = dev_get_priv(dev);
>> +       struct ofnode_phandle_args args;
>> +       int ret;
>> +
>> +       /* Ignore the top-level button node */
>> +       if (!uc_plat->label)
>> +               return 0;
>> +
>> +       ret = dev_read_phandle_with_args(dev->parent, "io-channels",
>> +                                        "#io-channel-cells", 0, 0, &args);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
>> +                                       &priv->adc);
> How about uclass_get_device_by_ofnode() ?
>
>> +       if (ret)
>> +               return ret;
>> +
>> +       priv->channel = args.args[0];
>> +
>> +       return ret;
>> +}
>> +
>> +static int button_adc_bind(struct udevice *parent)
>> +{
>> +       struct udevice *dev;
>> +       ofnode node;
>> +       int ret;
>> +
>> +       dev_for_each_subnode(node, parent) {
>> +               struct button_uc_plat *uc_plat;
>> +               const char *label;
>> +
>> +               label = ofnode_read_string(node, "label");
>> +               if (!label) {
>> +                       debug("%s: node %s has no label\n", __func__,
>> +                             ofnode_get_name(node));
>> +                       return -EINVAL;
>> +               }
>> +               ret = device_bind_driver_to_node(parent, "button_adc",
>> +                                                ofnode_get_name(node),
>> +                                                node, &dev);
>> +               if (ret)
>> +                       return ret;
>> +               uc_plat = dev_get_uclass_platdata(dev);
>> +               uc_plat->label = label;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct button_ops button_adc_ops = {
>> +       .get_state      = button_adc_get_state,
>> +};
>> +
>> +static const struct udevice_id button_adc_ids[] = {
>> +       { .compatible = "adc-keys" },
> Please add the binding file for this.
Do you want me to copy the 
Documentation/devicetree/bindings/input/adc-keys.txt file from Linux 
kernel to doc/device-tree-bindings/input or should I modify it somehow? 
(drop all the ignored properties?)
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(button_adc) = {
>> +       .name           = "button_adc",
>> +       .id             = UCLASS_BUTTON,
>> +       .of_match       = button_adc_ids,
>> +       .ops            = &button_adc_ops,
>> +       .priv_auto_alloc_size = sizeof(struct button_adc_priv),
>> +       .bind           = button_adc_bind,
>> +       .probe          = button_adc_probe,
>> +};
>> --
>> 2.17.1
>>
> Regards,
> Simon
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 4/6] button: add a simple ADC-based button driver

[Marek Szyprowski]

Hi Simon,

On 19.12.2020 03:28, Simon Glass wrote:
> On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
>> device tree node.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
>> ---
>>   drivers/button/Kconfig      |   8 +++
>>   drivers/button/Makefile     |   1 +
>>   drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 126 insertions(+)
>>   create mode 100644 drivers/button/button-adc.c
>>
>> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
>> index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
>> --- /dev/null
>> +++ b/drivers/button/button-adc.c
>> @@ -0,0 +1,117 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
>> + *             http://www.samsung.com
>> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <button.h>
>> +#include <dm.h>
>> +#include <dm/lists.h>
>> +#include <dm/uclass-internal.h>
>> +#include <log.h>
>> +#include <adc.h>
> Please check header order
>
> https://protect2.fireeye.com/v1/url?k=0827922c-57bcaba3-08261963-0cc47a31307c-c2add94742a708a4&q=1&e=e6ddd072-23a6-4591-9ca7-8d41aa498536&u=https%3A%2F%2Fwww.denx.de%2Fwiki%2FU-Boot%2FCodingStyle
>
>> +
>> +struct button_adc_priv {
>> +       struct udevice *adc;
>> +       int channel;
> comments
>
>> +};
>> +
>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
>> +{
>> +       struct button_adc_priv *priv = dev_get_priv(dev);
>> +       unsigned int val, mask;
>> +       int ret;
>> +
>> +       ret = adc_start_channel(priv->adc, priv->channel);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = adc_channel_data(priv->adc, priv->channel, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = adc_data_mask(priv->adc, &mask);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* getting state is simplified a bit */
>> +       if (ret == 0)
>> +               return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
>> +
>> +       return ret;
>> +}
>> +
>> +static int button_adc_probe(struct udevice *dev)
>> +{
>> +       struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
>> +       struct button_adc_priv *priv = dev_get_priv(dev);
>> +       struct ofnode_phandle_args args;
>> +       int ret;
>> +
>> +       /* Ignore the top-level button node */
>> +       if (!uc_plat->label)
>> +               return 0;
>> +
>> +       ret = dev_read_phandle_with_args(dev->parent, "io-channels",
>> +                                        "#io-channel-cells", 0, 0, &args);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
>> +                                       &priv->adc);
> How about uclass_get_device_by_ofnode() ?
>
>> +       if (ret)
>> +               return ret;
>> +
>> +       priv->channel = args.args[0];
>> +
>> +       return ret;
>> +}
>> +
>> +static int button_adc_bind(struct udevice *parent)
>> +{
>> +       struct udevice *dev;
>> +       ofnode node;
>> +       int ret;
>> +
>> +       dev_for_each_subnode(node, parent) {
>> +               struct button_uc_plat *uc_plat;
>> +               const char *label;
>> +
>> +               label = ofnode_read_string(node, "label");
>> +               if (!label) {
>> +                       debug("%s: node %s has no label\n", __func__,
>> +                             ofnode_get_name(node));
>> +                       return -EINVAL;
>> +               }
>> +               ret = device_bind_driver_to_node(parent, "button_adc",
>> +                                                ofnode_get_name(node),
>> +                                                node, &dev);
>> +               if (ret)
>> +                       return ret;
>> +               uc_plat = dev_get_uclass_platdata(dev);
>> +               uc_plat->label = label;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct button_ops button_adc_ops = {
>> +       .get_state      = button_adc_get_state,
>> +};
>> +
>> +static const struct udevice_id button_adc_ids[] = {
>> +       { .compatible = "adc-keys" },
> Please add the binding file for this.
Do you want me to copy the 
Documentation/devicetree/bindings/input/adc-keys.txt file from Linux 
kernel to doc/device-tree-bindings/input or should I modify it somehow? 
(drop all the ignored properties?)
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(button_adc) = {
>> +       .name           = "button_adc",
>> +       .id             = UCLASS_BUTTON,
>> +       .of_match       = button_adc_ids,
>> +       .ops            = &button_adc_ops,
>> +       .priv_auto_alloc_size = sizeof(struct button_adc_priv),
>> +       .bind           = button_adc_bind,
>> +       .probe          = button_adc_probe,
>> +};
>> --
>> 2.17.1
>>
> Regards,
> Simon
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH 4/6] button: add a simple ADC-based button driver

[Simon Glass]

Hi Marek,

On Mon, 21 Dec 2020 at 02:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Simon,
>
> On 19.12.2020 03:28, Simon Glass wrote:
> > On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
> >> device tree node.

I think it is best to copy it exactly as is.

Regards,
Simon


> >>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
> >> ---
> >>   drivers/button/Kconfig      |   8 +++
> >>   drivers/button/Makefile     |   1 +
> >>   drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 126 insertions(+)
> >>   create mode 100644 drivers/button/button-adc.c
> >>
> >> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> >> index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
> >> --- /dev/null
> >> +++ b/drivers/button/button-adc.c
> >> @@ -0,0 +1,117 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
> >> + *             http://www.samsung.com
> >> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> >> + */
> >> +
> >> +#include <common.h>
> >> +#include <button.h>
> >> +#include <dm.h>
> >> +#include <dm/lists.h>
> >> +#include <dm/uclass-internal.h>
> >> +#include <log.h>
> >> +#include <adc.h>
> > Please check header order
> >
> > https://protect2.fireeye.com/v1/url?k=0827922c-57bcaba3-08261963-0cc47a31307c-c2add94742a708a4&q=1&e=e6ddd072-23a6-4591-9ca7-8d41aa498536&u=https%3A%2F%2Fwww.denx.de%2Fwiki%2FU-Boot%2FCodingStyle
> >
> >> +
> >> +struct button_adc_priv {
> >> +       struct udevice *adc;
> >> +       int channel;
> > comments
> >
> >> +};
> >> +
> >> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >> +{
> >> +       struct button_adc_priv *priv = dev_get_priv(dev);
> >> +       unsigned int val, mask;
> >> +       int ret;
> >> +
> >> +       ret = adc_start_channel(priv->adc, priv->channel);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = adc_channel_data(priv->adc, priv->channel, &val);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = adc_data_mask(priv->adc, &mask);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       /* getting state is simplified a bit */
> >> +       if (ret == 0)
> >> +               return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int button_adc_probe(struct udevice *dev)
> >> +{
> >> +       struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> >> +       struct button_adc_priv *priv = dev_get_priv(dev);
> >> +       struct ofnode_phandle_args args;
> >> +       int ret;
> >> +
> >> +       /* Ignore the top-level button node */
> >> +       if (!uc_plat->label)
> >> +               return 0;
> >> +
> >> +       ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> >> +                                        "#io-channel-cells", 0, 0, &args);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
> >> +                                       &priv->adc);
> > How about uclass_get_device_by_ofnode() ?
> >
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       priv->channel = args.args[0];
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int button_adc_bind(struct udevice *parent)
> >> +{
> >> +       struct udevice *dev;
> >> +       ofnode node;
> >> +       int ret;
> >> +
> >> +       dev_for_each_subnode(node, parent) {
> >> +               struct button_uc_plat *uc_plat;
> >> +               const char *label;
> >> +
> >> +               label = ofnode_read_string(node, "label");
> >> +               if (!label) {
> >> +                       debug("%s: node %s has no label\n", __func__,
> >> +                             ofnode_get_name(node));
> >> +                       return -EINVAL;
> >> +               }
> >> +               ret = device_bind_driver_to_node(parent, "button_adc",
> >> +                                                ofnode_get_name(node),
> >> +                                                node, &dev);
> >> +               if (ret)
> >> +                       return ret;
> >> +               uc_plat = dev_get_uclass_platdata(dev);
> >> +               uc_plat->label = label;
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct button_ops button_adc_ops = {
> >> +       .get_state      = button_adc_get_state,
> >> +};
> >> +
> >> +static const struct udevice_id button_adc_ids[] = {
> >> +       { .compatible = "adc-keys" },
> > Please add the binding file for this.
> Do you want me to copy the
> Documentation/devicetree/bindings/input/adc-keys.txt file from Linux
> kernel to doc/device-tree-bindings/input or should I modify it somehow?
> (drop all the ignored properties?)
> >> +       { }
> >> +};
> >> +
> >> +U_BOOT_DRIVER(button_adc) = {
> >> +       .name           = "button_adc",
> >> +       .id             = UCLASS_BUTTON,
> >> +       .of_match       = button_adc_ids,
> >> +       .ops            = &button_adc_ops,
> >> +       .priv_auto_alloc_size = sizeof(struct button_adc_priv),
> >> +       .bind           = button_adc_bind,
> >> +       .probe          = button_adc_probe,
> >> +};
> >> --
> >> 2.17.1
> >>
> > Regards,
> > Simon
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Re: [PATCH 4/6] button: add a simple ADC-based button driver

[Simon Glass]

Hi Marek,

On Mon, 21 Dec 2020 at 02:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Simon,
>
> On 19.12.2020 03:28, Simon Glass wrote:
> > On Mon, 14 Dec 2020 at 04:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> Add a simple ADC-based button driver. This driver binds to the 'adc-keys'
> >> device tree node.

I think it is best to copy it exactly as is.

Regards,
Simon


> >>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> Change-Id: I6da7101eff3ce53766d899f49f5839d728d52fb3
> >> ---
> >>   drivers/button/Kconfig      |   8 +++
> >>   drivers/button/Makefile     |   1 +
> >>   drivers/button/button-adc.c | 117 ++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 126 insertions(+)
> >>   create mode 100644 drivers/button/button-adc.c
> >>
> >> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> >> index 6b3ec7e55de..283367f2bd3 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 ADC lines. 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 fcc10ebe8db..bbd18af1494 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 00000000000..086c676c02a
> >> --- /dev/null
> >> +++ b/drivers/button/button-adc.c
> >> @@ -0,0 +1,117 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
> >> + *             http://www.samsung.com
> >> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> >> + */
> >> +
> >> +#include <common.h>
> >> +#include <button.h>
> >> +#include <dm.h>
> >> +#include <dm/lists.h>
> >> +#include <dm/uclass-internal.h>
> >> +#include <log.h>
> >> +#include <adc.h>
> > Please check header order
> >
> > https://protect2.fireeye.com/v1/url?k=0827922c-57bcaba3-08261963-0cc47a31307c-c2add94742a708a4&q=1&e=e6ddd072-23a6-4591-9ca7-8d41aa498536&u=https%3A%2F%2Fwww.denx.de%2Fwiki%2FU-Boot%2FCodingStyle
> >
> >> +
> >> +struct button_adc_priv {
> >> +       struct udevice *adc;
> >> +       int channel;
> > comments
> >
> >> +};
> >> +
> >> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >> +{
> >> +       struct button_adc_priv *priv = dev_get_priv(dev);
> >> +       unsigned int val, mask;
> >> +       int ret;
> >> +
> >> +       ret = adc_start_channel(priv->adc, priv->channel);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = adc_channel_data(priv->adc, priv->channel, &val);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = adc_data_mask(priv->adc, &mask);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       /* getting state is simplified a bit */
> >> +       if (ret == 0)
> >> +               return (val < mask / 2) ? BUTTON_ON : BUTTON_OFF;
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int button_adc_probe(struct udevice *dev)
> >> +{
> >> +       struct button_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> >> +       struct button_adc_priv *priv = dev_get_priv(dev);
> >> +       struct ofnode_phandle_args args;
> >> +       int ret;
> >> +
> >> +       /* Ignore the top-level button node */
> >> +       if (!uc_plat->label)
> >> +               return 0;
> >> +
> >> +       ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> >> +                                        "#io-channel-cells", 0, 0, &args);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       ret = uclass_get_device_by_name(UCLASS_ADC, ofnode_get_name(args.node),
> >> +                                       &priv->adc);
> > How about uclass_get_device_by_ofnode() ?
> >
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       priv->channel = args.args[0];
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int button_adc_bind(struct udevice *parent)
> >> +{
> >> +       struct udevice *dev;
> >> +       ofnode node;
> >> +       int ret;
> >> +
> >> +       dev_for_each_subnode(node, parent) {
> >> +               struct button_uc_plat *uc_plat;
> >> +               const char *label;
> >> +
> >> +               label = ofnode_read_string(node, "label");
> >> +               if (!label) {
> >> +                       debug("%s: node %s has no label\n", __func__,
> >> +                             ofnode_get_name(node));
> >> +                       return -EINVAL;
> >> +               }
> >> +               ret = device_bind_driver_to_node(parent, "button_adc",
> >> +                                                ofnode_get_name(node),
> >> +                                                node, &dev);
> >> +               if (ret)
> >> +                       return ret;
> >> +               uc_plat = dev_get_uclass_platdata(dev);
> >> +               uc_plat->label = label;
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct button_ops button_adc_ops = {
> >> +       .get_state      = button_adc_get_state,
> >> +};
> >> +
> >> +static const struct udevice_id button_adc_ids[] = {
> >> +       { .compatible = "adc-keys" },
> > Please add the binding file for this.
> Do you want me to copy the
> Documentation/devicetree/bindings/input/adc-keys.txt file from Linux
> kernel to doc/device-tree-bindings/input or should I modify it somehow?
> (drop all the ignored properties?)
> >> +       { }
> >> +};
> >> +
> >> +U_BOOT_DRIVER(button_adc) = {
> >> +       .name           = "button_adc",
> >> +       .id             = UCLASS_BUTTON,
> >> +       .of_match       = button_adc_ids,
> >> +       .ops            = &button_adc_ops,
> >> +       .priv_auto_alloc_size = sizeof(struct button_adc_priv),
> >> +       .bind           = button_adc_bind,
> >> +       .probe          = button_adc_probe,
> >> +};
> >> --
> >> 2.17.1
> >>
> > Regards,
> > Simon
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>