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

[Heinrich Schuchardt]

On 26.01.21 12:25, Marek Szyprowski wrote:
> Hi Heinrich,
>
> On 26.01.2021 12:10, Heinrich Schuchardt wrote:
>> On 1/26/21 10:50 AM, Marek Szyprowski wrote:
>>> Add a simple Analog to Digital Converter device based button driver.
>>> This
>>> driver binds to the 'adc-keys' device tree node.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski 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.

Best regards

Heinrich

>
>>> +static int button_adc_bind(struct udevice *parent)
>>> +{
>>> +    struct udevice *dev;
>>> +    ofnode node;
>>> +    int ret;
>>> +
>>> +    dev_for_each_subnode(node, parent) {
>>> +        struct button_uc_plat *uc_plat;
>>> +        const char *label;
>>> +
>>> +        label = ofnode_read_string(node, "label");
>>> +        if (!label) {
>>> +            debug("%s: node %s has no label\n", __func__,
>>> +                  ofnode_get_name(node));
>>> +            return -EINVAL;
>>> +        }
>>> +        ret = device_bind_driver_to_node(parent, "button_adc",
>>> +                         ofnode_get_name(node),
>>> +                         node, &dev);
>>> +        if (ret)
>>> +            return ret;
>>> +        uc_plat = dev_get_uclass_plat(dev);
>>> +        uc_plat->label = label;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct button_ops button_adc_ops = {
>>> +    .get_state    = button_adc_get_state,
>>> +};
>>> +
>>> +static const struct udevice_id button_adc_ids[] = {
>>> +    { .compatible = "adc-keys" },
>>> +    { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(button_adc) = {
>>> +    .name        = "button_adc",
>>> +    .id        = UCLASS_BUTTON,
>>> +    .of_match    = button_adc_ids,
>>> +    .ops        = &button_adc_ops,
>>> +    .priv_auto    = sizeof(struct button_adc_priv),
>>> +    .bind        = button_adc_bind,
>>> +    .probe        = button_adc_probe,
>>> +};
>>>
>>
>>
> Best regards
>