lkml.org 
[lkml]   [2008]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: gpio patches in mmotm
On Tue, 8 Apr 2008, Uwe Kleine-König wrote:

> > I'm storing the GPIO number locally, and if the system doesn't have a
> > valid GPIO for me, I'm storing an invalid GPIO number. Then at any time if
> > the GPIO has to be used, I just verify if gpio_is_valid(), and if not,
> > return an error code for this request, but the driver remains otherwise
> > functional.
> OK, so in your driver you have:
>
> if (gpio_is_valid(gpio)) {
> /* We have a data bus switch. */
> ret = gpio_request(gpio, "mt9m001");
> if (ret < 0) {
> dev_err(&mt9m001->client->dev, "Cannot get GPIO %u\n",
> gpio);
> return ret;
> }
> ret = gpio_direction_output(gpio, 0);
> if (ret < 0) {
> ...
>
>
> In my eyes the following is better:
>
> /* Do we have a data bus switch? */
> ret = gpio_request(gpio, "mt9m001");
> if (ret < 0) {
> if (ret != -EINVAL) {
> dev_err(...);
> return ret;
> }
> } else {
> ret = gpio_direction_output(gpio, 0);
> if (ret < 0) {
> ...

Yes, you could do that. But then you have to test either before calling
gpio_set_value_cansleep() or inside it. And the test you have to perform
_is_ the validity check, so, you need it anyway.

> Then you don't need to extend the API. Moreover with your variant the
> check that gpio is valid must be done twice[1].

Actually three times. The one before gpio_free() is not actually needed,
right, it is anyway checked inside. But gpio_set_value_cansleep() doesn't
check, so, it would be rude to call it with an invalid value.

> [1] OK, gpio_is_valid and gpio_request might be inline functions, but
> for "my" architecture it is not.

Which arch is it? As I said, you could simplify the two specific camera
drivers by removing the checks where they are redundant. But on other
occasions the checks have to be done anyway, so, it is not a question of
runtime performance (apart from maybe the difference between calling a
function and executing inline), but just of an extra API member, which you
can have different opinions about:-)

Thanks
Guennadi
---
Guennadi Liakhovetski
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2008-04-08 08:31    [W:0.056 / U:1.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site