[U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function
check_gpio() and gpio_is_valid() are both used to check if a gpio is valid or not. It looks pointless to have both function because we can just call gpio_is_valid() instead of check_gpio(). Thus remove check_gpio() function.
Signed-off-by: Axel Lin axel.lin@ingics.com --- drivers/gpio/omap_gpio.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c index a30d7f0..221acc6 100644 --- a/drivers/gpio/omap_gpio.c +++ b/drivers/gpio/omap_gpio.c @@ -58,15 +58,6 @@ int gpio_is_valid(int gpio) return (gpio >= 0) && (gpio < 192); }
-static int check_gpio(int gpio) -{ - if (!gpio_is_valid(gpio)) { - printf("ERROR : check_gpio: invalid GPIO %d\n", gpio); - return -1; - } - return 0; -} - static void _set_gpio_direction(const struct gpio_bank *bank, int gpio, int is_input) { @@ -142,7 +133,7 @@ int gpio_set_value(unsigned gpio, int value) { const struct gpio_bank *bank;
- if (check_gpio(gpio) < 0) + if (!gpio_is_valid(gpio)) return -1; bank = get_gpio_bank(gpio); _set_gpio_dataout(bank, get_gpio_index(gpio), value); @@ -159,7 +150,7 @@ int gpio_get_value(unsigned gpio) void *reg; int input;
- if (check_gpio(gpio) < 0) + if (!gpio_is_valid(gpio)) return -1; bank = get_gpio_bank(gpio); reg = bank->base; @@ -191,7 +182,7 @@ int gpio_direction_input(unsigned gpio) { const struct gpio_bank *bank;
- if (check_gpio(gpio) < 0) + if (!gpio_is_valid(gpio)) return -1;
bank = get_gpio_bank(gpio); @@ -207,7 +198,7 @@ int gpio_direction_output(unsigned gpio, int value) { const struct gpio_bank *bank;
- if (check_gpio(gpio) < 0) + if (!gpio_is_valid(gpio)) return -1;
bank = get_gpio_bank(gpio); @@ -224,7 +215,7 @@ int gpio_direction_output(unsigned gpio, int value) */ int gpio_request(unsigned gpio, const char *label) { - if (check_gpio(gpio) < 0) + if (!gpio_is_valid(gpio)) return -1;
return 0;
Hi Axel,
On 06/20/2013 04:03 AM, Axel Lin wrote:
check_gpio() and gpio_is_valid() are both used to check if a gpio is valid or not. It looks pointless to have both function because we can just call gpio_is_valid() instead of check_gpio(). Thus remove check_gpio() function.
I don't see the benefit in this patch. The functions clearly exist for different reasons: gpio_is_valid() is part of the gpio api, while check_gpio() exists to attach an error message to a failed validity check without causing code duplication all over omap_gpio.c.
Essentially what you are doing is removing a helpful error message.
2013/6/20 Nikita Kiryanov nikita@compulab.co.il
Hi Axel,
On 06/20/2013 04:03 AM, Axel Lin wrote:
check_gpio() and gpio_is_valid() are both used to check if a gpio is valid or not. It looks pointless to have both function because we can just call gpio_is_valid() instead of check_gpio(). Thus remove check_gpio() function.
I don't see the benefit in this patch. The functions clearly exist for different reasons: gpio_is_valid() is part of the gpio api, while check_gpio() exists to attach an error message to a failed validity check without causing code duplication all over omap_gpio.c.
How about just showing error in gpio_is_valid() and then remove check_gpio()?
int gpio_is_valid(int gpio) { if ((gpio >= 0) && (gpio < 192)) return 1;
printf("ERROR : invalid GPIO %d\n", gpio); return 0; }
Essentially what you are doing is removing a helpful error message.
My intention is to remove redundant code.
Regards, Axel
On 06/20/2013 10:20 AM, Axel Lin wrote:
2013/6/20 Nikita Kiryanov nikita@compulab.co.il
Hi Axel,
On 06/20/2013 04:03 AM, Axel Lin wrote:
check_gpio() and gpio_is_valid() are both used to check if a gpio is valid or not. It looks pointless to have both function because we can just call gpio_is_valid() instead of check_gpio(). Thus remove check_gpio() function.
I don't see the benefit in this patch. The functions clearly exist for different reasons: gpio_is_valid() is part of the gpio api, while check_gpio() exists to attach an error message to a failed validity check without causing code duplication all over omap_gpio.c.
How about just showing error in gpio_is_valid() and then remove check_gpio()?
It is not an error for gpio_is_valid() to report that a gpio is not valid. It is however an error to attempt to use an invalid gpio.
int gpio_is_valid(int gpio) { if ((gpio >= 0) && (gpio < 192)) return 1;
printf("ERROR : invalid GPIO %d\n", gpio); return 0;}
Essentially what you are doing is removing a helpful error message.
My intention is to remove redundant code.
check_gpio() adds upon gpio_is_valid(). If you want to argue redundancy you should explain why this addition is unnecessary.
Regards, Axel
participants (2)
-
Axel Lin -
Nikita Kiryanov