Messages in this thread |  | | Date | Tue, 8 Apr 2008 08:02:30 +0200 | From | Uwe Kleine-König <> | Subject | Re: gpio patches in mmotm |
| |
Hello Guennadi,
Guennadi Liakhovetski wrote: > Please, do not trim the CC: list. I've also added lkml. Oh, thanks. I thought I'm used to hitting reply-to-all 8-(. I also added Andrew back (even though adding lkml might be just as good. :-))
> On Tue, 18 Mar 2008, Uwe Kleine-KЖnig wrote: > > Guennadi Liakhovetski wrote: > > > On Mon, 17 Mar 2008, Uwe Kleine-KЖnig wrote: > > > > > > > I'm nure sure if I like gpio_is_valid(). When do you think it should be > > > > used? (i.e. in which situations gpio_request doesn't do the right > > > > thing?) > > > > > > For example, in situations similar to what I have in mt9m001 and mt9v022 > > > camera drivers. Those cameras can be built with an i2c gpio extender, > > > which can be used to switch between 8 and 10 bit data bus widths. But that > > > extender is not always available. So, those drivers request a gpio, and if > > > it is not available on the system, the gpio_is_valid() test fails. > > I found your patch, but no tree where it applies. Can you point me to a > > tree where it applies? > > These drivers are currently in the v4l-dvb tree > http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=summary in > the devel branch. OK, when I searched your driver I found the tree, but only looked in the master (=HEAD) branch.
> > Why isn't it enough that gpio_request fails in such a situation? > > 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) { ...
Then you don't need to extend the API. Moreover with your variant the check that gpio is valid must be done twice[1].
For me gpio_is_valid would only make sense if there might be situations where you want to know if a certain GPIO exists but even if it does you won't gpio_request it.
Best regards Uwe
[1] OK, gpio_is_valid and gpio_request might be inline functions, but for "my" architecture it is not. -- Uwe Kleine-König, Software Engineer Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962 -- 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/
|  |