* [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function
@ 2013-06-20 1:03 Axel Lin
2013-06-20 5:19 ` Nikita Kiryanov
0 siblings, 1 reply; 4+ messages in thread
From: Axel Lin @ 2013-06-20 1:03 UTC (permalink / raw)
To: u-boot
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;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function
2013-06-20 1:03 [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function Axel Lin
@ 2013-06-20 5:19 ` Nikita Kiryanov
2013-06-20 7:20 ` Axel Lin
0 siblings, 1 reply; 4+ messages in thread
From: Nikita Kiryanov @ 2013-06-20 5:19 UTC (permalink / raw)
To: u-boot
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.
--
Regards,
Nikita.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function
2013-06-20 5:19 ` Nikita Kiryanov
@ 2013-06-20 7:20 ` Axel Lin
2013-06-20 7:44 ` Nikita Kiryanov
0 siblings, 1 reply; 4+ messages in thread
From: Axel Lin @ 2013-06-20 7:20 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function
2013-06-20 7:20 ` Axel Lin
@ 2013-06-20 7:44 ` Nikita Kiryanov
0 siblings, 0 replies; 4+ messages in thread
From: Nikita Kiryanov @ 2013-06-20 7:44 UTC (permalink / raw)
To: u-boot
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
>
--
Regards,
Nikita.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-20 7:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-20 1:03 [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function Axel Lin
2013-06-20 5:19 ` Nikita Kiryanov
2013-06-20 7:20 ` Axel Lin
2013-06-20 7:44 ` Nikita Kiryanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox