From: Marek Szyprowski <m.szyprowski@samsung.com> To: u-boot@lists.denx.de Subject: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver Date: Tue, 26 Jan 2021 12:25:04 +0100 [thread overview] Message-ID: <1d5b1718-9b28-61dd-8137-3a50d9ec2e6e@samsung.com> (raw) In-Reply-To: <e2bf2621-a9a4-9ba6-ccbb-0a6b35c8d8bf@gmx.de> Hi Heinrich, On 26.01.2021 12:10, Heinrich Schuchardt wrote: > On 1/26/21 10:50 AM, Marek Szyprowski wrote: >> Add a simple Analog to Digital Converter device based button driver. >> This >> driver binds to the 'adc-keys' device tree node. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> ? drivers/button/Kconfig????? |?? 8 ++ >> ? drivers/button/Makefile???? |?? 1 + >> ? drivers/button/button-adc.c | 156 ++++++++++++++++++++++++++++++++++++ >> ? 3 files changed, 165 insertions(+) >> ? create mode 100644 drivers/button/button-adc.c >> >> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig >> index 6b3ec7e55d..6db3c5e93a 100644 >> --- a/drivers/button/Kconfig >> +++ b/drivers/button/Kconfig >> @@ -9,6 +9,14 @@ config BUTTON >> ??????? can provide access to board-specific buttons. Use of the >> device tree >> ??????? for configuration is encouraged. >> >> +config BUTTON_ADC >> +??? bool "Button adc" >> +??? depends on BUTTON >> +??? help >> +????? Enable support for buttons which are connected to Analog to >> Digital >> +????? Converter device. The ADC driver must use driver model. >> Buttons are >> +????? configured using the device tree. >> + >> ? config BUTTON_GPIO >> ????? bool "Button gpio" >> ????? depends on BUTTON >> diff --git a/drivers/button/Makefile b/drivers/button/Makefile >> index fcc10ebe8d..bbd18af149 100644 >> --- a/drivers/button/Makefile >> +++ b/drivers/button/Makefile >> @@ -3,4 +3,5 @@ >> ? # Copyright (C) 2020 Philippe Reynes <philippe.reynes@softathome.com> >> >> ? obj-$(CONFIG_BUTTON) += button-uclass.o >> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o >> ? obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o >> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c >> new file mode 100644 >> index 0000000000..1901d59a0e >> --- /dev/null >> +++ b/drivers/button/button-adc.c >> @@ -0,0 +1,156 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2021 Samsung Electronics Co., Ltd. >> + *??????? http://www.samsung.com >> + * Author: Marek Szyprowski <m.szyprowski@samsung.com> >> + */ >> + >> +#include <common.h> >> +#include <adc.h> >> +#include <button.h> >> +#include <log.h> >> +#include <dm.h> >> +#include <dm/lists.h> >> +#include <dm/of_access.h> >> +#include <dm/uclass-internal.h> >> + >> +/** >> + * struct button_adc_priv - private data for button-adc driver. >> + * >> + * @adc: Analog to Digital Converter device to which button is >> connected. >> + * @channel: channel of the ADC device to probe the button state. >> + * @min: minimal raw ADC sample value to consider button as pressed. >> + * @max: maximal raw ADC sample value to consider button as pressed. >> + */ >> +struct button_adc_priv { >> +??? struct udevice *adc; >> +??? int channel; >> +??? int min; >> +??? int max; >> +}; >> + >> +static enum button_state_t button_adc_get_state(struct udevice *dev) >> +{ >> +??? struct button_adc_priv *priv = dev_get_priv(dev); >> +??? unsigned int val; >> +??? int ret; >> + >> +??? ret = adc_start_channel(priv->adc, priv->channel); >> +??? if (ret) >> +??????? return ret; >> + >> +??? ret = adc_channel_data(priv->adc, priv->channel, &val); >> +??? if (ret) >> +??????? return ret; >> + >> +??? if (ret == 0) >> +??????? return (val >= priv->min && val < priv->max) ? >> +??????????? BUTTON_ON : BUTTON_OFF; >> + >> +??? return ret; >> +} >> + >> +static int button_adc_probe(struct udevice *dev) >> +{ >> +??? struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev); >> +??? struct button_adc_priv *priv = dev_get_priv(dev); >> +??? struct ofnode_phandle_args args; >> +??? u32 treshold, up_treshold, t; >> +??? unsigned int mask; >> +??? ofnode node; >> +??? int ret, vdd; >> + >> +??? /* Ignore the top-level button node */ >> +??? if (!uc_plat->label) >> +??????? return 0; >> + >> +??? ret = dev_read_phandle_with_args(dev->parent, "io-channels", >> +???????????????????? "#io-channel-cells", 0, 0, &args); >> +??? if (ret) >> +??????? return ret; >> + >> +??? ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, >> &priv->adc); >> +??? if (ret) >> +??????? return ret; >> + >> +??? ret = ofnode_read_u32(dev_ofnode(dev->parent), >> +????????????????? "keyup-threshold-microvolt", &up_treshold); >> +??? if (ret) >> +??????? return ret; >> + >> +??? ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt", >> +????????????????? &treshold); >> +??? if (ret) >> +??????? return ret; >> + >> +??? dev_for_each_subnode(node, dev->parent) { >> +??????? ret = ofnode_read_u32(dev_ofnode(dev), >> +????????????????????? "press-threshold-microvolt", &t); >> +??????? if (ret) >> +??????????? return ret; >> + >> +??????? if (t > treshold) >> +??????????? up_treshold = t; > > Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes > that one virtual key is created per sub-node. > > If I read your code correctly, this is not what you are implementing. > Instead you only define a single key per adc-keys node. > > Why are your deviating from the bindings document? No I don't. button_adc_bind() binds to the root node with 'adc-keys' compatible, while the dev_for_each_subnode() loop instantiates driver for each subnode, so the button_adc_probe() is called for each defined key. I've copied this pattern from gpio-keys driver. >> ... Here is the related code: >> +static int button_adc_bind(struct udevice *parent) >> +{ >> +??? struct udevice *dev; >> +??? ofnode node; >> +??? int ret; >> + >> +??? dev_for_each_subnode(node, parent) { >> +??????? struct button_uc_plat *uc_plat; >> +??????? const char *label; >> + >> +??????? label = ofnode_read_string(node, "label"); >> +??????? if (!label) { >> +??????????? debug("%s: node %s has no label\n", __func__, >> +????????????????? ofnode_get_name(node)); >> +??????????? return -EINVAL; >> +??????? } >> +??????? ret = device_bind_driver_to_node(parent, "button_adc", >> +???????????????????????? ofnode_get_name(node), >> +???????????????????????? node, &dev); >> +??????? if (ret) >> +??????????? return ret; >> +??????? uc_plat = dev_get_uclass_plat(dev); >> +??????? uc_plat->label = label; >> +??? } >> + >> +??? return 0; >> +} >> + >> +static const struct button_ops button_adc_ops = { >> +??? .get_state??? = button_adc_get_state, >> +}; >> + >> +static const struct udevice_id button_adc_ids[] = { >> +??? { .compatible = "adc-keys" }, >> +??? { } >> +}; >> + >> +U_BOOT_DRIVER(button_adc) = { >> +??? .name??????? = "button_adc", >> +??? .id??????? = UCLASS_BUTTON, >> +??? .of_match??? = button_adc_ids, >> +??? .ops??????? = &button_adc_ops, >> +??? .priv_auto??? = sizeof(struct button_adc_priv), >> +??? .bind??????? = button_adc_bind, >> +??? .probe??????? = button_adc_probe, >> +}; >> > > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
WARNING: multiple messages have this Message-ID (diff)
From: Marek Szyprowski <m.szyprowski@samsung.com> To: Heinrich Schuchardt <xypron.glpk@gmx.de>, u-boot@lists.denx.de, u-boot-amlogic@groups.io Cc: Neil Armstrong <narmstrong@baylibre.com>, Lukasz Majewski <lukma@denx.de>, Philippe Reynes <philippe.reynes@softathome.com>, Simon Glass <sjg@chromium.org>, Jaehoon Chung <jh80.chung@samsung.com>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Subject: Re: [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver Date: Tue, 26 Jan 2021 12:25:04 +0100 [thread overview] Message-ID: <1d5b1718-9b28-61dd-8137-3a50d9ec2e6e@samsung.com> (raw) In-Reply-To: <e2bf2621-a9a4-9ba6-ccbb-0a6b35c8d8bf@gmx.de> Hi Heinrich, On 26.01.2021 12:10, Heinrich Schuchardt wrote: > On 1/26/21 10:50 AM, Marek Szyprowski wrote: >> Add a simple Analog to Digital Converter device based button driver. >> This >> driver binds to the 'adc-keys' device tree node. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/button/Kconfig | 8 ++ >> drivers/button/Makefile | 1 + >> drivers/button/button-adc.c | 156 ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 165 insertions(+) >> create mode 100644 drivers/button/button-adc.c >> >> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig >> index 6b3ec7e55d..6db3c5e93a 100644 >> --- a/drivers/button/Kconfig >> +++ b/drivers/button/Kconfig >> @@ -9,6 +9,14 @@ config BUTTON >> can provide access to board-specific buttons. Use of the >> device tree >> for configuration is encouraged. >> >> +config BUTTON_ADC >> + bool "Button adc" >> + depends on BUTTON >> + help >> + Enable support for buttons which are connected to Analog to >> Digital >> + Converter device. The ADC driver must use driver model. >> Buttons are >> + configured using the device tree. >> + >> config BUTTON_GPIO >> bool "Button gpio" >> depends on BUTTON >> diff --git a/drivers/button/Makefile b/drivers/button/Makefile >> index fcc10ebe8d..bbd18af149 100644 >> --- a/drivers/button/Makefile >> +++ b/drivers/button/Makefile >> @@ -3,4 +3,5 @@ >> # Copyright (C) 2020 Philippe Reynes <philippe.reynes@softathome.com> >> >> obj-$(CONFIG_BUTTON) += button-uclass.o >> +obj-$(CONFIG_BUTTON_ADC) += button-adc.o >> obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o >> diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c >> new file mode 100644 >> index 0000000000..1901d59a0e >> --- /dev/null >> +++ b/drivers/button/button-adc.c >> @@ -0,0 +1,156 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2021 Samsung Electronics Co., Ltd. >> + * http://www.samsung.com >> + * Author: Marek Szyprowski <m.szyprowski@samsung.com> >> + */ >> + >> +#include <common.h> >> +#include <adc.h> >> +#include <button.h> >> +#include <log.h> >> +#include <dm.h> >> +#include <dm/lists.h> >> +#include <dm/of_access.h> >> +#include <dm/uclass-internal.h> >> + >> +/** >> + * struct button_adc_priv - private data for button-adc driver. >> + * >> + * @adc: Analog to Digital Converter device to which button is >> connected. >> + * @channel: channel of the ADC device to probe the button state. >> + * @min: minimal raw ADC sample value to consider button as pressed. >> + * @max: maximal raw ADC sample value to consider button as pressed. >> + */ >> +struct button_adc_priv { >> + struct udevice *adc; >> + int channel; >> + int min; >> + int max; >> +}; >> + >> +static enum button_state_t button_adc_get_state(struct udevice *dev) >> +{ >> + struct button_adc_priv *priv = dev_get_priv(dev); >> + unsigned int val; >> + int ret; >> + >> + ret = adc_start_channel(priv->adc, priv->channel); >> + if (ret) >> + return ret; >> + >> + ret = adc_channel_data(priv->adc, priv->channel, &val); >> + if (ret) >> + return ret; >> + >> + if (ret == 0) >> + return (val >= priv->min && val < priv->max) ? >> + BUTTON_ON : BUTTON_OFF; >> + >> + return ret; >> +} >> + >> +static int button_adc_probe(struct udevice *dev) >> +{ >> + struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev); >> + struct button_adc_priv *priv = dev_get_priv(dev); >> + struct ofnode_phandle_args args; >> + u32 treshold, up_treshold, t; >> + unsigned int mask; >> + ofnode node; >> + int ret, vdd; >> + >> + /* Ignore the top-level button node */ >> + if (!uc_plat->label) >> + return 0; >> + >> + ret = dev_read_phandle_with_args(dev->parent, "io-channels", >> + "#io-channel-cells", 0, 0, &args); >> + if (ret) >> + return ret; >> + >> + ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node, >> &priv->adc); >> + if (ret) >> + return ret; >> + >> + ret = ofnode_read_u32(dev_ofnode(dev->parent), >> + "keyup-threshold-microvolt", &up_treshold); >> + if (ret) >> + return ret; >> + >> + ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt", >> + &treshold); >> + if (ret) >> + return ret; >> + >> + dev_for_each_subnode(node, dev->parent) { >> + ret = ofnode_read_u32(dev_ofnode(dev), >> + "press-threshold-microvolt", &t); >> + if (ret) >> + return ret; >> + >> + if (t > treshold) >> + up_treshold = t; > > Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes > that one virtual key is created per sub-node. > > If I read your code correctly, this is not what you are implementing. > Instead you only define a single key per adc-keys node. > > Why are your deviating from the bindings document? No I don't. button_adc_bind() binds to the root node with 'adc-keys' compatible, while the dev_for_each_subnode() loop instantiates driver for each subnode, so the button_adc_probe() is called for each defined key. I've copied this pattern from gpio-keys driver. >> ... Here is the related code: >> +static int button_adc_bind(struct udevice *parent) >> +{ >> + struct udevice *dev; >> + ofnode node; >> + int ret; >> + >> + dev_for_each_subnode(node, parent) { >> + struct button_uc_plat *uc_plat; >> + const char *label; >> + >> + label = ofnode_read_string(node, "label"); >> + if (!label) { >> + debug("%s: node %s has no label\n", __func__, >> + ofnode_get_name(node)); >> + return -EINVAL; >> + } >> + ret = device_bind_driver_to_node(parent, "button_adc", >> + ofnode_get_name(node), >> + node, &dev); >> + if (ret) >> + return ret; >> + uc_plat = dev_get_uclass_plat(dev); >> + uc_plat->label = label; >> + } >> + >> + return 0; >> +} >> + >> +static const struct button_ops button_adc_ops = { >> + .get_state = button_adc_get_state, >> +}; >> + >> +static const struct udevice_id button_adc_ids[] = { >> + { .compatible = "adc-keys" }, >> + { } >> +}; >> + >> +U_BOOT_DRIVER(button_adc) = { >> + .name = "button_adc", >> + .id = UCLASS_BUTTON, >> + .of_match = button_adc_ids, >> + .ops = &button_adc_ops, >> + .priv_auto = sizeof(struct button_adc_priv), >> + .bind = button_adc_bind, >> + .probe = button_adc_probe, >> +}; >> > > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
next prev parent reply other threads:[~2021-01-26 11:25 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20210126095111eucas1p21e10e8470c39d555080f68fa36957c72@eucas1p2.samsung.com> 2021-01-26 9:50 ` [PATCH v5 0/4] VIM3: add support for checking 'Function' button state Marek Szyprowski 2021-01-26 9:50 ` Marek Szyprowski [not found] ` <CGME20210126095112eucas1p2d9c00a9d509ac482a08dca1cf35b22de@eucas1p2.samsung.com> 2021-01-26 9:50 ` [PATCH v5 1/4] dt-bindings: input: adc-keys bindings documentation Marek Szyprowski 2021-01-26 9:50 ` Marek Szyprowski 2021-02-01 20:38 ` Simon Glass [not found] ` <CGME20210126095112eucas1p1af8ac76eaede4baf2b0cfeac561a6a06@eucas1p1.samsung.com> 2021-01-26 9:50 ` [PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver Marek Szyprowski 2021-01-26 9:50 ` Marek Szyprowski 2021-01-26 11:10 ` Heinrich Schuchardt 2021-01-26 11:10 ` Heinrich Schuchardt 2021-01-26 11:25 ` Marek Szyprowski [this message] 2021-01-26 11:25 ` Marek Szyprowski 2021-01-26 12:58 ` Heinrich Schuchardt 2021-01-26 12:58 ` Heinrich Schuchardt 2021-02-01 20:38 ` Simon Glass 2021-02-04 10:36 ` Marek Szyprowski 2021-02-06 16:21 ` Simon Glass 2021-02-08 16:10 ` Marek Szyprowski 2021-02-08 17:08 ` Simon Glass 2021-02-08 17:56 ` Heinrich Schuchardt 2021-02-08 22:13 ` Heinrich Schuchardt 2021-02-09 4:28 ` Simon Glass 2021-02-09 8:43 ` Marek Szyprowski 2021-02-10 5:10 ` Simon Glass [not found] ` <CGME20210126095113eucas1p1cbd41e98d0d5f454fe45a4c5e4bb5e2f@eucas1p1.samsung.com> 2021-01-26 9:50 ` [PATCH v5 3/4] adc: meson-saradc: add support for getting reference voltage value Marek Szyprowski 2021-01-26 9:50 ` Marek Szyprowski 2021-02-01 20:38 ` Simon Glass [not found] ` <CGME20210126095113eucas1p1154fc4a8742f9a969a5a19114fb79000@eucas1p1.samsung.com> 2021-01-26 9:50 ` [PATCH v5 4/4] configs: khadas-vim3(l): enable Function button support Marek Szyprowski 2021-01-26 9:50 ` Marek Szyprowski 2021-02-01 20:38 ` Simon Glass
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1d5b1718-9b28-61dd-8137-3a50d9ec2e6e@samsung.com \ --to=m.szyprowski@samsung.com \ --cc=u-boot@lists.denx.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.