[PATCH v2 3/4] dm: button: add support for linux_code in button-gpio.c driver

[Simon Glass]

Hi Dzmitry,

On Thu, 19 Jan 2023 at 10:19, Dzmitry Sankouski <dsankouski at gmail.com> wrote:
>
> Linux event code may be used in input devices, using buttons.

Do you mean 'must be used' ?

>
> Signed-off-by: Dzmitry Sankouski <dsankouski at gmail.com>
> ---
> Changes for v2:
> - fail, if linux,code not found
>
>  drivers/button/button-gpio.c   | 16 +++++++++++++++-
>  drivers/button/button-uclass.c | 10 ++++++++++
>  include/button.h               | 16 ++++++++++++++++
>  3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/button/button-gpio.c b/drivers/button/button-gpio.c
> index dbb000622c..cfed9f17d1 100644
> --- a/drivers/button/button-gpio.c
> +++ b/drivers/button/button-gpio.c
> @@ -13,6 +13,7 @@
>
>  struct button_gpio_priv {
>         struct gpio_desc gpio;
> +       u32 linux_code;
>  };
>
>  static enum button_state_t button_gpio_get_state(struct udevice *dev)
> @@ -29,6 +30,17 @@ static enum button_state_t button_gpio_get_state(struct udevice *dev)
>         return ret ? BUTTON_ON : BUTTON_OFF;
>  }
>
> +static u32 button_gpio_get_code(struct udevice *dev)

Can you use int instead? There is no need to use smaller types on
64-bit machines, and 'int' is more commonly used for DM functions.

> +{
> +       struct button_gpio_priv *priv = dev_get_priv(dev);
> +       u32 code = priv->linux_code;
> +
> +       if (!code)
> +               return 0;

Is that an error? If so, perhaps return -ENODATA or something like that?

> +
> +       return code;
> +}
> +
>  static int button_gpio_probe(struct udevice *dev)
>  {
>         struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> @@ -43,7 +55,8 @@ static int button_gpio_probe(struct udevice *dev)
>         if (ret)
>                 return ret;
>
> -       return 0;
> +       ret = dev_read_u32(dev, "linux,code", &priv->linux_code);

blank line here

> +       return ret;

Hmm, so it will not probe if there is no code?

>  }
>
>  static int button_gpio_remove(struct udevice *dev)
> @@ -92,6 +105,7 @@ static int button_gpio_bind(struct udevice *parent)
>
>  static const struct button_ops button_gpio_ops = {
>         .get_state      = button_gpio_get_state,
> +       .get_code       = button_gpio_get_code,
>  };
>
>  static const struct udevice_id button_gpio_ids[] = {
> diff --git a/drivers/button/button-uclass.c b/drivers/button/button-uclass.c
> index e33ed7d01d..6d0c6f69c5 100644
> --- a/drivers/button/button-uclass.c
> +++ b/drivers/button/button-uclass.c
> @@ -38,6 +38,16 @@ enum button_state_t button_get_state(struct udevice *dev)
>         return ops->get_state(dev);
>  }
>
> +u32 button_get_code(struct udevice *dev)
> +{
> +       struct button_ops *ops = button_get_ops(dev);
> +
> +       if (!ops->get_code)
> +               return -ENOSYS;
> +
> +       return ops->get_code(dev);
> +}
> +
>  UCLASS_DRIVER(button) = {
>         .id             = UCLASS_BUTTON,
>         .name           = "button",
> diff --git a/include/button.h b/include/button.h
> index 96e6b1901f..27af4a6a1a 100644
> --- a/include/button.h
> +++ b/include/button.h
> @@ -37,6 +37,14 @@ struct button_ops {
>          * @return button state button_state_t, or -ve on error
>          */
>         enum button_state_t (*get_state)(struct udevice *dev);
> +
> +       /**
> +        * get_code() - get linux event code of a button
> +        *
> +        * @dev:        button device to change
> +        * @return button code, or -ve on error

Could mention -ENODATA here if that is what you use

> +        */
> +       u32 (*get_code)(struct udevice *dev);

Please can you add a test for this? See test/dm/button.c

>  };
>
>  #define button_get_ops(dev)    ((struct button_ops *)(dev)->driver->ops)
> @@ -58,4 +66,12 @@ int button_get_by_label(const char *label, struct udevice **devp);
>   */
>  enum button_state_t button_get_state(struct udevice *dev);
>
> +/**
> + * button_get_code() - get linux event code of a button
> + *
> + * @dev:       button device to change
> + * @return button code, or -ve on error
> + */
> +u32 button_get_code(struct udevice *dev);
> +
>  #endif
> --
> 2.30.2
>

Regards,
Simon