On Mon, 07 Jul 2014, Rickard Strandqvist wrote:

> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose,
> and made same change to clarify the similarities in function.
> 
> This was found using a static code analysis program called cppcheck
> 
> Signed-off-by: Rickard Strandqvist <rickard_strandqv...@spectrumdigital.se>
> ---
>  drivers/video/backlight/jornada720_lcd.c |   31 
> ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/video/backlight/jornada720_lcd.c 
> b/drivers/video/backlight/jornada720_lcd.c
> index da3876c..d16abcf 100644
> --- a/drivers/video/backlight/jornada720_lcd.c
> +++ b/drivers/video/backlight/jornada720_lcd.c
> @@ -36,44 +36,41 @@ static int jornada_lcd_get_power(struct lcd_device *ld)
>  
>  static int jornada_lcd_get_contrast(struct lcd_device *ld)
>  {
> -     int ret;
> +     int ret = -ETIMEDOUT;
>  
>       if (jornada_lcd_get_power(ld) != FB_BLANK_UNBLANK)
>               return 0;
>  
>       jornada_ssp_start();
>  
> -     if (jornada_ssp_byte(GETCONTRAST) != TXDUMMY) {
> +     if (jornada_ssp_byte(GETCONTRAST) != TXDUMMY)
>               dev_err(&ld->dev, "get contrast failed\n");
> -             jornada_ssp_end();
> -             return -ETIMEDOUT;
> -     } else {
> +     else
>               ret = jornada_ssp_byte(TXDUMMY);
> -             jornada_ssp_end();
> -             return ret;
> -     }
> +
> +     jornada_ssp_end();
> +
> +     return ret;
>  }
>  
>  static int jornada_lcd_set_contrast(struct lcd_device *ld, int value)
>  {
> -     int ret;
> +     int ret = -ETIMEDOUT;
>  
>       jornada_ssp_start();
>  
>       /* start by sending our set contrast cmd to mcu */
> -     ret = jornada_ssp_byte(SETCONTRAST);
> -
> +     if (jornada_ssp_byte(SETCONTRAST) != TXDUMMY)
> +             dev_err(&ld->dev, "set contrast failed\n");
>       /* push the new value */
> -     if (jornada_ssp_byte(value) != TXDUMMY) {
> +     else if (jornada_ssp_byte(value) != TXDUMMY)
>               dev_err(&ld->dev, "set contrast failed\n");
> -             jornada_ssp_end();
> -             return -ETIMEDOUT;
> -     }
> +     else /* if we get here we can assume everything went well */
> +             ret = 0;
>  
> -     /* if we get here we can assume everything went well */
>       jornada_ssp_end();
>  
> -     return 0;
> +     return ret;
>  }
>  
>  static int jornada_lcd_set_power(struct lcd_device *ld, int power)

Counter suggestion.

What do you think, I think this looks cleaner:

static int jornada_lcd_get_contrast(struct lcd_device *ld)
{
        int ret;

        if (jornada_lcd_get_power(ld) != FB_BLANK_UNBLANK)
                return 0;

        jornada_ssp_start();

        if (jornada_ssp_byte(GETCONTRAST) == TXDUMMY) {
                ret = jornada_ssp_byte(TXDUMMY);
                goto success;
        }

        dev_err(&ld->dev, "failed to set contrast\n");
        ret = -ETIMEDOUT;

success:
        jornada_ssp_end();
        return ret;
}

static int jornada_lcd_set_contrast(struct lcd_device *ld, int value)
{
        int ret = 0;

        jornada_ssp_start();

        /* start by sending our set contrast cmd to mcu */
        if (jornada_ssp_byte(SETCONTRAST) == TXDUMMY) {
                /* if successful, push the new value */
                if (jornada_ssp_byte(value) == TXDUMMY)
                        goto success;
        }

        dev_err(&ld->dev, "failed to set contrast\n");
        ret = -ETIMEDOUT;

success:
        jornada_ssp_end();
        return ret;
}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to