linux-kernel - Re: [PATCH v1 1/1] gpiolib: Deduplicate gpiod_direction_input

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=MeiZ_U0UoU1atWxyWxEhcKwUcCK_cbYu_p-DQar27Fw+A@mail.gmail.com>
Date: Thu, 31 Oct 2024 20:14:56 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH v1 1/1] gpiolib: Deduplicate gpiod_direction_input_nonotify()
 call

On Thu, Oct 31, 2024 at 10:22 AM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> 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@...ux.intel.com>
> ---
>  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
>