| Message ID | 20241031092154.2244728-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] gpiolib: Deduplicate gpiod_direction_input_nonotify() call | expand |
On Thu, Oct 31, 2024 at 10:22 AM Andy Shevchenko <andriy.shevchenko@linux.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@linux.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 >
On Thu, Oct 31, 2024 at 08:14:56PM +0100, Bartosz Golaszewski wrote: > On Thu, Oct 31, 2024 at 10:22 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: ... > > 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. It's out of scope of this patch.
On Fri, Nov 1, 2024 at 9:15 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Oct 31, 2024 at 08:14:56PM +0100, Bartosz Golaszewski wrote: > > On Thu, Oct 31, 2024 at 10:22 AM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > ... > > > > 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. > > It's out of scope of this patch. > It is not though. You are already touching this code, make it even better. Just say in the commit message: "while at it, rename the label to ..." Bartosz
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: + 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; } /**
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@linux.intel.com> --- drivers/gpio/gpiolib.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)