| Message ID | 1429630951-27082-9-git-send-email-johan@kernel.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: > Remove gpiod_sysfs_set_active_low (and gpio_sysfs_set_active_low) which > allowed code to change the polarity of a gpio line even after it had > been exported through sysfs. > > Drivers should not care, and generally does not know, about gpio-line > polarity which is a hardware feature that needs to be described by > firmware. > > It is currently possible to define gpio-line polarity in device-tree and > acpi firmware or using platform data. Userspace can also change the > polarity through sysfs. > > Note that drivers using the legacy gpio interface could still use > GPIOF_ACTIVE_LOW to change the polarity before exporting the gpio. > > There are no in-kernel users of this interface. > > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Harry Wei <harryxiyou@gmail.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: linux-doc@vger.kernel.org > Cc: linux-kernel@zh-kernel.org > Cc: linux-arch@vger.kernel.org > Signed-off-by: Johan Hovold <johan@kernel.org> > --- > Documentation/gpio/gpio-legacy.txt | 9 ------- > Documentation/gpio/sysfs.txt | 8 ------- > Documentation/zh_CN/gpio.txt | 8 ------- > drivers/gpio/gpiolib-sysfs.c | 48 ++------------------------------------ > include/asm-generic/gpio.h | 5 ---- > include/linux/gpio.h | 7 ------ > include/linux/gpio/consumer.h | 6 ----- > 7 files changed, 2 insertions(+), 89 deletions(-) > > diff --git a/Documentation/gpio/gpio-legacy.txt b/Documentation/gpio/gpio-legacy.txt > index 6f83fa965b4b..79ab5648d69b 100644 > --- a/Documentation/gpio/gpio-legacy.txt > +++ b/Documentation/gpio/gpio-legacy.txt > @@ -751,9 +751,6 @@ requested using gpio_request(): > int gpio_export_link(struct device *dev, const char *name, > unsigned gpio) > > - /* change the polarity of a GPIO node in sysfs */ > - int gpio_sysfs_set_active_low(unsigned gpio, int value); > - > After a kernel driver requests a GPIO, it may only be made available in > the sysfs interface by gpio_export(). The driver can control whether the > signal direction may change. This helps drivers prevent userspace code > @@ -767,9 +764,3 @@ After the GPIO has been exported, gpio_export_link() allows creating > symlinks from elsewhere in sysfs to the GPIO sysfs node. Drivers can > use this to provide the interface under their own device in sysfs with > a descriptive name. > - > -Drivers can use gpio_sysfs_set_active_low() to hide GPIO line polarity > -differences between boards from user space. This only affects the > -sysfs interface. Polarity change can be done both before and after > -gpio_export(), and previously enabled poll(2) support for either > -rising or falling edge will be reconfigured to follow this setting. > diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt > index c2c3a97f8ff7..535b6a8a7a7c 100644 > --- a/Documentation/gpio/sysfs.txt > +++ b/Documentation/gpio/sysfs.txt > @@ -132,9 +132,6 @@ requested using gpio_request(): > int gpiod_export_link(struct device *dev, const char *name, > struct gpio_desc *desc); > > - /* change the polarity of a GPIO node in sysfs */ > - int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value); > - > After a kernel driver requests a GPIO, it may only be made available in > the sysfs interface by gpiod_export(). The driver can control whether the > signal direction may change. This helps drivers prevent userspace code > @@ -148,8 +145,3 @@ After the GPIO has been exported, gpiod_export_link() allows creating > symlinks from elsewhere in sysfs to the GPIO sysfs node. Drivers can > use this to provide the interface under their own device in sysfs with > a descriptive name. > - > -Drivers can use gpiod_sysfs_set_active_low() to hide GPIO line polarity > -differences between boards from user space. Polarity change can be done both > -before and after gpiod_export(), and previously enabled poll(2) support for > -either rising or falling edge will be reconfigured to follow this setting. > diff --git a/Documentation/zh_CN/gpio.txt b/Documentation/zh_CN/gpio.txt > index d5b8f01833f4..bce972521065 100644 > --- a/Documentation/zh_CN/gpio.txt > +++ b/Documentation/zh_CN/gpio.txt > @@ -638,9 +638,6 @@ GPIO 控制器的路径类似 /sys/class/gpio/gpiochip42/ (对于从#42 GPIO > int gpio_export_link(struct device *dev, const char *name, > unsigned gpio) > > - /* 改变 sysfs 中的一个 GPIO 节点的极性 */ > - int gpio_sysfs_set_active_low(unsigned gpio, int value); > - > 在一个内核驱动申请一个 GPIO 之后，它可以通过 gpio_export()使其在 sysfs > 接口中可见。该驱动可以控制信号方向是否可修改。这有助于防止用户空间代码无意间 > 破坏重要的系统状态。 > @@ -651,8 +648,3 @@ GPIO 控制器的路径类似 /sys/class/gpio/gpiochip42/ (对于从#42 GPIO > 在 GPIO 被导出之后，gpio_export_link()允许在 sysfs 文件系统的任何地方 > 创建一个到这个 GPIO sysfs 节点的符号链接。这样驱动就可以通过一个描述性的 > 名字，在 sysfs 中他们所拥有的设备下提供一个(到这个 GPIO sysfs 节点的)接口。 > - > -驱动可以使用 gpio_sysfs_set_active_low() 来在用户空间隐藏电路板之间 > -GPIO 线的极性差异。这个仅对 sysfs 接口起作用。极性的改变可以在 gpio_export() > -前后进行,且之前使能的轮询操作(poll(2))支持(上升或下降沿)将会被重新配置来遵循 > -这个设置。 > diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c > index 31434c5a90ef..8a95a954f514 100644 > --- a/drivers/gpio/gpiolib-sysfs.c > +++ b/drivers/gpio/gpiolib-sysfs.c > @@ -293,8 +293,8 @@ static int sysfs_set_active_low(struct gpio_desc *desc, struct device *dev, > clear_bit(FLAG_ACTIVE_LOW, &desc->flags); > > /* reconfigure poll(2) support if enabled on one edge only */ > - if (dev != NULL && (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ > - !!test_bit(FLAG_TRIG_FALL, &desc->flags))) { > + if (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ > + !!test_bit(FLAG_TRIG_FALL, &desc->flags)) { This change seems to be unrelated to this patch... Otherwise, I agree and good riddance! -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 12:54:15PM +0900, Alexandre Courbot wrote: > On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: > > Remove gpiod_sysfs_set_active_low (and gpio_sysfs_set_active_low) which > > allowed code to change the polarity of a gpio line even after it had > > been exported through sysfs. > > > > Drivers should not care, and generally does not know, about gpio-line > > polarity which is a hardware feature that needs to be described by > > firmware. > > > > It is currently possible to define gpio-line polarity in device-tree and > > acpi firmware or using platform data. Userspace can also change the > > polarity through sysfs. > > > > Note that drivers using the legacy gpio interface could still use > > GPIOF_ACTIVE_LOW to change the polarity before exporting the gpio. > > > > There are no in-kernel users of this interface. > > > > Cc: Jonathan Corbet <corbet@lwn.net> > > Cc: Harry Wei <harryxiyou@gmail.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: linux-doc@vger.kernel.org > > Cc: linux-kernel@zh-kernel.org > > Cc: linux-arch@vger.kernel.org > > Signed-off-by: Johan Hovold <johan@kernel.org> > > --- > > diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c > > index 31434c5a90ef..8a95a954f514 100644 > > --- a/drivers/gpio/gpiolib-sysfs.c > > +++ b/drivers/gpio/gpiolib-sysfs.c > > @@ -293,8 +293,8 @@ static int sysfs_set_active_low(struct gpio_desc *desc, struct device *dev, > > clear_bit(FLAG_ACTIVE_LOW, &desc->flags); > > > > /* reconfigure poll(2) support if enabled on one edge only */ > > - if (dev != NULL && (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ > > - !!test_bit(FLAG_TRIG_FALL, &desc->flags))) { > > + if (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ > > + !!test_bit(FLAG_TRIG_FALL, &desc->flags)) { > > This change seems to be unrelated to this patch... This helper is now only called from the attribute operation and dev will never be NULL. On the other hand, it was never called with a NULL argument before this patch either (the test has always been bogus). Let me know if you prefer this bit to be a separate patch. > Otherwise, I agree and good riddance! Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 5:16 PM, Johan Hovold <johan@kernel.org> wrote: > On Mon, Apr 27, 2015 at 12:54:15PM +0900, Alexandre Courbot wrote: >> On Wed, Apr 22, 2015 at 12:42 AM, Johan Hovold <johan@kernel.org> wrote: >> > Remove gpiod_sysfs_set_active_low (and gpio_sysfs_set_active_low) which >> > allowed code to change the polarity of a gpio line even after it had >> > been exported through sysfs. >> > >> > Drivers should not care, and generally does not know, about gpio-line >> > polarity which is a hardware feature that needs to be described by >> > firmware. >> > >> > It is currently possible to define gpio-line polarity in device-tree and >> > acpi firmware or using platform data. Userspace can also change the >> > polarity through sysfs. >> > >> > Note that drivers using the legacy gpio interface could still use >> > GPIOF_ACTIVE_LOW to change the polarity before exporting the gpio. >> > >> > There are no in-kernel users of this interface. >> > >> > Cc: Jonathan Corbet <corbet@lwn.net> >> > Cc: Harry Wei <harryxiyou@gmail.com> >> > Cc: Arnd Bergmann <arnd@arndb.de> >> > Cc: linux-doc@vger.kernel.org >> > Cc: linux-kernel@zh-kernel.org >> > Cc: linux-arch@vger.kernel.org >> > Signed-off-by: Johan Hovold <johan@kernel.org> >> > --- > >> > diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c >> > index 31434c5a90ef..8a95a954f514 100644 >> > --- a/drivers/gpio/gpiolib-sysfs.c >> > +++ b/drivers/gpio/gpiolib-sysfs.c >> > @@ -293,8 +293,8 @@ static int sysfs_set_active_low(struct gpio_desc *desc, struct device *dev, >> > clear_bit(FLAG_ACTIVE_LOW, &desc->flags); >> > >> > /* reconfigure poll(2) support if enabled on one edge only */ >> > - if (dev != NULL && (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ >> > - !!test_bit(FLAG_TRIG_FALL, &desc->flags))) { >> > + if (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ >> > + !!test_bit(FLAG_TRIG_FALL, &desc->flags)) { >> >> This change seems to be unrelated to this patch... > > This helper is now only called from the attribute operation and dev > will never be NULL. > > On the other hand, it was never called with a NULL argument before this > patch either (the test has always been bogus). Let me know if you prefer > this bit to be a separate patch. Nope, after reading your explanation I understand why you want to have this here. Thanks for clarifying! -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/gpio/gpio-legacy.txt b/Documentation/gpio/gpio-legacy.txt index 6f83fa965b4b..79ab5648d69b 100644 --- a/Documentation/gpio/gpio-legacy.txt +++ b/Documentation/gpio/gpio-legacy.txt @@ -751,9 +751,6 @@ requested using gpio_request(): int gpio_export_link(struct device *dev, const char *name, unsigned gpio) - /* change the polarity of a GPIO node in sysfs */ - int gpio_sysfs_set_active_low(unsigned gpio, int value); - After a kernel driver requests a GPIO, it may only be made available in the sysfs interface by gpio_export(). The driver can control whether the signal direction may change. This helps drivers prevent userspace code @@ -767,9 +764,3 @@ After the GPIO has been exported, gpio_export_link() allows creating symlinks from elsewhere in sysfs to the GPIO sysfs node. Drivers can use this to provide the interface under their own device in sysfs with a descriptive name. - -Drivers can use gpio_sysfs_set_active_low() to hide GPIO line polarity -differences between boards from user space. This only affects the -sysfs interface. Polarity change can be done both before and after -gpio_export(), and previously enabled poll(2) support for either -rising or falling edge will be reconfigured to follow this setting. diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt index c2c3a97f8ff7..535b6a8a7a7c 100644 --- a/Documentation/gpio/sysfs.txt +++ b/Documentation/gpio/sysfs.txt @@ -132,9 +132,6 @@ requested using gpio_request(): int gpiod_export_link(struct device *dev, const char *name, struct gpio_desc *desc); - /* change the polarity of a GPIO node in sysfs */ - int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value); - After a kernel driver requests a GPIO, it may only be made available in the sysfs interface by gpiod_export(). The driver can control whether the signal direction may change. This helps drivers prevent userspace code @@ -148,8 +145,3 @@ After the GPIO has been exported, gpiod_export_link() allows creating symlinks from elsewhere in sysfs to the GPIO sysfs node. Drivers can use this to provide the interface under their own device in sysfs with a descriptive name. - -Drivers can use gpiod_sysfs_set_active_low() to hide GPIO line polarity -differences between boards from user space. Polarity change can be done both -before and after gpiod_export(), and previously enabled poll(2) support for -either rising or falling edge will be reconfigured to follow this setting. diff --git a/Documentation/zh_CN/gpio.txt b/Documentation/zh_CN/gpio.txt index d5b8f01833f4..bce972521065 100644 --- a/Documentation/zh_CN/gpio.txt +++ b/Documentation/zh_CN/gpio.txt @@ -638,9 +638,6 @@ GPIO 控制器的路径类似 /sys/class/gpio/gpiochip42/ (对于从#42 GPIO int gpio_export_link(struct device *dev, const char *name, unsigned gpio) - /* 改变 sysfs 中的一个 GPIO 节点的极性 */ - int gpio_sysfs_set_active_low(unsigned gpio, int value); - 在一个内核驱动申请一个 GPIO 之后，它可以通过 gpio_export()使其在 sysfs 接口中可见。该驱动可以控制信号方向是否可修改。这有助于防止用户空间代码无意间 破坏重要的系统状态。 @@ -651,8 +648,3 @@ GPIO 控制器的路径类似 /sys/class/gpio/gpiochip42/ (对于从#42 GPIO 在 GPIO 被导出之后，gpio_export_link()允许在 sysfs 文件系统的任何地方 创建一个到这个 GPIO sysfs 节点的符号链接。这样驱动就可以通过一个描述性的 名字，在 sysfs 中他们所拥有的设备下提供一个(到这个 GPIO sysfs 节点的)接口。 - -驱动可以使用 gpio_sysfs_set_active_low() 来在用户空间隐藏电路板之间 -GPIO 线的极性差异。这个仅对 sysfs 接口起作用。极性的改变可以在 gpio_export() -前后进行,且之前使能的轮询操作(poll(2))支持(上升或下降沿)将会被重新配置来遵循 -这个设置。 diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c index 31434c5a90ef..8a95a954f514 100644 --- a/drivers/gpio/gpiolib-sysfs.c +++ b/drivers/gpio/gpiolib-sysfs.c @@ -293,8 +293,8 @@ static int sysfs_set_active_low(struct gpio_desc *desc, struct device *dev, clear_bit(FLAG_ACTIVE_LOW, &desc->flags); /* reconfigure poll(2) support if enabled on one edge only */ - if (dev != NULL && (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ - !!test_bit(FLAG_TRIG_FALL, &desc->flags))) { + if (!!test_bit(FLAG_TRIG_RISE, &desc->flags) ^ + !!test_bit(FLAG_TRIG_FALL, &desc->flags)) { unsigned long trigger_flags = desc->flags & GPIO_TRIGGER_MASK; gpio_setup_irq(desc, dev, 0); @@ -666,50 +666,6 @@ int gpiod_export_link(struct device *dev, const char *name, EXPORT_SYMBOL_GPL(gpiod_export_link); /** - * gpiod_sysfs_set_active_low - set the polarity of gpio sysfs value - * @gpio: gpio to change - * @value: non-zero to use active low, i.e. inverted values - * - * Set the polarity of /sys/class/gpio/gpioN/value sysfs attribute. - * The GPIO does not have to be exported yet. If poll(2) support has - * been enabled for either rising or falling edge, it will be - * reconfigured to follow the new polarity. - * - * Returns zero on success, else an error. - */ -int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value) -{ - struct device *dev = NULL; - int status = -EINVAL; - - if (!desc) { - pr_warn("%s: invalid GPIO\n", __func__); - return -EINVAL; - } - - mutex_lock(&sysfs_lock); - - if (test_bit(FLAG_EXPORT, &desc->flags)) { - dev = class_find_device(&gpio_class, NULL, desc, match_export); - if (dev == NULL) { - status = -ENODEV; - goto unlock; - } - } - - status = sysfs_set_active_low(desc, dev, value); - put_device(dev); -unlock: - mutex_unlock(&sysfs_lock); - - if (status) - gpiod_dbg(desc, "%s: status %d\n", __func__, status); - - return status; -} -EXPORT_SYMBOL_GPL(gpiod_sysfs_set_active_low); - -/** * gpiod_unexport - reverse effect of gpio_export() * @gpio: gpio to make unavailable * diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 9bb0d11729c9..40ec1433f05d 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -128,11 +128,6 @@ static inline int gpio_export_link(struct device *dev, const char *name, return gpiod_export_link(dev, name, gpio_to_desc(gpio)); } -static inline int gpio_sysfs_set_active_low(unsigned gpio, int value) -{ - return gpiod_sysfs_set_active_low(gpio_to_desc(gpio), value); -} - static inline void gpio_unexport(unsigned gpio) { gpiod_unexport(gpio_to_desc(gpio)); diff --git a/include/linux/gpio.h b/include/linux/gpio.h index ab81339a8590..d12b5d566e4b 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -196,13 +196,6 @@ static inline int gpio_export_link(struct device *dev, const char *name, return -EINVAL; } -static inline int gpio_sysfs_set_active_low(unsigned gpio, int value) -{ - /* GPIO can never have been requested */ - WARN_ON(1); - return -EINVAL; -} - static inline void gpio_unexport(unsigned gpio) { /* GPIO can never have been exported */ diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 3a7c9ffd5ab9..09a7fb0062a6 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -449,7 +449,6 @@ static inline int desc_to_gpio(const struct gpio_desc *desc) int gpiod_export(struct gpio_desc *desc, bool direction_may_change); int gpiod_export_link(struct device *dev, const char *name, struct gpio_desc *desc); -int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value); void gpiod_unexport(struct gpio_desc *desc); #else /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */ @@ -466,11 +465,6 @@ static inline int gpiod_export_link(struct device *dev, const char *name, return -ENOSYS; } -static inline int gpiod_sysfs_set_active_low(struct gpio_desc *desc, int value) -{ - return -ENOSYS; -} - static inline void gpiod_unexport(struct gpio_desc *desc) { }
Remove gpiod_sysfs_set_active_low (and gpio_sysfs_set_active_low) which allowed code to change the polarity of a gpio line even after it had been exported through sysfs. Drivers should not care, and generally does not know, about gpio-line polarity which is a hardware feature that needs to be described by firmware. It is currently possible to define gpio-line polarity in device-tree and acpi firmware or using platform data. Userspace can also change the polarity through sysfs. Note that drivers using the legacy gpio interface could still use GPIOF_ACTIVE_LOW to change the polarity before exporting the gpio. There are no in-kernel users of this interface. Cc: Jonathan Corbet <corbet@lwn.net> Cc: Harry Wei <harryxiyou@gmail.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: linux-doc@vger.kernel.org Cc: linux-kernel@zh-kernel.org Cc: linux-arch@vger.kernel.org Signed-off-by: Johan Hovold <johan@kernel.org> --- Documentation/gpio/gpio-legacy.txt | 9 ------- Documentation/gpio/sysfs.txt | 8 ------- Documentation/zh_CN/gpio.txt | 8 ------- drivers/gpio/gpiolib-sysfs.c | 48 ++------------------------------------ include/asm-generic/gpio.h | 5 ---- include/linux/gpio.h | 7 ------ include/linux/gpio/consumer.h | 6 ----- 7 files changed, 2 insertions(+), 89 deletions(-)