Skip to contentSkip to conversation

gpio: omap_gpio: Remove check_gpio() function

PATCH New Axel Lin 2 participants 4 messages Jun 20, 2013 GPIO

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
---
 drivers/gpio/omap_gpio.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)
drivers/gpio/omap_gpio.c +5 -14
index a30d7f0..221acc6 100644
@@ -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;
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH 1/1 gpio: omap_gpio: Remove check_gpio() function

Hi Axel,

Quoted 3 lines · Axel Lin

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.

Signature · 2 lines
Regards,
Nikita.
author
#2 lore raw

PATCH 1/1 gpio: omap_gpio: Remove check_gpio() function

2013/6/20 Nikita Kiryanov <[email protected]>

Quoted 12 lines

Hi Axel,

Quoted 6 lines · Axel Lin

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.

Quoted 6 lines

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;
}
Quoted 1 line

Essentially what you are doing is removing a helpful error message.

My intention is to remove redundant code.

Regards,
Axel

#3 lore raw

PATCH 1/1 gpio: omap_gpio: Remove check_gpio() function

Quoted 21 lines · Axel Lin

2013/6/20 Nikita Kiryanov <[email protected]>

Quoted 12 lines

Hi Axel,

Quoted 6 lines · Axel Lin

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.

Quoted 6 lines

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.

Quoted 2 lines

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.

Quoted 11 lines

int gpio_is_valid(int gpio)

{
         if ((gpio >= 0) && (gpio < 192))
                 return 1;

         printf("ERROR : invalid GPIO %d\n", gpio);
         return 0;
}
Quoted 3 lines
Quoted 1 line

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.

Quoted 4 lines

Regards,
Axel

Signature · 2 lines
Regards,
Nikita.