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
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/

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