[PATCH v5 2/4] button: add a simple Analog to Digital Converter device based button driver
Simon Glass
sjg at chromium.org
Sat Feb 6 17:21:35 CET 2021
Hi Marek,
On Thu, 4 Feb 2021 at 03:36, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>
> Hi Simon,
>
> On 01.02.2021 21:38, Simon Glass wrote:
> > On Tue, 26 Jan 2021 at 06:03, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >> On 26.01.21 12:25, Marek Szyprowski wrote:
> >>> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
> >>>> On 1/26/21 10:50 AM, Marek Szyprowski wrote:
> >>>>> Add a simple Analog to Digital Converter device based button driver.
> >>>>> This
> >>>>> driver binds to the 'adc-keys' device tree node.
> >>>>>
> >>>>> Signed-off-by: Marek Szyprowski <m.szyprowski at 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 at 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 at samsung.com>
> >>>>> + */
> >>>>> +
> >>>>> +#include <common.h>
> >>>>> +#include <adc.h>
> >>>>> +#include <button.h>
> >>>>> +#include <log.h>
> >>>>> +#include <dm.h>
> >>>>> +#include <dm/lists.h>
> >>>>> +#include <dm/of_access.h>
> >>>>> +#include <dm/uclass-internal.h>
> >>>>> +
> >>>>> +/**
> >>>>> + * struct button_adc_priv - private data for button-adc driver.
> >>>>> + *
> >>>>> + * @adc: Analog to Digital Converter device to which button is
> >>>>> connected.
> >>>>> + * @channel: channel of the ADC device to probe the button state.
> >>>>> + * @min: minimal raw ADC sample value to consider button as pressed.
> >>>>> + * @max: maximal raw ADC sample value to consider button as pressed.
> >>>>> + */
> >>>>> +struct button_adc_priv {
> >>>>> + struct udevice *adc;
> >>>>> + int channel;
> >>>>> + int min;
> >>>>> + int max;
> >>>>> +};
> >>>>> +
> >>>>> +static enum button_state_t button_adc_get_state(struct udevice *dev)
> >>>>> +{
> >>>>> + struct button_adc_priv *priv = dev_get_priv(dev);
> >>>>> + unsigned int val;
> >>>>> + int ret;
> >>>>> +
> >>>>> + ret = adc_start_channel(priv->adc, priv->channel);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + ret = adc_channel_data(priv->adc, priv->channel, &val);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + if (ret == 0)
> >>>>> + return (val >= priv->min && val < priv->max) ?
> >>>>> + BUTTON_ON : BUTTON_OFF;
> >>>>> +
> >>>>> + return ret;
> >>>>> +}
> >>>>> +
> >>>>> +static int button_adc_probe(struct udevice *dev)
> >>>>> +{
> >>>>> + struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >>>>> + struct button_adc_priv *priv = dev_get_priv(dev);
> >>>>> + struct ofnode_phandle_args args;
> >>>>> + u32 treshold, up_treshold, t;
> >>>>> + unsigned int mask;
> >>>>> + ofnode node;
> >>>>> + int ret, vdd;
> >>>>> +
> >>>>> + /* Ignore the top-level button node */
> >>>>> + if (!uc_plat->label)
> >>>>> + return 0;
> >>>>> +
> >>>>> + ret = dev_read_phandle_with_args(dev->parent, "io-channels",
> >>>>> + "#io-channel-cells", 0, 0, &args);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + ret = uclass_get_device_by_ofnode(UCLASS_ADC, args.node,
> >>>>> &priv->adc);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + ret = ofnode_read_u32(dev_ofnode(dev->parent),
> >>>>> + "keyup-threshold-microvolt", &up_treshold);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt",
> >>>>> + &treshold);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + dev_for_each_subnode(node, dev->parent) {
> >>>>> + ret = ofnode_read_u32(dev_ofnode(dev),
> >>>>> + "press-threshold-microvolt", &t);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>>> +
> >>>>> + if (t > treshold)
> >>>>> + up_treshold = t;
> >>>> Linux' Documentation/devicetree/bindings/input/adc-keys.txt describes
> >>>> that one virtual key is created per sub-node.
> >>>>
> >>>> If I read your code correctly, this is not what you are implementing.
> >>>> Instead you only define a single key per adc-keys node.
> >>>>
> >>>> Why are your deviating from the bindings document?
> >>> No I don't. button_adc_bind() binds to the root node with 'adc-keys'
> >>> compatible, while the dev_for_each_subnode() loop instantiates driver
> >>> for each subnode, so the button_adc_probe() is called for each defined
> >>> key. I've copied this pattern from gpio-keys driver.
> >>>
> >>> >> ...
> >>>
> >>> Here is the related code:
> >> Thanks for pointing this out.
> >>
> >> To really test the driver we would need an emulated device on the
> >> sandbox where we can set the voltage and see which button is activated.
> >>
> >> I assume this can be added to test/dm/adc.c.
> > Yes please!
>
> Could you give me a bit more hints or point where to start? I've tried
> to build sandbox, but it fails for v2021.01 release (I've did make
> sandbox_defconfig && make all). I assume I would need to add adc and
> adc-keys devices to some sandbox dts and some code triggering and
> checking the key values, but that's all I know now.
Well you do need to be able to build sandbox or you will get
nowhere...what error did you get? Once we understand what went wrong
we can update the docs. Maybe it is missing a dependency.
BTW for testing there is a series with more docs at
u-boot-dm/test-working that might be useful.
You can add your node to sandbox.dtsi and then run U-Boot with -T to
get the test devicetree. Something like:
$ ./u-boot -T
(U-Boot starts)
> ut dm adc_multi_channel_shot
(test runs)
You just need to probe your device and then button_get_state() on it.
BTW I think all of the code in your probe() method should move to
of_to_plat(). It has nothing to do with probing the device and is all
about reading from the devicetree.
Regards,
Simon
More information about the U-Boot
mailing list