All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikita Kiryanov <nikita@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] gpio: omap_gpio: Remove check_gpio() function
Date: Thu, 20 Jun 2013 10:44:43 +0300	[thread overview]
Message-ID: <51C2B2EB.8080300@compulab.co.il> (raw)
In-Reply-To: <CAFRkauAYn3SUOihfYRPXricJnGHZru3d7YiLEKzKRWcaevsT5Q@mail.gmail.com>

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.

      reply	other threads:[~2013-06-20  7:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51C2B2EB.8080300@compulab.co.il \
    --to=nikita@compulab.co.il \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.