On 9/30/18 12:45 AM, Vagrant Cascadian wrote:
> From: Vasily Khoruzhick <anars...@gmail.com>
> 
> Sleep gpio is optional, so it's possible to have reset gpio, but no sleep 
> gpio.
> We shouldn't fail early in case of missing sleep gpio, otherwise we won't
> deassert reset.
> 
> Signed-off-by: Vasily Khoruzhick <anars...@gmail.com>
> Signed-off-by: Vagrant Cascadian <vagr...@debian.org>
> ---
> 
>  drivers/video/bridge/video-bridge-uclass.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/bridge/video-bridge-uclass.c 
> b/drivers/video/bridge/video-bridge-uclass.c
> index cd4959cc71..46936a0626 100644
> --- a/drivers/video/bridge/video-bridge-uclass.c
> +++ b/drivers/video/bridge/video-bridge-uclass.c
> @@ -110,7 +110,7 @@ int video_bridge_set_active(struct udevice *dev, bool 
> active)
>  
>       debug("%s: %d\n", __func__, active);
>       ret = dm_gpio_set_value(&uc_priv->sleep, !active);

So if I get this correctly, uc_priv->sleep.dev would be NULL if there
was no GPIO specified? So wouldn't it be cleaner to say:
        if (uc_priv->sleep.dev) {
                ret = dm_gpio_set_value(&uc_priv->sleep, !active);
                ...

> -     if (ret)
> +     if (ret != -ENOENT)
>               return ret;
>       if (active) {
>               ret = dm_gpio_set_value(&uc_priv->reset, true);
> @@ -120,7 +120,7 @@ int video_bridge_set_active(struct udevice *dev, bool 
> active)
>               ret = dm_gpio_set_value(&uc_priv->reset, false);
>       }
>  
> -     return ret;
> +     return 0;

This would loose the return value from the last statement in the if
clause. So what about negating this clause:
        if (!active)
                return 0;
and change the rest accordingly?

Cheers,
Andre.


>  }
>  
>  UCLASS_DRIVER(video_bridge) = {
> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to