* [PATCH 00/54] GPIO: clamp return values from drivers @ 2015-12-22 14:08 Linus Walleij 2015-12-22 15:06 ` Vladimir Zapolskiy 0 siblings, 1 reply; 3+ messages in thread From: Linus Walleij @ 2015-12-22 14:08 UTC (permalink / raw) To: linux-gpio, Björn Andersson, Johan Hovold, Vladimir Zapolskiy, Michael Trimarchi Cc: Linus Walleij It was recently discovered that a bunch of GPIO drivers were not clamping their return values from the .get() function to [0,1]. Some did. Some did and returned a proper errorcode, for example for GPIOs on slow buses. Some returned the bit set in some register, so GPIO at offset 0 would return 0/1 but offset 2 would return 0/4 and expect the core to clamp it. Which it used to do, until we decided to propagate error codes. Which we shouldn't have, before going over all drivers to check if they were doing this "return something other than zero and expect it to be treated as asserted" business. Usually it was all OK because the gpiolib core did this: value = value < 0 ? value : !!value; So the only time the error pops out is when a 32bit machine expect something greater than 2^31 to be interpreted as asserted. This is typically the case when a 32bit register contains 32 GPIOs, bit 31 is a GPIO line and is read: then that is seen as a negative value and propagated as an error code, so all other cases including the 31 first GPIOs work fine. That is why the error is not so prominent, and took some time to detect. Anyway, this series fixes all drivers in the kernel. Merge tactic: I will merge the GPIO and pinctrl subsystem changes for v4.5, returning the propagation of the error code. Subsystem maintainers may merge this as a fix for their v4.5 trees or ACK them so that I can merge them. In the next kernel cycle for v4.6 I will take a sweep and merge remaining fixes. Linus Walleij (54): gpio: da9052: Be sure to clamp return value gpio: davinci: Be sure to clamp return value gpio: em: Be sure to clamp return value gpio: ich: Be sure to clamp return value gpio: intel-mid: Be sure to clamp return value gpio: janz-ttl: Be sure to clamp return value gpio: kempld: Be sure to clamp return value gpio: lpc32xx: Be sure to clamp return value gpio: max732x: Be sure to clamp return value gpio: ml-ioh: Be sure to clamp return value gpio: mpc8xxx: Be sure to clamp return value gpio: msic: Be sure to clamp return value gpio: pcf857x: Be sure to clamp return value gpio: pch: Be sure to clamp return value gpio: sa1100: Be sure to clamp return value gpio: sta2x11: Be sure to clamp return value gpio: sx150x: Be sure to clamp return value gpio: tc3589x: Be sure to clamp return value gpio: twl4030: Be sure to clamp return value gpio: tz1090: Be sure to clamp return value gpio: tx1090-pdc: Be sure to clamp return value gpio: ucb1400: Be sure to clamp return value gpio: viperboard: Be sure to clamp return value pinctrl: baytrail: Be sure to clamp return value pinctrl: coh901: Be sure to clamp return value pinctrl: xway: Be sure to clamp return value pinctrl: spmi-gpio: Be sure to clamp return value pinctrl: spmi-mpp: Be sure to clamp return value pinctrl: ssbi-gpio: Be sure to clamp return value gpio: ssbi-mpp: Be sure to clamp return value pinctrl: sunxi: Be sure to clamp return value ARM: scoop: Be sure to clamp return value ARM: sa1100/simpad: Be sure to clamp return value blackfin: gpio: Be sure to clamp return value m68k: coldfire/gpio: Be sure to clamp return value mips: alchemy: Be sure to clamp return value mips: ar7/gpio: Be sure to clamp return value mips: txx9: Be sure to clamp return value mips: rb532: Be sure to clamp return value mips: txx9: iocled: Be sure to clamp return value powerpc: ppc4cc/gpio: Be sure to clamp return value powerpc: qe_lib/gpio: Be sure to clamp return value powerpc: simple_gpio: Be sure to clamp return value unicore: gpio: Be sure to clamp return value mfd: asic3: Be sure to clamp return value mfd: dm355evm_mps: Be sure to clamp return value mfd: htc-egpio: Be sure to clamp return value mfd: tc6393xb: Be sure to clamp return value mfd: tps65010: Be sure to clamp return value mfd: ucb1x00-core: Be sure to clamp return value fbdev: via-gpio: Be sure to clamp return value sound: soc: wm8903: Be sure to clamp return value sound: soc: ac97: Be sure to clamp return value Revert "gpio: revert get() to non-errorprogating behaviour" arch/arm/common/scoop.c | 2 +- arch/arm/mach-sa1100/simpad.c | 4 ++-- arch/blackfin/kernel/bfin_gpio.c | 2 +- arch/m68k/coldfire/gpio.c | 2 +- arch/mips/alchemy/common/gpiolib.c | 6 +++--- arch/mips/ar7/gpio.c | 2 +- arch/mips/kernel/gpio_txx9.c | 2 +- arch/mips/rb532/gpio.c | 2 +- arch/mips/txx9/generic/setup.c | 2 +- arch/powerpc/sysdev/ppc4xx_gpio.c | 2 +- arch/powerpc/sysdev/qe_lib/gpio.c | 2 +- arch/powerpc/sysdev/simple_gpio.c | 2 +- arch/unicore32/kernel/gpio.c | 2 +- drivers/gpio/gpio-da9052.c | 9 +++------ drivers/gpio/gpio-davinci.c | 2 +- drivers/gpio/gpio-em.c | 2 +- drivers/gpio/gpio-ich.c | 4 ++-- drivers/gpio/gpio-intel-mid.c | 2 +- drivers/gpio/gpio-janz-ttl.c | 2 +- drivers/gpio/gpio-kempld.c | 2 +- drivers/gpio/gpio-lpc32xx.c | 8 ++++---- drivers/gpio/gpio-max732x.c | 4 ++-- drivers/gpio/gpio-ml-ioh.c | 2 +- drivers/gpio/gpio-mpc8xxx.c | 2 +- drivers/gpio/gpio-msic.c | 2 +- drivers/gpio/gpio-pcf857x.c | 2 +- drivers/gpio/gpio-pch.c | 2 +- drivers/gpio/gpio-sa1100.c | 2 +- drivers/gpio/gpio-sta2x11.c | 2 +- drivers/gpio/gpio-sx150x.c | 2 +- drivers/gpio/gpio-tc3589x.c | 2 +- drivers/gpio/gpio-twl4030.c | 2 +- drivers/gpio/gpio-tz1090-pdc.c | 2 +- drivers/gpio/gpio-tz1090.c | 2 +- drivers/gpio/gpio-ucb1400.c | 2 +- drivers/gpio/gpio-viperboard.c | 2 +- drivers/gpio/gpiolib.c | 8 +------- drivers/mfd/asic3.c | 3 ++- drivers/mfd/dm355evm_msp.c | 2 +- drivers/mfd/htc-egpio.c | 2 +- drivers/mfd/tc6393xb.c | 4 ++-- drivers/mfd/tps65010.c | 4 ++-- drivers/mfd/ucb1x00-core.c | 2 +- drivers/pinctrl/intel/pinctrl-baytrail.c | 2 +- drivers/pinctrl/pinctrl-coh901.c | 2 +- drivers/pinctrl/pinctrl-xway.c | 2 +- drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 +- drivers/pinctrl/qcom/pinctrl-spmi-mpp.c | 2 +- drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 4 ++-- drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c | 6 +++--- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 +- drivers/video/fbdev/via/via-gpio.c | 2 +- sound/soc/codecs/wm8903.c | 2 +- sound/soc/soc-ac97.c | 2 +- 54 files changed, 70 insertions(+), 78 deletions(-) -- 2.4.3 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 00/54] GPIO: clamp return values from drivers 2015-12-22 14:08 [PATCH 00/54] GPIO: clamp return values from drivers Linus Walleij @ 2015-12-22 15:06 ` Vladimir Zapolskiy 2015-12-24 18:51 ` Linus Walleij 0 siblings, 1 reply; 3+ messages in thread From: Vladimir Zapolskiy @ 2015-12-22 15:06 UTC (permalink / raw) To: Linus Walleij, linux-gpio, Björn Andersson, Johan Hovold, Michael Trimarchi Hi Linus, On 22.12.2015 16:08, Linus Walleij wrote: > It was recently discovered that a bunch of GPIO drivers were not > clamping their return values from the .get() function to [0,1]. > Some did. Some did and returned a proper errorcode, for example > for GPIOs on slow buses. Some returned the bit set in some register, > so GPIO at offset 0 would return 0/1 but offset 2 would return > 0/4 and expect the core to clamp it. Which it used to do, until we > decided to propagate error codes. Which we shouldn't have, before > going over all drivers to check if they were doing this "return > something other than zero and expect it to be treated as asserted" > business. > > Usually it was all OK because the gpiolib core did this: > > value = value < 0 ? value : !!value; > > So the only time the error pops out is when a 32bit machine expect > something greater than 2^31 to be interpreted as asserted. This is > typically the case when a 32bit register contains 32 GPIOs, bit 31 > is a GPIO line and is read: then that is seen as a negative > value and propagated as an error code, so all other cases including > the 31 first GPIOs work fine. That is why the error is not so > prominent, and took some time to detect. > > Anyway, this series fixes all drivers in the kernel. > > Merge tactic: I will merge the GPIO and pinctrl subsystem changes > for v4.5, returning the propagation of the error code. > > Subsystem maintainers may merge this as a fix for their v4.5 trees > or ACK them so that I can merge them. In the next kernel cycle > for v4.6 I will take a sweep and merge remaining fixes. > thank you for this work. Please consider to add to the pile one more change. With best holiday wishes, Vladimir --- >From a19047b2131f73cdf494abd44d76de6f57ca81ff Mon Sep 17 00:00:00 2001 From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> Date: Tue, 22 Dec 2015 16:37:28 +0200 Subject: [PATCH] gpio: update gpiochip .get() callback description Since gpiochip .get() callback may return a negative error value, it strictly limits the range of possible non-error returned values to a subset of [30:0] bitmask, however on practice on success all gpiochip drivers return either 0 for low signal or 1 for high signal, this is assured by "gpio: *: Be sure to clamp return value" series of changes. To avoid any confusion, misinterpretation and potential errors while developing gpiochip drivers in future convert this implicit assumption to a mandatory rule. For output signals with unknown output signal state gpiochip drivers should return a negative error instead of 0. Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> --- include/linux/gpio/driver.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index b02c43b..9907975 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -32,8 +32,7 @@ struct seq_file; * (same as GPIOF_DIR_XXX), or negative error * @direction_input: configures signal "offset" as input, or returns error * @direction_output: configures signal "offset" as output, or returns error - * @get: returns value for signal "offset"; for output signals this - * returns either the value actually sensed, or zero + * @get: returns value for signal "offset", 0=low, 1=high, or negative error * @set: assigns output value for signal "offset" * @set_multiple: assigns output values for multiple signals defined by "mask" * @set_debounce: optional hook for setting debounce time for specified gpio in -- 2.5.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 00/54] GPIO: clamp return values from drivers 2015-12-22 15:06 ` Vladimir Zapolskiy @ 2015-12-24 18:51 ` Linus Walleij 0 siblings, 0 replies; 3+ messages in thread From: Linus Walleij @ 2015-12-24 18:51 UTC (permalink / raw) To: Vladimir Zapolskiy Cc: linux-gpio@vger.kernel.org, Björn Andersson, Johan Hovold, Michael Trimarchi On Tue, Dec 22, 2015 at 4:06 PM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > From a19047b2131f73cdf494abd44d76de6f57ca81ff Mon Sep 17 00:00:00 2001 > From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> > Date: Tue, 22 Dec 2015 16:37:28 +0200 > Subject: [PATCH] gpio: update gpiochip .get() callback description > > Since gpiochip .get() callback may return a negative error value, it > strictly limits the range of possible non-error returned values to > a subset of [30:0] bitmask, however on practice on success all > gpiochip drivers return either 0 for low signal or 1 for high signal, > this is assured by "gpio: *: Be sure to clamp return value" series of > changes. To avoid any confusion, misinterpretation and potential > errors while developing gpiochip drivers in future convert this > implicit assumption to a mandatory rule. > > For output signals with unknown output signal state gpiochip drivers > should return a negative error instead of 0. > > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> Patch applied! Merry Christmas, Linus Walleij ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-24 18:51 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-22 14:08 [PATCH 00/54] GPIO: clamp return values from drivers Linus Walleij 2015-12-22 15:06 ` Vladimir Zapolskiy 2015-12-24 18:51 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).