Re: [PATCH v1 1/1] gpiolib: Deduplicate gpiod_direction_input_nonotify() call

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Thu, Oct 31, 2024 at 10:22 AM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> Deduplicate gpiod_direction_input_nonotify() call in
> gpiod_direction_output_nonotify() when emulating open-drain
> or open-source behaviour. It also aligns the error check
> approaches in set_output_value and set_output_flag labels.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpio/gpiolib.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5666c462248c..9376dd270344 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2877,19 +2877,15 @@ int gpiod_direction_output_nonotify(struct gpio_desc *desc, int value)
>                 if (!ret)
>                         goto set_output_value;
>                 /* Emulate open drain by not actively driving the line high */
> -               if (value) {
> -                       ret = gpiod_direction_input_nonotify(desc);
> +               if (value)
>                         goto set_output_flag;
> -               }
>         } else if (test_bit(FLAG_OPEN_SOURCE, &flags)) {
>                 ret = gpio_set_config(desc, PIN_CONFIG_DRIVE_OPEN_SOURCE);
>                 if (!ret)
>                         goto set_output_value;
>                 /* Emulate open source by not actively driving the line low */
> -               if (!value) {
> -                       ret = gpiod_direction_input_nonotify(desc);
> +               if (!value)
>                         goto set_output_flag;
> -               }
>         } else {
>                 gpio_set_config(desc, PIN_CONFIG_DRIVE_PUSH_PULL);
>         }
> @@ -2901,15 +2897,17 @@ int gpiod_direction_output_nonotify(struct gpio_desc *desc, int value)
>         return gpiod_direction_output_raw_commit(desc, value);
>
>  set_output_flag:

Ok, I'll take it but please change this label to "emulate_output" or
something similar that's more indicative on what the goal here is.

Bart

> +       ret = gpiod_direction_input_nonotify(desc);
> +       if (ret)
> +               return ret;
>         /*
>          * When emulating open-source or open-drain functionalities by not
>          * actively driving the line (setting mode to input) we still need to
>          * set the IS_OUT flag or otherwise we won't be able to set the line
>          * value anymore.
>          */
> -       if (ret == 0)
> -               set_bit(FLAG_IS_OUT, &desc->flags);
> -       return ret;
> +       set_bit(FLAG_IS_OUT, &desc->flags);
> +       return 0;
>  }
>
>  /**
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>