Hi Dzmitry,
On Thu, 19 Jan 2023 at 10:19, Dzmitry Sankouski wrote:
>
> Linux event code may be used in input devices, using buttons.
Do you mean 'must be used' ?
>
> Signed-off-by: Dzmitry Sankouski
> ---
> 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", >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
