Message-ID: <20130522094958.GA4789@ab42.lan>
Date:	Wed, 22 May 2013 11:49:59 +0200
From:	Christian Ruppert <christian.ruppert@...lis.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	Stephen Warren <swarren@...dotorg.org>,
	Patrice CHOTARD <patrice.chotard@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	Sascha Leuenberger <sascha.leuenberger@...lis.com>,
	Pierrick Hascoet <pierrick.hascoet@...lis.com>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Alexandre Courbot <acourbot@...dia.com>
Subject: Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver

On Mon, May 20, 2013 at 10:03:24AM +0200, Linus Walleij wrote:
> On Wed, May 15, 2013 at 11:41 AM, Christian Ruppert
> <christian.ruppert@...lis.com> wrote:
> > On Tue, May 14, 2013 at 02:29:46PM +0200, Linus Walleij wrote:
> 
> >> Look at for example
> >> drivers/pinctrl/pinctrl-abx500.c:
> >>
> >> static u8 abx500_get_mode(struct pinctrl_dev *pctldev, struct gpio_chip *chip,
> >>                           unsigned gpio)
> >> {
> >> (...)
> >>         /* on ABx5xx, there is no GPIO0, so adjust the offset */
> >>         unsigned offset = gpio - 1;
> >>
> >> As you can see, this driver, which does not use device tree,
> >> is working around the same problem. Here the problem is that
> >> the pins are numbered starting at 1 instead of 0, a very trivial
> >> numberspace shuffleing.
> >
> > Let me see if I understand this right:
> > In ABx5xx, the pin numbering is offset by 1 wrt. GPIO numbering?
> 
> No, sorry that the code is strange... The variable is named "gpio"
> because that is the numbering used in the data sheet.
> 
> Think of "gpio" as "the number in the data sheet".
> 
> Then offset is calculated to start from 0.

OK, seems like I thoroughly misunderstood what you were saying and drew
some wrong conclusions from that. In order to avoid confusion I'll
delete everything related to this misunderstanding below.

> [...]
> If you mean that you try to map the global GPIO number space
> 1:1 on top of what your datasheet has, just *don't do that*.
> Because we want to get rid of this global GPIO numberspace.
> Alexandre is already working hard on this!

Thanks for the hint. Defining a global GPIO number space in the data
sheet was one of our ideas but I agree it would be preferable to do away
with this completely and rather deal with GPIOs on a bank level.

> >  - The custom logic inside many pinctrl drivers would be confined in one
> >    translation function the driver provides instead of being spread out
> >    all over the driver.
> >  - "Sparse GPIO ranges" are easy to implement if required by the
> >    platform/driver.
> 
> This looks good.

I think these advantages could be maintained through two alternative
methods:
1) Extend the GPIO ranges mechanism to allow for pin groups in addition
   to contiguous pin ranges. This would be my preferred method since no
   pin numbers are exposed to the user at all (neither kernel internal
   nor physical). All pin numbering remains inside the driver. This
   would be a clean version of what I did in the original patch.
2) Add a physical-to-Linux-internal pin number translation callback to
   the pinmux driver core interface. Map 1-to-1 if this callback is not
   defined to maintain backwards compatibility. I don't like this
   solution as much as 1) for the following reasons:
     . It adds yet another number space to confuse things even more.
     . Managing contiguous ranges becomes more difficult because ranges
       contiguous in one number space (physical pins, Linux pins, GPIOs)
       are not necessarily contiguous in the others, requiring more
       complex translation mechanisms.
     . Some chips have matrix like number schemes (A1, A2, ..., B1, B1,
       ...) etc. which cannot be directly addressed with this and would
       need to introduce yet another layer of indirection.
     . In cases where different package options need to be supported, a
       translation table is required for each inside the driver. For
       modern SOCs, each of these tables can easily be a few kB in size
       and I don't know if we want to add that amount of static data to
       the kernel, esp. because all but one of the tables are unused.

I don't know ACPI and one of the two solutions might not be applicable
there.

> [...]
> We need to iterate this discussion a few turns until I actually
> understand your problem I think.

I agree. Let me resume my high level constraints again:
- Align customer visible interfaces (such as device tree and
  sys/class/gpio) with our hardware documentation.
- Documentation can be augmented (e.g. a global GPIO number space
  _could_ be added) but fundamentals (such as physical pin numbering,
  our GPIO bank structure) must not be changed.
- Several distinct naming schemes for the same thing (such as pins) are
  only allowed if it is obvious to which naming scheme a given name
  belongs. E.g. pins may have both a name (string) and a number (and
  they actually have) but it is not allowed to have two different
  numbers designating the same pin.

Greetings,
  Christian

-- 
  Christian Ruppert              ,          <christian.ruppert@...lis.com>
                                /|
  Tel: +41/(0)22 816 19-42     //|                 3, Chemin du Pré-Fleuri
                             _// | bilis Systems   CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists