Re: [PATCH] gpio: altera-a10sr: Set proper output level for direction_output

[Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>]

czw., 17 sty 2019 o 11:16 Axel Lin <axel.lin@xxxxxxxxxx> napisał(a):
>
> The altr_a10sr_gpio_direction_output should set proper output level
> based on the value argument.
>
> Also change the coding style to make it does error checking first.
>

Please, split it into two patches.

> Signed-off-by: Axel Lin <axel.lin@xxxxxxxxxx>
> ---
> Hi Thor,
> I don't have this h/w, so please help to review and test it.
>
> BTW, the coding style change is because checkpatch complains too long line
> if adding bracket like below statemet:
>  if (nr <= (ALTR_A10SR_OUT_VALID_RANGE_HI - ALTR_A10SR_LED_VALID_SHIFT)) {
>
> Thanks,
> Axel
>  drivers/gpio/gpio-altera-a10sr.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpio-altera-a10sr.c b/drivers/gpio/gpio-altera-a10sr.c
> index 6b11f1314248..1cea4efccf7c 100644
> --- a/drivers/gpio/gpio-altera-a10sr.c
> +++ b/drivers/gpio/gpio-altera-a10sr.c
> @@ -58,17 +58,20 @@ static void altr_a10sr_gpio_set(struct gpio_chip *chip, unsigned int offset,
>  static int altr_a10sr_gpio_direction_input(struct gpio_chip *gc,
>                                            unsigned int nr)
>  {
> -       if (nr >= (ALTR_A10SR_IN_VALID_RANGE_LO - ALTR_A10SR_LED_VALID_SHIFT))
> -               return 0;
> -       return -EINVAL;
> +       if (nr < (ALTR_A10SR_IN_VALID_RANGE_LO - ALTR_A10SR_LED_VALID_SHIFT))
> +               return -EINVAL;
> +
> +       return 0;
>  }
>
>  static int altr_a10sr_gpio_direction_output(struct gpio_chip *gc,
>                                             unsigned int nr, int value)
>  {
> -       if (nr <= (ALTR_A10SR_OUT_VALID_RANGE_HI - ALTR_A10SR_LED_VALID_SHIFT))
> -               return 0;
> -       return -EINVAL;
> +       if (nr > (ALTR_A10SR_OUT_VALID_RANGE_HI - ALTR_A10SR_LED_VALID_SHIFT))
> +               return -EINVAL;
> +
> +       altr_a10sr_gpio_set(gc, nr, value);

Do this in the first patch and add a Fixes: tag, since this should go
into stable.

Bart

> +       return 0;
>  }
>
>  static const struct gpio_chip altr_a10sr_gc = {
> --
> 2.17.1
>