| Message ID | 1455527640-10446-1-git-send-email-alexander.stein@systec-electronic.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Feb 15, 2016 at 10:14 AM, Alexander Stein <alexander.stein@systec-electronic.com> wrote: > At first view I thought this function returned an error, but actually > it is the input direction. Use the define for input which makes reading > the code much easier. > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> NACK that flag is for consumers, not drivers. Drivers have their own API and should ideally return a bool true/false, but that would be another major refactoring.... Yours, Linus Walleij -- 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 Tuesday 16 February 2016 15:55:47, Linus Walleij wrote: > On Mon, Feb 15, 2016 at 10:14 AM, Alexander Stein > <alexander.stein@systec-electronic.com> wrote: > > > At first view I thought this function returned an error, but actually > > it is the input direction. Use the define for input which makes reading > > the code much easier. > > > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> > > NACK that flag is for consumers, not drivers. > Drivers have their own API and should ideally > return a bool true/false, but that would be another major > refactoring.... Well, having a callback get_direction returning a bool would seem really strange. Actually the comments on gpiod_get_direction explicitly state GPIOF_DIR_IN and GPIOF_DIR_OUT as return values which aren't used in that function itself, they come from the callback. Also other dirvers like e.g. gpio-ich return (in ichx_gpio_get_direction) GPIOF_DIR_IN or GPIOF_DIR_OUT. Best regards, Alexander
On Tue, Feb 16, 2016 at 4:35 PM, Alexander Stein <alexander.stein@systec-electronic.com> wrote: > Well, having a callback get_direction returning a bool would seem really strange. Well maybe an enum would be better. Patches accepted. > Actually the comments on gpiod_get_direction explicitly state GPIOF_DIR_IN and GPIOF_DIR_OUT > as return values which aren't used in that function itself, they come from the callback. Yeah so it is the gpiolib core that need to make sure the right value is propagated back to the API, not the driver. It exploits the fact that driver return a suiting 0/1 that happen to correspond to that. > Also other > dirvers like e.g. gpio-ich return (in ichx_gpio_get_direction) GPIOF_DIR_IN or GPIOF_DIR_OUT. Two wrong doesn't make one right. Patch gpio-ich to return 0/1 if you like, or even better: add a direction enum to <linux/gpio/driver.h> and start patching the callback and drivers to use that. Overall this is a thing that "just works" mostly, but I have had many problems with the driver and consumer API being mixed up (mostly by everyone using <linux/gpio.h> and I don't want to encourage that mess. Drivers should use <linux/gpio/driver.h> and nothing else. Consumers should use <linus/gpio/consumer.h> and nothing else. The latter require that they switch to GPIO descriptors though. I think. Yours, Linus Walleij -- 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/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c index 58ea08d..068de54 100644 --- a/drivers/gpio/gpio-pisosr.c +++ b/drivers/gpio/gpio-pisosr.c @@ -13,6 +13,7 @@ */ #include <linux/delay.h> +#include <linux/gpio.h> #include <linux/gpio/consumer.h> #include <linux/gpio/driver.h> #include <linux/module.h> @@ -65,7 +66,7 @@ static int pisosr_gpio_get_direction(struct gpio_chip *chip, unsigned offset) { /* This device always input */ - return 1; + return GPIOF_DIR_IN; } static int pisosr_gpio_direction_input(struct gpio_chip *chip,
At first view I thought this function returned an error, but actually it is the input direction. Use the define for input which makes reading the code much easier. Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> --- drivers/gpio/gpio-pisosr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)