devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Courbot <acourbot@nvidia.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Grant Likely <grant.likely@secretlab.ca>,
	Linus Walleij <linus.walleij@linaro.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree-discuss@lists.ozlabs.org"
	<devicetree-discuss@lists.ozlabs.org>
Subject: Re: [PATCH 0/4] gpio: introduce descriptor-based interface
Date: Thu, 10 Jan 2013 13:07:36 +0900	[thread overview]
Message-ID: <2270940.DTxEYMZtED@percival> (raw)
In-Reply-To: <201301091046.13020.arnd@arndb.de>

On Wednesday 09 January 2013 18:46:12 Arnd Bergmann wrote:
> > The question is, do we want to totally get rid of the integer
> > namespace? That would be the ultimate step, but would require another
> > way to identify GPIOs (controller_device:offset might be such a way),
> > and also to reorganize sysfs nodes. Wouldn't that be considered
> > breaking user-space? 'cause we all know what happens to those who
> > break user-space.
> 
> 
> The user interface could eventually be the only part of the kernel that
> uses the numbers, but you are right that we cannot change that.

That's sad, as it makes it necessary to maintain the global integer namespace 
(assigning a base GPIO to each controller and making sure the controllers 
ranges do no overlap) even if it is not used internally anymore. We could make 
global numbers assignment transparent, but that would potentially change the 
GPIO numbers in user-space and cause another incompatibility.

> > With the current code, a driver should depend on gpiolib being
> > compiled if it uses the new interface. It is not even declared if
> > gpiolib is not used.
> > 
> > Given that both interfaces are quite close, one could imagine having a
> > gpiod wrapper around the integer namespace (the "opaque descriptors"
> > would then just be casted integers). This way drivers would only need
> > to depend on GENERIC_GPIO. It's a little bit weird to have gpiod
> > wrapping around gpio in one case and the opposite in another though -
> > I'd rather have these platforms convert to GPIO descriptors internally
> > (or even better, to gpiolib), but this is probably asking too much.
> 
> 
> I think it would be reasonable to force everybody to use gpiolib,
> that's much easier than converting everyone to the descriptor based
> interface.
> 
> 
> > I do not know all the details of gpiolib's history, but why would
> > anyone want to implement the generic gpio interface and not use
> > gpiolib anyways?
> 
> 
> Only legacy users did this. Initially there was only the header file,
> with the API declared but several different implementations of it.
> gpiolib was introduced later to reduce code duplication and allow having
> multiple implementations in the same kernel.

Does the following sound reasonable?
1) Make sure every target that uses GENERIC_GPIO also implements its drivers 
using gpiolib, convert the (hopefully) few ones that don't to use gpiolib
2) Make GENERIC_GPIO require GPIOLIB or just merge both options into a single 
one
3) Turn gpio into a full subsystem (like pinctrl)

This should make things less blurry and easier to maintain (less header files, 
only one interface, etc.) GPIO controllers would also be better integrated 
into the driver model.

> > > If we are adding a new way to deal with GPIOs, would it make sense to
> > > have that more closely integrated into pinctrl in one form or another?
> > > My feeling is that there is already a significant overlap between the
> > > two, and while the addition of the gpiod_* functions doesn't
> > > necessarily
> > > make this worse, it could be a chance to improve the current situation
> > > to make the interface more consistent with pinctrl.
> > 
> > 
> > That may be a chance to introduce deeper changes indeed - what do you
> > have in mind exactly?
> 
> 
> I don't know enough about pinctrl to have a specific idea yet, but maybe
> someone else has ideas.

I had a deeper look at pinctrl, and indeed I can see the connection between 
the two. There already interfaces to link GPIO ranges to pin ranges and have 
GPIO drivers switch the pin to the correct state when a GPIO is requested 
(this, btw, should also be updated to not use global GPIO numbers at some 
point). Maybe some tighter integration that I just don't see yet can be done 
too.

