Hi Dzmitry, On Thu, 19 Jan 2023 at 10:19, Dzmitry Sankouski <dsankou...@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 <dsankou...@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