public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Tyser <ptyser@xes-inc.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Grant Likely <grant.likely@secretlab.ca>,
	linux-kernel@vger.kernel.org, Alek Du <alek.du@intel.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	David Brownell <dbrownell@users.sourceforge.net>,
	Eric Miao <eric.y.miao@gmail.com>,
	Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	Joe Perches <joe@perches.com>
Subject: Re: [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction
Date: Tue, 15 Feb 2011 11:05:48 -0600	[thread overview]
Message-ID: <1297789548.965.10101.camel@petert> (raw)
In-Reply-To: <20110215114210.1f1a8470@lxorguk.ukuu.org.uk>

> > Also, in most cases I'd think that the BIOS/U-Boot/firmware should have
> > configured the GPIO pins appropriately, which Linux should inherit in
> > general.  Linux currently inherits GPIO states that were set in firmware
> > when a GPIO is requested, but it doesn't properly report those values
> > via sysfs - that's the only bug I'm trying to fix.
> 
> Yes - however you can't fix it unless you are prepared to admit that the
> gpio has multiple states. At minimum you need to be able to report
> 
> 	input/output/unknown/unavailable
> 
> if you want to generalise it. Otherwise you don't solve the problem
> because you are asking a question that driver cannot answer correctly.

As far as the "unknown" state, I can update the patch to have the logic:
+       if (chip->get_direction) {
+               /* chip->get_direction may sleep */
+               spin_unlock_irqrestore(&gpio_lock, flags);
+               if (chip->get_direction(chip, gpio - chip->base) > 0)
+                       set_bit(FLAG_IS_OUT, &desc->flags);
+               spin_lock_irqsave(&gpio_lock, flags);
+       } else {
+               set_bit(FLAG_IS_UNKNOWN, &desc->flags);
+       }

This would have the side effect of having nearly all GPIO drivers
default to an "unknown" direction until they implement the new
get_direction() function, which I think is an improvement over the
current system where they are all unconditionally shown as "input",
often incorrectly.  Are you OK with this Grant?


For the "unavailable" state, I didn't think it would be necessary.  As
is, if someone calls gpio_request() on an invalid or alt_use pin, they
shouldn't get access to the GPIO, which makes the "unavailable value
moot since they couldn't access the GPIO in the first place.

Changing the logic to allow "unavailable" GPIO pins to be
requested/exported would require larger changes to the code, and
wouldn't provide much benefit without additional changes (eg an alt_func
feature).  So I'd vote to not add support for "unavailable" pins in this
patch and rather wait until someone has a specific use for it, and add
it then.

Peter


  reply	other threads:[~2011-02-15 17:06 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-06 19:54 [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction Peter Tyser
2011-01-06 19:54 ` [PATCH 2/3] gpio: pca953x: Implement get_direction() hook Peter Tyser
2011-01-06 23:16   ` David Brownell
2011-01-06 19:54 ` [PATCH 3/3] gpio: Add support for Intel ICHx/3100/Series[56] GPIO Peter Tyser
2011-01-06 23:12   ` David Brownell
2011-02-14 15:48 ` [PATCH 1/3] gpiolib: Add ability to get GPIO pin direction Peter Tyser
2011-02-14 16:02   ` Grant Likely
2011-02-14 19:14     ` Grant Likely
2011-02-14 20:01       ` Peter Tyser
2011-02-14 17:08   ` Alan Cox
2011-02-14 17:26     ` Grant Likely
2011-02-14 17:39       ` Mark Brown
2011-02-14 17:45       ` Peter Tyser
2011-02-14 18:04         ` Grant Likely
2011-02-14 18:46           ` Peter Tyser
2011-02-14 19:35       ` Alan Cox
2011-02-14 23:35         ` Peter Tyser
2011-02-15 11:42           ` Alan Cox
2011-02-15 17:05             ` Peter Tyser [this message]
2011-02-15 17:19               ` Alan Cox
2011-02-15 17:49                 ` Peter Tyser
2011-02-15 19:41                   ` Alan Cox
2011-02-17  8:06                   ` Uwe Kleine-König
2011-03-06  7:53               ` Grant Likely
2011-02-15 23:55           ` Mark Brown
2011-03-06  7:49         ` Grant Likely

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=1297789548.965.10101.camel@petert \
    --to=ptyser@xes-inc.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=alek.du@intel.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=dbrownell@users.sourceforge.net \
    --cc=eric.y.miao@gmail.com \
    --cc=grant.likely@secretlab.ca \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sameo@linux.intel.com \
    --cc=u.kleine-koenig@pengutronix.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox