On Fri, Jun 7, 2013 at 4:10 PM, Linus Walleij <linus.wall...@linaro.org> wrote: > Well it is designed as a sysfs-only thing according to the comment: > drivers/gpio/gpiolib.c:#define FLAG_ACTIVE_LOW 6 /* sysfs value > has active low */ > > So I think your actual question is whether it should also be > enabled for the kernel-internal interfaces.
Yes, that's my question. > Currently we have things like this: > > /** > * struct mmci_platform_data - platform configuration for the MMCI > * (also known as PL180) block. > (...) > * @gpio_cd: read this GPIO pin to detect card insertion > * @cd_invert: true if the gpio_cd pin value is active low > (...) > int gpio_cd; > bool cd_invert; > > So the knowledge of whether a certain GPIO is active high or low > is spread out through drivers, and the API only drives the line in a > very explicit way. > > I am uncertain whether it is wise to move this into gpiolib for > in-kernel consumers, it is mainly a convention of where the > knowledge should sit. In the mentioned case, the flags will still > be needed because we need to have the information there to > set the flag toward the gpiolib API anyway. > > So the only thing gained is that gpiolib gets some knowledge > of how the pin is used. But what is the gain of that? Well, my problem is clearly highlighted by the example you mention, which is a very common usage pattern especially for DT-obtained GPIOs which can have this OF_GPIO_ACTIVE_LOW flag. E.g. we have a lot of this: gpio = of_get_named_gpio_flags(np, "foo-gpios", 0, &flags); active_low = flags & OF_GPIO_ACTIVE_LOW ? 1 : 0; ... state = (gpio_get_value(gpio) ? 1 : 0) ^ active_low; ... gpio_set_value(gpio, (value ? 1 : 0) ^ active_low; which could be simplified into this: gpio = of_get_named_gpiod_flags(np, "foo-gpios", 0); ... state = gpiod_get_value(gpio); ... gpiod_set_value(gpio, value); Board design makes it possible to turn any GPIO active low for any (good or bad) reason, thus the drivers, which interact with gpiolib, are more likely to want to set the GPIO logical value (taking the active low parameter into account) rather than its raw level. In the end, drivers that are not using an active_low flag are most likely doing so just because they never encountered such a situation. In practice, I suspect nearly all drivers want to be aware of the active-low property. Since gpiolib is already half-aware of this property, through the sysfs active_low node and the OF_GPIO_ACTIVE_LOW DT property, why not complete the circle and ease the life of driver writers? As Stephen wrote, an alternative set of functions to get/set the actual raw value of the line could be provided, but my understanding is that an overwhelming majority of GPIO users would benefit from having the active low property being handled transparently. Alex. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/