Message ID | 20190910082138.30193-1-brgl@bgdev.pl |
---|---|
State | New |
Headers | show |
Series | gpiolib: don't clear FLAG_IS_OUT when emulating open-drain/open-source | expand |
wt., 10 wrz 2019 o 12:48 Sasha Levin <sashal@kernel.org> napisał(a): > > Hi, > > [This is an automated email] > > This commit has been processed because it contains a "Fixes:" tag, > fixing commit: c663e5f56737 gpio: support native single-ended hardware drivers. > > The bot has tested the following trees: v5.2.13, v4.19.71, v4.14.142, v4.9.191. > > v5.2.13: Build OK! > v4.19.71: Build OK! > v4.14.142: Failed to apply! Possible dependencies: > 02e479808b5d ("gpio: Alter semantics of *raw* operations to actually be raw") > fac9d8850a0c ("gpio: Get rid of _prefix and __prefixes") > > v4.9.191: Failed to apply! Possible dependencies: > 02e479808b5d ("gpio: Alter semantics of *raw* operations to actually be raw") > 0db0f26c2c5d ("pinctrl-sx150x: Convert driver to use regmap API") > 2956b5d94a76 ("pinctrl / gpio: Introduce .set_config() callback for GPIO chips") > 46a5c112a401 ("gpio: merrifield: Implement gpio_get_direction callback") > 6489677f86c3 ("pinctrl-sx150x: Replace sx150x_*_cfg by means of regmap API") > 6697546d650d ("pinctrl-sx150x: Add SX1503 specific data") > 9e80f9064e73 ("pinctrl: Add SX150X GPIO Extender Pinctrl Driver") > e3ba81206811 ("pinctrl-sx150x: Improve OF device matching code") > e7a718f9b1c1 ("gpio: merrifield: Add support for hardware debouncer") > > > NOTE: The patch will not be queued to stable trees until it is upstream. > > How should we proceed with this patch? > Once it's accepted, I'll prepare backports. Bart
wt., 10 wrz 2019 o 10:22 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > When emulating open-drain/open-source by not actively driving the output > lines - we're simply changing their mode to input. This is wrong as it > will then make it impossible to change the value of such line - it's now > considered to actually be in input mode. If we want to still use the > direction_input() callback for simplicity then we need to set FLAG_IS_OUT > manually in gpiod_direction_output() and not clear it in > gpio_set_open_drain_value_commit() and > gpio_set_open_source_value_commit(). > > Fixes: c663e5f56737 ("gpio: support native single-ended hardware drivers") > Cc: stable@vger.kernel.org > Reported-by: Kent Gibson <warthog618@gmail.com> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > drivers/gpio/gpiolib.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index cca749010cd0..6bb4191d3844 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -2769,8 +2769,10 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) > if (!ret) > goto set_output_value; > /* Emulate open drain by not actively driving the line high */ > - if (value) > - return gpiod_direction_input(desc); > + if (value) { > + ret = gpiod_direction_input(desc); > + goto set_output_flag; > + } > } > else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) { > ret = gpio_set_config(gc, gpio_chip_hwgpio(desc), > @@ -2778,8 +2780,10 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) > if (!ret) > goto set_output_value; > /* Emulate open source by not actively driving the line low */ > - if (!value) > - return gpiod_direction_input(desc); > + if (!value) { > + ret = gpiod_direction_input(desc); > + goto set_output_flag; > + } > } else { > gpio_set_config(gc, gpio_chip_hwgpio(desc), > PIN_CONFIG_DRIVE_PUSH_PULL); > @@ -2787,6 +2791,17 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) > > set_output_value: > return gpiod_direction_output_raw_commit(desc, value); > + > +set_output_flag: > + /* > + * 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; > } > EXPORT_SYMBOL_GPL(gpiod_direction_output); > > @@ -3147,8 +3162,6 @@ static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value) > > if (value) { > err = chip->direction_input(chip, offset); > - if (!err) > - clear_bit(FLAG_IS_OUT, &desc->flags); > } else { > err = chip->direction_output(chip, offset, 0); > if (!err) > @@ -3178,8 +3191,6 @@ static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value > set_bit(FLAG_IS_OUT, &desc->flags); > } else { > err = chip->direction_input(chip, offset); > - if (!err) > - clear_bit(FLAG_IS_OUT, &desc->flags); > } > trace_gpio_direction(desc_to_gpio(desc), !value, err); > if (err < 0) > -- > 2.21.0 > Queued for fixes. Bart
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index cca749010cd0..6bb4191d3844 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2769,8 +2769,10 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) if (!ret) goto set_output_value; /* Emulate open drain by not actively driving the line high */ - if (value) - return gpiod_direction_input(desc); + if (value) { + ret = gpiod_direction_input(desc); + goto set_output_flag; + } } else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) { ret = gpio_set_config(gc, gpio_chip_hwgpio(desc), @@ -2778,8 +2780,10 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) if (!ret) goto set_output_value; /* Emulate open source by not actively driving the line low */ - if (!value) - return gpiod_direction_input(desc); + if (!value) { + ret = gpiod_direction_input(desc); + goto set_output_flag; + } } else { gpio_set_config(gc, gpio_chip_hwgpio(desc), PIN_CONFIG_DRIVE_PUSH_PULL); @@ -2787,6 +2791,17 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) set_output_value: return gpiod_direction_output_raw_commit(desc, value); + +set_output_flag: + /* + * 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; } EXPORT_SYMBOL_GPL(gpiod_direction_output); @@ -3147,8 +3162,6 @@ static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value) if (value) { err = chip->direction_input(chip, offset); - if (!err) - clear_bit(FLAG_IS_OUT, &desc->flags); } else { err = chip->direction_output(chip, offset, 0); if (!err) @@ -3178,8 +3191,6 @@ static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value set_bit(FLAG_IS_OUT, &desc->flags); } else { err = chip->direction_input(chip, offset); - if (!err) - clear_bit(FLAG_IS_OUT, &desc->flags); } trace_gpio_direction(desc_to_gpio(desc), !value, err); if (err < 0)