> Regarding the integration of pinctrl with gpio,
> I was thinking in the past that we could make pinctrl provide everything
> that gpiolib does, and have a generic gpiolib driver on top of pinctrl
> so that platforms don't need to implement both interfaces but only need
> to provide a pure pinctrl driver. Not sure if this makes any sense.

That would work if all GPIOs were connected to a ball, but how about GPIO 
expanders that are external to the chip? They have no use for pinctrl AFAICT. 
On the other hand, maybe we can have one pinctrl-gpio driver for those chips 
where pinctrl alone can emulate all the functionality of a GPIO controller. 
Maybe such a driver exists already?

But in general, I agree pinctrl should be a source of inspiration for how to 
design GPIO. In particular, having a per-chip integer namespace instead of a 
single global one is definitely something to take (and that's how things work 
in the DT already).

Alex.

  reply	other threads:[~2013-01-10  4:07 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-08  7:18 [PATCH 0/4] gpio: introduce descriptor-based interface Alexandre Courbot
2013-01-08  7:18 ` [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface Alexandre Courbot
2013-01-08 12:59   ` Arnd Bergmann
2013-01-09  1:06     ` Alexandre Courbot
2013-01-09 10:25       ` Russell King - ARM Linux
2013-01-09 10:35       ` Arnd Bergmann
2013-01-09 10:44         ` Russell King - ARM Linux
2013-01-09 11:10           ` Russell King - ARM Linux
     [not found]             ` <20130109111055.GG3931-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2013-01-09 11:52               ` Arnd Bergmann
2013-01-09 14:44             ` Nicolas Pitre
2013-01-09 15:04               ` [PATCH] Proposed removal of IS_ERR_OR_NULL() (was: Re: [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface) Russell King - ARM Linux
     [not found]                 ` <20130109150427.GL3931-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2013-01-09 15:21                   ` Grant Likely
2013-01-09 15:26                     ` Arnd Bergmann
2013-01-09 15:27                 ` Nicolas Pitre
2013-01-09 15:51                   ` Russell King - ARM Linux
2013-01-09 16:09                     ` Nicolas Pitre
2013-01-09 16:21                       ` Russell King - ARM Linux
2013-01-09 17:12                         ` Russell King - ARM Linux
2013-01-09 17:52                           ` Tony Lindgren
2013-01-17 10:28                 ` Linus Walleij
2013-01-10  8:36             ` [PATCH 1/4] gpiolib: introduce descriptor-based GPIO interface Thierry Reding
2013-01-08  7:18 ` [PATCH 2/4] gpiolib: add gpiod_get and gpiod_put functions Alexandre Courbot
2013-01-08 13:07   ` Arnd Bergmann
2013-01-09  1:49     ` Alexandre Courbot
2013-01-08  7:18 ` [PATCH 3/4] gpiolib: of: convert OF helpers to descriptor API Alexandre Courbot
2013-01-08  7:18 ` [PATCH 4/4] gpiolib: add documentation for new gpiod_ API Alexandre Courbot
2013-01-08 13:06 ` [PATCH 0/4] gpio: introduce descriptor-based interface Arnd Bergmann
2013-01-09  1:48   ` Alexandre Courbot
2013-01-09 10:46     ` Arnd Bergmann
2013-01-10  4:07       ` Alex Courbot [this message]
2013-01-10 10:08         ` Arnd Bergmann
2013-01-14 10:21           ` Alex Courbot
2013-01-14 10:46             ` Arnd Bergmann
2013-01-17 11:15           ` Linus Walleij
2013-01-17 12:02             ` Greg Ungerer
2013-01-17 16:50               ` Steven King
2013-01-17 19:22                 ` Arnd Bergmann
2013-01-20  6:07                 ` Alex Courbot
2013-01-22  8:55                   ` Linus Walleij
2013-01-17 11:25           ` Linus Walleij

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=2270940.DTxEYMZtED@percival \
    --to=acourbot@nvidia.com \
    --cc=arnd@arndb.de \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=grant.likely@secretlab.ca \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /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;
as well as URLs for NNTP newsgroup(s).