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

[Dzmitry Sankouski]

dev_read_u32 will fail, if linux,code is not found.
We shouldn't fail here, as linux,code is optional, so maybe dev_read_u32_default
with 0 default value, instead of negative error code?

ср, 11 янв. 2023 г. в 18:48, Quentin Schulz
<quentin.schulz at theobroma-systems.com>:
>
> Hi Dzmitry,
>
> On 1/11/23 11:19, Dzmitry Sankouski wrote:
> > Linux event code may be used in input devices, using buttons.
> >
> > Signed-off-by: Dzmitry Sankouski <dsankouski at gmail.com>
> > ---
> >   drivers/button/button-gpio.c   | 20 ++++++++++++++++++++
> >   drivers/button/button-uclass.c | 10 ++++++++++
> >   include/button.h               | 16 ++++++++++++++++
> >   3 files changed, 46 insertions(+)
> >
> > diff --git a/drivers/button/button-gpio.c b/drivers/button/button-gpio.c
> > index dbb000622c..e6eff5c1da 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,10 +30,22 @@ 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)
> > +{
> > +     struct button_gpio_priv *priv = dev_get_priv(dev);
> > +     u32 code = priv->linux_code;
> > +
> > +     if (!code)
> > +             return 0;
> > +
> > +     return code;
> > +}
> > +
> >   static int button_gpio_probe(struct udevice *dev)
> >   {
> >       struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> >       struct button_gpio_priv *priv = dev_get_priv(dev);
> > +     u32 linux_code;
> >       int ret;
> >
> >       /* Ignore the top-level button node */
> > @@ -43,6 +56,12 @@ static int button_gpio_probe(struct udevice *dev)
> >       if (ret)
> >               return ret;
> >
> > +     linux_code = dev_read_u32_default(dev, "linux,code", -ENODATA);
> > +     debug("linux code value: %d, ret: %d", linux_code, ret);
> > +     if (ret)
> > +             return ret;
>
> ret is not modified here so it'll always pass even if it fails to parse
> dev_read_u32_default.
>
> I believe dev_read_u32_default incorrectly requests an int as last
> argument while it's going to fill it with u32 data on success.
> dev_read_u8/u16 get this correctly so that might just be an oversight.
>
> Please use dev_read_u32 instead:
>
> ret = dev_read_u32(dev, "linux,code", &priv->linux_code);
> debug("linux code value: %d, ret: %d", linux_code, ret);
> return ret;
>
> (no need to check the value of ret since it's the same as
> if (ret)
>      return ret;
>
> return 0;
> )
>
> Cheers,
> Quentin