All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Pinmux subsystem
@ 2011-05-02 19:16 ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-02 19:16 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Grant Likely, Lee Jones, Martin Persson, Linus Walleij

From: Linus Walleij <linus.walleij@linaro.org>

This patchset creates a pinmux subsystem and switches U300 to use that new
subsystem as an example. The problem is not that fantastically hard to
solve in a general way, nobody got around to it because it requires some
upfront code I believe, and this is my stab at it.

For details about what pinmuxing is see the Documentation/pinmux.txt file
in the patchset.

The problem is not ARM specific at all, I haven't looked around deeply but I
expect the same type of hacks to exist in most embedded hardware.

This patchset goes on top of my previous gpio consolidation patches that
bring the U300 GPIO driver into drivers/gpio.

If there is nothing fundamentally insane about the design I will likely
attempt to optimistically push it for 2.6.40 as part of the ARM depopulation
work.

Linus Walleij (4):
  drivers: create a pinmux subsystem
  pinmux: add a driver for the U300 pinmux
  amba: request muxing for PrimeCell devices
  pinmux: activate pinmux driver, delete old padmux driver

 Documentation/ABI/testing/sysfs-class-pinmux |   11 +
 Documentation/pinmux.txt                     |  306 +++++++++++
 MAINTAINERS                                  |    5 +
 arch/arm/mach-u300/Kconfig                   |    2 +
 arch/arm/mach-u300/Makefile                  |    2 +-
 arch/arm/mach-u300/core.c                    |   30 +-
 arch/arm/mach-u300/include/mach/pinmux.h     |    2 +
 arch/arm/mach-u300/include/mach/syscon.h     |  136 -----
 arch/arm/mach-u300/mmc.c                     |   16 -
 arch/arm/mach-u300/padmux.c                  |  367 -------------
 arch/arm/mach-u300/padmux.h                  |   39 --
 arch/arm/mach-u300/spi.c                     |   20 -
 drivers/Kconfig                              |    4 +
 drivers/Makefile                             |    2 +
 drivers/amba/bus.c                           |   49 ++-
 drivers/pinmux/Kconfig                       |   32 ++
 drivers/pinmux/Makefile                      |    6 +
 drivers/pinmux/core.c                        |  732 ++++++++++++++++++++++++++
 drivers/pinmux/pinmux-u300.c                 |  361 +++++++++++++
 drivers/pinmux/pinmux-u300.h                 |  141 +++++
 include/linux/amba/bus.h                     |    2 +
 include/linux/pinmux.h                       |  217 ++++++++
 22 files changed, 1899 insertions(+), 583 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-pinmux
 create mode 100644 Documentation/pinmux.txt
 create mode 100644 arch/arm/mach-u300/include/mach/pinmux.h
 delete mode 100644 arch/arm/mach-u300/padmux.c
 delete mode 100644 arch/arm/mach-u300/padmux.h
 create mode 100644 drivers/pinmux/Kconfig
 create mode 100644 drivers/pinmux/Makefile
 create mode 100644 drivers/pinmux/core.c
 create mode 100644 drivers/pinmux/pinmux-u300.c
 create mode 100644 drivers/pinmux/pinmux-u300.h
 create mode 100644 include/linux/pinmux.h

-- 
1.7.3.2


^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-02 19:16 ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-02 19:16 UTC (permalink / raw)
  To: linux-arm-kernel

From: Linus Walleij <linus.walleij@linaro.org>

This patchset creates a pinmux subsystem and switches U300 to use that new
subsystem as an example. The problem is not that fantastically hard to
solve in a general way, nobody got around to it because it requires some
upfront code I believe, and this is my stab at it.

For details about what pinmuxing is see the Documentation/pinmux.txt file
in the patchset.

The problem is not ARM specific at all, I haven't looked around deeply but I
expect the same type of hacks to exist in most embedded hardware.

This patchset goes on top of my previous gpio consolidation patches that
bring the U300 GPIO driver into drivers/gpio.

If there is nothing fundamentally insane about the design I will likely
attempt to optimistically push it for 2.6.40 as part of the ARM depopulation
work.

Linus Walleij (4):
  drivers: create a pinmux subsystem
  pinmux: add a driver for the U300 pinmux
  amba: request muxing for PrimeCell devices
  pinmux: activate pinmux driver, delete old padmux driver

 Documentation/ABI/testing/sysfs-class-pinmux |   11 +
 Documentation/pinmux.txt                     |  306 +++++++++++
 MAINTAINERS                                  |    5 +
 arch/arm/mach-u300/Kconfig                   |    2 +
 arch/arm/mach-u300/Makefile                  |    2 +-
 arch/arm/mach-u300/core.c                    |   30 +-
 arch/arm/mach-u300/include/mach/pinmux.h     |    2 +
 arch/arm/mach-u300/include/mach/syscon.h     |  136 -----
 arch/arm/mach-u300/mmc.c                     |   16 -
 arch/arm/mach-u300/padmux.c                  |  367 -------------
 arch/arm/mach-u300/padmux.h                  |   39 --
 arch/arm/mach-u300/spi.c                     |   20 -
 drivers/Kconfig                              |    4 +
 drivers/Makefile                             |    2 +
 drivers/amba/bus.c                           |   49 ++-
 drivers/pinmux/Kconfig                       |   32 ++
 drivers/pinmux/Makefile                      |    6 +
 drivers/pinmux/core.c                        |  732 ++++++++++++++++++++++++++
 drivers/pinmux/pinmux-u300.c                 |  361 +++++++++++++
 drivers/pinmux/pinmux-u300.h                 |  141 +++++
 include/linux/amba/bus.h                     |    2 +
 include/linux/pinmux.h                       |  217 ++++++++
 22 files changed, 1899 insertions(+), 583 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-pinmux
 create mode 100644 Documentation/pinmux.txt
 create mode 100644 arch/arm/mach-u300/include/mach/pinmux.h
 delete mode 100644 arch/arm/mach-u300/padmux.c
 delete mode 100644 arch/arm/mach-u300/padmux.h
 create mode 100644 drivers/pinmux/Kconfig
 create mode 100644 drivers/pinmux/Makefile
 create mode 100644 drivers/pinmux/core.c
 create mode 100644 drivers/pinmux/pinmux-u300.c
 create mode 100644 drivers/pinmux/pinmux-u300.h
 create mode 100644 include/linux/pinmux.h

-- 
1.7.3.2

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-02 19:16 ` Linus Walleij
@ 2011-05-02 22:57   ` Russell King - ARM Linux
  -1 siblings, 0 replies; 48+ messages in thread
From: Russell King - ARM Linux @ 2011-05-02 22:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, Grant Likely, Martin Persson,
	Linus Walleij, Lee Jones

On Mon, May 02, 2011 at 09:16:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This patchset creates a pinmux subsystem and switches U300 to use that new
> subsystem as an example. The problem is not that fantastically hard to
> solve in a general way, nobody got around to it because it requires some
> upfront code I believe, and this is my stab at it.

While I like the idea of consolidating the pinmuxing stuff, I'm not sure
about having a struct pin_mux pointer in each bus types device structure.
It'd mean this would have to be added to platform devices as well...

Then there's SA1100 (and PXA?) to consider with its IrDA setup, where its
necessary to switch the pin muxing during driver operation, when switching
between SIR and FIR modes (SIR uses the UART, FIR uses a separate hardware
block.)

So any per-device pinmuxing subsystem also needs a way that a driver can
change the pinmuxing of its associated pins on the fly too.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-02 22:57   ` Russell King - ARM Linux
  0 siblings, 0 replies; 48+ messages in thread
From: Russell King - ARM Linux @ 2011-05-02 22:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 02, 2011 at 09:16:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This patchset creates a pinmux subsystem and switches U300 to use that new
> subsystem as an example. The problem is not that fantastically hard to
> solve in a general way, nobody got around to it because it requires some
> upfront code I believe, and this is my stab at it.

While I like the idea of consolidating the pinmuxing stuff, I'm not sure
about having a struct pin_mux pointer in each bus types device structure.
It'd mean this would have to be added to platform devices as well...

Then there's SA1100 (and PXA?) to consider with its IrDA setup, where its
necessary to switch the pin muxing during driver operation, when switching
between SIR and FIR modes (SIR uses the UART, FIR uses a separate hardware
block.)

So any per-device pinmuxing subsystem also needs a way that a driver can
change the pinmuxing of its associated pins on the fly too.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-02 19:16 ` Linus Walleij
@ 2011-05-03 17:27   ` Andrew Lunn
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-03 17:27 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, Grant Likely, Martin Persson,
	Linus Walleij, Lee Jones

On Mon, May 02, 2011 at 09:16:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This patchset creates a pinmux subsystem and switches U300 to use that new
> subsystem as an example. The problem is not that fantastically hard to
> solve in a general way, nobody got around to it because it requires some
> upfront code I believe, and this is my stab at it.

Hi Linus

I have some questions about how you see the following situations been
solved.

Say i have a UART core. I can be used as a 3-wire serial port, or
additionally it can have hardware flow control, or additionally it can
have all the modem signals. What would i expect to find in the pinmux
driver?

I can think of two different solutions: 

1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem.  The
   first just has 2 pins, the second 4 and the last 8. The board code
   selects one of these for the serial driver to use.

2) Three functions: uart-core, uart-hw-flow, uart-mode.  The first has
   2 pins, the second has 2 pins and the last 4 pins. The board code tells
   the driver to use uart-core, plus say uart-hw-flow.

Do you think the documentation should have guidelines how best to do
this sort of core + additional optional extras?

Say i have a SoC with an SPI core. This core has the usual MISC, MOSI
and SCLK. It additionally has 4 chip select lines. My board uses chip
select lines 2 and 3 for SPI, and select lines 0 and 1 as GPIO lines.
How does the pinmux driver handle this? Again, it is the problems of a
few core pins plus additional optional extras.

Maybe the documentation should make some recommendations about what is
placed into the drivers/pinmux/pinmux-foo.c and what should be kept in
the board specific code? Without these guidelines, it seems to me,
each board for a given SoC is going to add its own peculiar pin
combination to the drivers/pinmux/pinmux-foo.c file.  Maybe it makes
more sense to have a collection of standard pin functions in
drivers/pinmux/pinmux-foo.c, which cover 75% of the likely
combinations. And recommend a board file to have its own additional
list of strange pin functions which it registers with the pinmux core?

     Thanks
        Andrew

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-03 17:27   ` Andrew Lunn
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-03 17:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 02, 2011 at 09:16:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This patchset creates a pinmux subsystem and switches U300 to use that new
> subsystem as an example. The problem is not that fantastically hard to
> solve in a general way, nobody got around to it because it requires some
> upfront code I believe, and this is my stab at it.

Hi Linus

I have some questions about how you see the following situations been
solved.

Say i have a UART core. I can be used as a 3-wire serial port, or
additionally it can have hardware flow control, or additionally it can
have all the modem signals. What would i expect to find in the pinmux
driver?

I can think of two different solutions: 

1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem.  The
   first just has 2 pins, the second 4 and the last 8. The board code
   selects one of these for the serial driver to use.

2) Three functions: uart-core, uart-hw-flow, uart-mode.  The first has
   2 pins, the second has 2 pins and the last 4 pins. The board code tells
   the driver to use uart-core, plus say uart-hw-flow.

Do you think the documentation should have guidelines how best to do
this sort of core + additional optional extras?

Say i have a SoC with an SPI core. This core has the usual MISC, MOSI
and SCLK. It additionally has 4 chip select lines. My board uses chip
select lines 2 and 3 for SPI, and select lines 0 and 1 as GPIO lines.
How does the pinmux driver handle this? Again, it is the problems of a
few core pins plus additional optional extras.

Maybe the documentation should make some recommendations about what is
placed into the drivers/pinmux/pinmux-foo.c and what should be kept in
the board specific code? Without these guidelines, it seems to me,
each board for a given SoC is going to add its own peculiar pin
combination to the drivers/pinmux/pinmux-foo.c file.  Maybe it makes
more sense to have a collection of standard pin functions in
drivers/pinmux/pinmux-foo.c, which cover 75% of the likely
combinations. And recommend a board file to have its own additional
list of strange pin functions which it registers with the pinmux core?

     Thanks
        Andrew

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-03 17:27   ` Andrew Lunn
@ 2011-05-03 19:29     ` Valdis.Kletnieks at vt.edu
  -1 siblings, 0 replies; 48+ messages in thread
From: Valdis.Kletnieks @ 2011-05-03 19:29 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Linus Walleij, linux-kernel, linux-arm-kernel, Grant Likely,
	Martin Persson, Linus Walleij, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Tue, 03 May 2011 19:27:12 +0200, Andrew Lunn said:

> I can think of two different solutions: 
> 
> 1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem.  The
>    first just has 2 pins, the second 4 and the last 8. The board code
>    selects one of these for the serial driver to use.
> 
> 2) Three functions: uart-core, uart-hw-flow, uart-mode.  The first has
>    2 pins, the second has 2 pins and the last 4 pins. The board code tells
>    the driver to use uart-core, plus say uart-hw-flow.

For this second solution, what happens if some bozo selects *only* uart-modem
but not uart-core?  If there's a strict ordering (hw-flow requires core, modem
requires both hw-flow and core), it's essentially the same thing as the first
solution.  If there's not a strict ordering (i.e you can select uart-modem
without uart-core), you need to come up with sane semantics.


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-03 19:29     ` Valdis.Kletnieks at vt.edu
  0 siblings, 0 replies; 48+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2011-05-03 19:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 03 May 2011 19:27:12 +0200, Andrew Lunn said:

> I can think of two different solutions: 
> 
> 1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem.  The
>    first just has 2 pins, the second 4 and the last 8. The board code
>    selects one of these for the serial driver to use.
> 
> 2) Three functions: uart-core, uart-hw-flow, uart-mode.  The first has
>    2 pins, the second has 2 pins and the last 4 pins. The board code tells
>    the driver to use uart-core, plus say uart-hw-flow.

For this second solution, what happens if some bozo selects *only* uart-modem
but not uart-core?  If there's a strict ordering (hw-flow requires core, modem
requires both hw-flow and core), it's essentially the same thing as the first
solution.  If there's not a strict ordering (i.e you can select uart-modem
without uart-core), you need to come up with sane semantics.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110503/ccb5316f/attachment.sig>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-02 22:57   ` Russell King - ARM Linux
@ 2011-05-10 21:25     ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-10 21:25 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-kernel, Grant Likely, Martin Persson, Lee Jones, linux-arm-kernel

Finally I'm back at the pinmux patch set trying to answer all questions
accumulated:

2011/5/3 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> While I like the idea of consolidating the pinmuxing stuff, I'm not sure
> about having a struct pin_mux pointer in each bus types device structure.
> It'd mean this would have to be added to platform devices as well...

Yeah I'm not sure about the patch to the AMBA/PrimeCell bus really,
an alternative is for all drivers to request their mux individually. But in
the AMBA bus we already request the silicon block clock and the
silicon power switch so it seemed natural to also request a mux setting,
if there is one.

> Then there's SA1100 (and PXA?) to consider with its IrDA setup, where its
> necessary to switch the pin muxing during driver operation, when switching
> between SIR and FIR modes (SIR uses the UART, FIR uses a separate hardware
> block.)
>
> So any per-device pinmuxing subsystem also needs a way that a driver can
> change the pinmuxing of its associated pins on the fly too.

It can, basically:

struct pinmux *pmx;

pmx = pinmux_get(dev, "irda-uart");
pinmux_enable(pmx);
(... SIR UART operations ...)
pinmux_disable(pmx);
pinmux_put(pmx);
(... stuff to init FIR silicon ...)
pmx = pinmux_get(dev, "irda-fir");
pinmux_enable(pmx);
(... etc ...)

Just like you would take an alternative clock or regulator in case using i would
exclude the use of another clock/regulator.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-10 21:25     ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-10 21:25 UTC (permalink / raw)
  To: linux-arm-kernel

Finally I'm back at the pinmux patch set trying to answer all questions
accumulated:

2011/5/3 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> While I like the idea of consolidating the pinmuxing stuff, I'm not sure
> about having a struct pin_mux pointer in each bus types device structure.
> It'd mean this would have to be added to platform devices as well...

Yeah I'm not sure about the patch to the AMBA/PrimeCell bus really,
an alternative is for all drivers to request their mux individually. But in
the AMBA bus we already request the silicon block clock and the
silicon power switch so it seemed natural to also request a mux setting,
if there is one.

> Then there's SA1100 (and PXA?) to consider with its IrDA setup, where its
> necessary to switch the pin muxing during driver operation, when switching
> between SIR and FIR modes (SIR uses the UART, FIR uses a separate hardware
> block.)
>
> So any per-device pinmuxing subsystem also needs a way that a driver can
> change the pinmuxing of its associated pins on the fly too.

It can, basically:

struct pinmux *pmx;

pmx = pinmux_get(dev, "irda-uart");
pinmux_enable(pmx);
(... SIR UART operations ...)
pinmux_disable(pmx);
pinmux_put(pmx);
(... stuff to init FIR silicon ...)
pmx = pinmux_get(dev, "irda-fir");
pinmux_enable(pmx);
(... etc ...)

Just like you would take an alternative clock or regulator in case using i would
exclude the use of another clock/regulator.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-03 17:27   ` Andrew Lunn
@ 2011-05-10 21:42     ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-10 21:42 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: linux-kernel, Grant Likely, Martin Persson, Lee Jones, linux-arm-kernel

2011/5/3 Andrew Lunn <andrew@lunn.ch>:

> Say i have a UART core. I can be used as a 3-wire serial port, or
> additionally it can have hardware flow control, or additionally it can
> have all the modem signals. What would i expect to find in the pinmux
> driver?

If and only if it involves reassigning the pins coming out of your package,
enumerated as a one dimensional array, to alternative functions,
defined as possibly overlapping groups of pins in said array.

Anything else is orthogonal.

> 1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem.  The
>   first just has 2 pins, the second 4 and the last 8. The board code
>   selects one of these for the serial driver to use.

Sounds like a solid case for pinmuxing. Here the board code defines
three mux settings by name that your pinmux driver shall be able to handle,
and associate each of them with the same device.

> 2) Three functions: uart-core, uart-hw-flow, uart-mode.  The first has
>   2 pins, the second has 2 pins and the last 4 pins. The board code tells
>   the driver to use uart-core, plus say uart-hw-flow.

Same thing.

> Do you think the documentation should have guidelines how best to do
> this sort of core + additional optional extras?

I believe it already has, it describes a simple case of an overlapping
I2C and SPI port, two overlapping UART configurations are not
conceptually different.

> Say i have a SoC with an SPI core. This core has the usual MISC, MOSI
> and SCLK. It additionally has 4 chip select lines. My board uses chip
> select lines 2 and 3 for SPI, and select lines 0 and 1 as GPIO lines.
> How does the pinmux driver handle this? Again, it is the problems of a
> few core pins plus additional optional extras.

The pinmux API keeps track of individual pins and whether they are in
use or not. It keeps track of groups of pins and whether they overlap.
It denies a certain function to be muxed in in case its desired pins
overlap with another function.

Once you start modeling it you decide what overlapping means,
and in practice in this case taking the two lines that could have been
used for SPI chipselects as GPIO lines assures any attempts to
use them as SPI chipselects at the same time will be denied.

So the idea is not to keep track of all possible weird ways you may
use your pins, the idea is to put constraints on what you can physically
do and from there define the groups of pins you may use in practice.
Additionally requesting a single GPIO pin will mark a single pin as
"taken".

I *could* have modeled every pin that can be taken for GPIO as a
function containing a single pin but it seems too silly, mathematicians
would have loved it.

> Maybe the documentation should make some recommendations about what is
> placed into the drivers/pinmux/pinmux-foo.c and what should be kept in
> the board specific code?

It does by written example. And as an additional example a real-world
pinmux for the U300 has been moved from mach-u300 to
drivers/pinmux.

> Without these guidelines, it seems to me,
> each board for a given SoC is going to add its own peculiar pin
> combination to the drivers/pinmux/pinmux-foo.c file.

No not board. The driver is a driver for the package or pad ring,
nothing else. And the package certainly knows, by virtue of knowing
what differernt functions it can mux in/out by poking bits in the
hardware, about the actual groups that exists on the package.

The board in turn defines which of these possible functions that
it wants to mux in to certain devices. Note that it can assign multiple
functions to a single device, as mentioned in the reply to Russell.

>  Maybe it makes
> more sense to have a collection of standard pin functions in
> drivers/pinmux/pinmux-foo.c, which cover 75% of the likely
> combinations.

I don't get it. There are as many combinations as there are
chips. A function on pin 12, 13 and 35 on my package is very
unlikely to be viable on another package.

The pinmux API is *not* about keeping track of the fact that
SPI uses so or so many pins, it's just about groups of pins
in an array, nothing else.

> And recommend a board file to have its own additional
> list of strange pin functions which it registers with the pinmux core?

The board file only references the functions that the the package
can mux. The board file should *never* define any functions
or pin groups whatsoever, the pinmux driver knows about those.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-10 21:42     ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-10 21:42 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/3 Andrew Lunn <andrew@lunn.ch>:

> Say i have a UART core. I can be used as a 3-wire serial port, or
> additionally it can have hardware flow control, or additionally it can
> have all the modem signals. What would i expect to find in the pinmux
> driver?

If and only if it involves reassigning the pins coming out of your package,
enumerated as a one dimensional array, to alternative functions,
defined as possibly overlapping groups of pins in said array.

Anything else is orthogonal.

> 1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem. ?The
> ? first just has 2 pins, the second 4 and the last 8. The board code
> ? selects one of these for the serial driver to use.

Sounds like a solid case for pinmuxing. Here the board code defines
three mux settings by name that your pinmux driver shall be able to handle,
and associate each of them with the same device.

> 2) Three functions: uart-core, uart-hw-flow, uart-mode. ?The first has
> ? 2 pins, the second has 2 pins and the last 4 pins. The board code tells
> ? the driver to use uart-core, plus say uart-hw-flow.

Same thing.

> Do you think the documentation should have guidelines how best to do
> this sort of core + additional optional extras?

I believe it already has, it describes a simple case of an overlapping
I2C and SPI port, two overlapping UART configurations are not
conceptually different.

> Say i have a SoC with an SPI core. This core has the usual MISC, MOSI
> and SCLK. It additionally has 4 chip select lines. My board uses chip
> select lines 2 and 3 for SPI, and select lines 0 and 1 as GPIO lines.
> How does the pinmux driver handle this? Again, it is the problems of a
> few core pins plus additional optional extras.

The pinmux API keeps track of individual pins and whether they are in
use or not. It keeps track of groups of pins and whether they overlap.
It denies a certain function to be muxed in in case its desired pins
overlap with another function.

Once you start modeling it you decide what overlapping means,
and in practice in this case taking the two lines that could have been
used for SPI chipselects as GPIO lines assures any attempts to
use them as SPI chipselects at the same time will be denied.

So the idea is not to keep track of all possible weird ways you may
use your pins, the idea is to put constraints on what you can physically
do and from there define the groups of pins you may use in practice.
Additionally requesting a single GPIO pin will mark a single pin as
"taken".

I *could* have modeled every pin that can be taken for GPIO as a
function containing a single pin but it seems too silly, mathematicians
would have loved it.

> Maybe the documentation should make some recommendations about what is
> placed into the drivers/pinmux/pinmux-foo.c and what should be kept in
> the board specific code?

It does by written example. And as an additional example a real-world
pinmux for the U300 has been moved from mach-u300 to
drivers/pinmux.

> Without these guidelines, it seems to me,
> each board for a given SoC is going to add its own peculiar pin
> combination to the drivers/pinmux/pinmux-foo.c file.

No not board. The driver is a driver for the package or pad ring,
nothing else. And the package certainly knows, by virtue of knowing
what differernt functions it can mux in/out by poking bits in the
hardware, about the actual groups that exists on the package.

The board in turn defines which of these possible functions that
it wants to mux in to certain devices. Note that it can assign multiple
functions to a single device, as mentioned in the reply to Russell.

> ?Maybe it makes
> more sense to have a collection of standard pin functions in
> drivers/pinmux/pinmux-foo.c, which cover 75% of the likely
> combinations.

I don't get it. There are as many combinations as there are
chips. A function on pin 12, 13 and 35 on my package is very
unlikely to be viable on another package.

The pinmux API is *not* about keeping track of the fact that
SPI uses so or so many pins, it's just about groups of pins
in an array, nothing else.

> And recommend a board file to have its own additional
> list of strange pin functions which it registers with the pinmux core?

The board file only references the functions that the the package
can mux. The board file should *never* define any functions
or pin groups whatsoever, the pinmux driver knows about those.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-10 21:25     ` Linus Walleij
@ 2011-05-10 21:45       ` Russell King - ARM Linux
  -1 siblings, 0 replies; 48+ messages in thread
From: Russell King - ARM Linux @ 2011-05-10 21:45 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, Grant Likely, Martin Persson, Lee Jones, linux-arm-kernel

On Tue, May 10, 2011 at 11:25:44PM +0200, Linus Walleij wrote:
> It can, basically:
> 
> struct pinmux *pmx;
> 
> pmx = pinmux_get(dev, "irda-uart");
> pinmux_enable(pmx);
> (... SIR UART operations ...)
> pinmux_disable(pmx);
> pinmux_put(pmx);
> (... stuff to init FIR silicon ...)
> pmx = pinmux_get(dev, "irda-fir");
> pinmux_enable(pmx);
> (... etc ...)

You really don't want to do this.  It's not that SIR and FIR are that
exclusive.  You only switch to FIR mode when you've negotiated in SIR
mode, and then there's tight timings for doing that.  Essentially you
agree in SIR mode to switch to FIR mode, switch to FIR mode and expect
a response in FIR mode from the remote end.

Calling out to lots of functions to perform the switch is asking for
that response to be missed because you've not set things up for it.

You'd want to have SIR and FIR mode 'got', and then switch as quickly
as possible between them.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-10 21:45       ` Russell King - ARM Linux
  0 siblings, 0 replies; 48+ messages in thread
From: Russell King - ARM Linux @ 2011-05-10 21:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 10, 2011 at 11:25:44PM +0200, Linus Walleij wrote:
> It can, basically:
> 
> struct pinmux *pmx;
> 
> pmx = pinmux_get(dev, "irda-uart");
> pinmux_enable(pmx);
> (... SIR UART operations ...)
> pinmux_disable(pmx);
> pinmux_put(pmx);
> (... stuff to init FIR silicon ...)
> pmx = pinmux_get(dev, "irda-fir");
> pinmux_enable(pmx);
> (... etc ...)

You really don't want to do this.  It's not that SIR and FIR are that
exclusive.  You only switch to FIR mode when you've negotiated in SIR
mode, and then there's tight timings for doing that.  Essentially you
agree in SIR mode to switch to FIR mode, switch to FIR mode and expect
a response in FIR mode from the remote end.

Calling out to lots of functions to perform the switch is asking for
that response to be missed because you've not set things up for it.

You'd want to have SIR and FIR mode 'got', and then switch as quickly
as possible between them.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-10 21:45       ` Russell King - ARM Linux
@ 2011-05-10 23:15         ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-10 23:15 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grant Likely, Martin Persson, Lee Jones, linux-kernel, linux-arm-kernel

2011/5/10 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, May 10, 2011 at 11:25:44PM +0200, Linus Walleij wrote:
>> It can, basically:
>>
>> struct pinmux *pmx;
>>
>> pmx = pinmux_get(dev, "irda-uart");
>> pinmux_enable(pmx);
>> (... SIR UART operations ...)
>> pinmux_disable(pmx);
>> pinmux_put(pmx);
>> (... stuff to init FIR silicon ...)
>> pmx = pinmux_get(dev, "irda-fir");
>> pinmux_enable(pmx);
>> (... etc ...)
>
> You really don't want to do this.  It's not that SIR and FIR are that
> exclusive.  You only switch to FIR mode when you've negotiated in SIR
> mode, and then there's tight timings for doing that.  Essentially you
> agree in SIR mode to switch to FIR mode, switch to FIR mode and expect
> a response in FIR mode from the remote end.
>
> Calling out to lots of functions to perform the switch is asking for
> that response to be missed because you've not set things up for it.
>
> You'd want to have SIR and FIR mode 'got', and then switch as quickly
> as possible between them.

Sorry I might have got things backwards here, let me see if I got it:

Either the SIR and FIR modes use the exact same pins albeit pushing
in a differnt silicon block to control them without any side-effects.
In that case from the pinmux API point of view there is no conflict,
since there is no competition for using the same pins, the pinmux API
get/put is about resolving such conflicts.

So if we want to use the pinmux_enable()/disable() to quickly poke
some bit to switch between these two, I could think about things like
encoding the pins in the first setting and not encoding any pins at
all in the second one and then they can both be be get/put without
conflicting, and the only possible conflict would be if you tried
to enable both at the same time and this can be handled by the
specific pinmux driver.

Albeit not elegant, it works for this scenario.

A more elegant way is that I supply the same custom configuration
extension mechanism for pinmux as for GPIO, similar to ioctl()
like so:

pmx = pinmux_get(dev, "irda-sir-fir");
pmx_enable(pmx);
/* Unless the first enablement implies SIR mode */
pmx_config(pmx, PINMUX_CONFIG_CUSTOM_SIR, NULL);
(... negotiated ...)
pmx_config(pmx, PINMUX_CONFIG_CUSTOM_FIR, NULL);
(...)
pmx_disable(pmx);
pmx_put(pmx);

I planned to add this interface anyway (needed for pin biasing and
such fun stuff), so I'll poke it in.

The latter would be analog to suppling an enumerated "what" to
the pinmux_enable() function, if it turns out to be an everyday
thing we can think about a more generic extension like
pinmux_enable_selector(struct pinmux *pmx, unsigned selector);
in the API but I feel the above will cut it just as fine.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-10 23:15         ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-10 23:15 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/10 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, May 10, 2011 at 11:25:44PM +0200, Linus Walleij wrote:
>> It can, basically:
>>
>> struct pinmux *pmx;
>>
>> pmx = pinmux_get(dev, "irda-uart");
>> pinmux_enable(pmx);
>> (... SIR UART operations ...)
>> pinmux_disable(pmx);
>> pinmux_put(pmx);
>> (... stuff to init FIR silicon ...)
>> pmx = pinmux_get(dev, "irda-fir");
>> pinmux_enable(pmx);
>> (... etc ...)
>
> You really don't want to do this. ?It's not that SIR and FIR are that
> exclusive. ?You only switch to FIR mode when you've negotiated in SIR
> mode, and then there's tight timings for doing that. ?Essentially you
> agree in SIR mode to switch to FIR mode, switch to FIR mode and expect
> a response in FIR mode from the remote end.
>
> Calling out to lots of functions to perform the switch is asking for
> that response to be missed because you've not set things up for it.
>
> You'd want to have SIR and FIR mode 'got', and then switch as quickly
> as possible between them.

Sorry I might have got things backwards here, let me see if I got it:

Either the SIR and FIR modes use the exact same pins albeit pushing
in a differnt silicon block to control them without any side-effects.
In that case from the pinmux API point of view there is no conflict,
since there is no competition for using the same pins, the pinmux API
get/put is about resolving such conflicts.

So if we want to use the pinmux_enable()/disable() to quickly poke
some bit to switch between these two, I could think about things like
encoding the pins in the first setting and not encoding any pins at
all in the second one and then they can both be be get/put without
conflicting, and the only possible conflict would be if you tried
to enable both at the same time and this can be handled by the
specific pinmux driver.

Albeit not elegant, it works for this scenario.

A more elegant way is that I supply the same custom configuration
extension mechanism for pinmux as for GPIO, similar to ioctl()
like so:

pmx = pinmux_get(dev, "irda-sir-fir");
pmx_enable(pmx);
/* Unless the first enablement implies SIR mode */
pmx_config(pmx, PINMUX_CONFIG_CUSTOM_SIR, NULL);
(... negotiated ...)
pmx_config(pmx, PINMUX_CONFIG_CUSTOM_FIR, NULL);
(...)
pmx_disable(pmx);
pmx_put(pmx);

I planned to add this interface anyway (needed for pin biasing and
such fun stuff), so I'll poke it in.

The latter would be analog to suppling an enumerated "what" to
the pinmux_enable() function, if it turns out to be an everyday
thing we can think about a more generic extension like
pinmux_enable_selector(struct pinmux *pmx, unsigned selector);
in the API but I feel the above will cut it just as fine.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-10 21:42     ` Linus Walleij
@ 2011-05-11  9:50       ` Andrew Lunn
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-11  9:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, linux-kernel, Grant Likely, Martin Persson,
	Lee Jones, linux-arm-kernel

> > 1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem. ?The
> > ? first just has 2 pins, the second 4 and the last 8. The board code
> > ? selects one of these for the serial driver to use.
> 
> Sounds like a solid case for pinmuxing. Here the board code defines
> three mux settings by name that your pinmux driver shall be able to handle,
> and associate each of them with the same device.
> 
> > 2) Three functions: uart-core, uart-hw-flow, uart-mode. ?The first has
> > ? 2 pins, the second has 2 pins and the last 4 pins. The board code tells
> > ? the driver to use uart-core, plus say uart-hw-flow.
> No not board. The driver is a driver for the package or pad ring,
> nothing else. And the package certainly knows, by virtue of knowing
> what differernt functions it can mux in/out by poking bits in the
> hardware, about the actual groups that exists on the package.
> 
> The board in turn defines which of these possible functions that
> it wants to mux in to certain devices. Note that it can assign multiple
> functions to a single device, as mentioned in the reply to Russell.
> 
> > ?Maybe it makes
> > more sense to have a collection of standard pin functions in
> > drivers/pinmux/pinmux-foo.c, which cover 75% of the likely
> > combinations.
> 
> I don't get it. There are as many combinations as there are
> chips. A function on pin 12, 13 and 35 on my package is very
> unlikely to be viable on another package.
> 
> The pinmux API is *not* about keeping track of the fact that
> SPI uses so or so many pins, it's just about groups of pins
> in an array, nothing else.
> 
> > And recommend a board file to have its own additional
> > list of strange pin functions which it registers with the pinmux core?
> 
> The board file only references the functions that the the package
> can mux. The board file should *never* define any functions
> or pin groups whatsoever, the pinmux driver knows about those.

Hi Linus

Thanks for your reply. 

I still however don't get how the different part of this are spread
around the different levels of the logical hierarchy.

Lets take a system i'm familiar with. I'm working of a kirkwood board,
called bubba3. The logical hierarchy is:

linux | arm | plat-orion | mach-kirkwood | bubba3.

The pinmux core belongs in the linux level. It can be used by any
architecture. It is generic.

For the arm level, we have nothing.

plat-orion knows about the registers for controlling muxing.  It knows
it has a bank of four registers, each register containing one nibble
per pad. It can provide functions to set these registers. However
plat-orion has no idea what these pads are used for. It does not know
what functions each pad has.

mach-kirkwood does know what selection of functions each pad can take.
PAD0 can be a GPIO, or a NAND FLASH data bit2, or an SPI chip select
line. Maybe more interestingly, PAD4 can be UART0 RXD, but also PAD11
can also be UART0 RXD. It can also know that the TWI interface 0 needs
PADs 8 and 9 and that there are no other alternatives. So it could
export this information. However TWI interface 1 has two pads for SDA
and two pads for SCL. It could in theory export all four combinations
which make valid TWI configurations.

bubba3 board code knows that the board uses UART0 RXD from PAD 10. It
also knows that its a simple 3 wire UART, so the flow control signals
on various pads are not needed etc.

Now lets map these different logical hierarchies onto a pinmux driver
that would be placed into drivers/pinmux/kirkwood.c. I'm making a big
assumption here. Each mach will provide a pinmux driver, not each
board. We are trying to reduce code bloat, so we want to minimize
repeated code. This is why i expect a pinmux driver per mach, not per
board, since i expect there is a lot of duplication between boards of
the same mach. We also want to keep maintenance simple, so what is
probably never going to be used outside of bubba3 should probably be
kept in the bubba3 board file.

foo_enable() and foo_disable() methods come from plat-orion. So i
#include <plat/orion> and use the functions. That works fine for all
the other systems based on orion.

Now to pins

The example driver in your Documentation has

static unsigned int i2c0_pins[] = { 24, 25 };

We can do the same for the TWI interface 0

static unsigned int twi_0_pins[] = { 8, 9};

but what about TWI interface 1? There are only four combinations, so i
could do:

static unsigned int twi_1_a_pins[] = { 12, 17};
static unsigned int twi_1_b_pins[] = { 12, 37};
static unsigned int twi_1_c_pins[] = { 36, 17};
static unsigned int twi_1_d_pins[] = { 36, 37;

static struct foo_pmx_func myfuncs[] = {
       {
               .name = "twi-1-a",
               .pins = twi-1-a,
               .num_pins = ARRAY_SIZE(twi_1_a_pins),
       },
       {
               .name = "twi-1-b",
               .pins = twi-1-b,
               .num_pins = ARRAY_SIZE(twi_1_b_pins),
       },
       {
               .name = "twi-1-c",
               .pins = twi-1-c,
               .num_pins = ARRAY_SIZE(twi_1_c_pins),
       },
       {
               .name = "twi-1-d",
               .pins = twi-1-d,
               .num_pins = ARRAY_SIZE(twi_1_d_pins),
       },
};

and let the board tell the Marvell TWI driver to use twi-1-c with:

       PINMUX_MAP("twi-1-c", "mv-spi.1")

Four combinations is not too bad. But SPI is worse, there are
something like 32 combinations, maybe more. I could look at all the
different kirkwood boards and find the subset used and add just those?
And when a new board is added, it might have to add yet another
combination. The same problem exists for the UART. Not just which pins
are used, but also, is HW flow control used, are modem lines used,
etc.

To me, it makes more sense that drivers/pinmux/kirkwood.c provides the
unique pin functions, where there are no alternatives, eg the twi
0. It could also provide the pin functions used my Marvell in the
kirkwood reference design. It is likely that many real boards will be
based on that. However, if my bubba3 SPI pins don't fit with the
Marvell RDK, i probably want to put them in my board file.

Looking at the API, i don't see how my board could provide the list of
SPI pins. So it looks like each kirkwood board will have to extend
drivers/pinmux/kirkwood.c with its own specific pin definitions. This
i don't like.

     Andrew

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-11  9:50       ` Andrew Lunn
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-11  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

> > 1) Three functions: uart-3wire, uart-hw-flow, uart-hw-flow-modem. ?The
> > ? first just has 2 pins, the second 4 and the last 8. The board code
> > ? selects one of these for the serial driver to use.
> 
> Sounds like a solid case for pinmuxing. Here the board code defines
> three mux settings by name that your pinmux driver shall be able to handle,
> and associate each of them with the same device.
> 
> > 2) Three functions: uart-core, uart-hw-flow, uart-mode. ?The first has
> > ? 2 pins, the second has 2 pins and the last 4 pins. The board code tells
> > ? the driver to use uart-core, plus say uart-hw-flow.
> No not board. The driver is a driver for the package or pad ring,
> nothing else. And the package certainly knows, by virtue of knowing
> what differernt functions it can mux in/out by poking bits in the
> hardware, about the actual groups that exists on the package.
> 
> The board in turn defines which of these possible functions that
> it wants to mux in to certain devices. Note that it can assign multiple
> functions to a single device, as mentioned in the reply to Russell.
> 
> > ?Maybe it makes
> > more sense to have a collection of standard pin functions in
> > drivers/pinmux/pinmux-foo.c, which cover 75% of the likely
> > combinations.
> 
> I don't get it. There are as many combinations as there are
> chips. A function on pin 12, 13 and 35 on my package is very
> unlikely to be viable on another package.
> 
> The pinmux API is *not* about keeping track of the fact that
> SPI uses so or so many pins, it's just about groups of pins
> in an array, nothing else.
> 
> > And recommend a board file to have its own additional
> > list of strange pin functions which it registers with the pinmux core?
> 
> The board file only references the functions that the the package
> can mux. The board file should *never* define any functions
> or pin groups whatsoever, the pinmux driver knows about those.

Hi Linus

Thanks for your reply. 

I still however don't get how the different part of this are spread
around the different levels of the logical hierarchy.

Lets take a system i'm familiar with. I'm working of a kirkwood board,
called bubba3. The logical hierarchy is:

linux | arm | plat-orion | mach-kirkwood | bubba3.

The pinmux core belongs in the linux level. It can be used by any
architecture. It is generic.

For the arm level, we have nothing.

plat-orion knows about the registers for controlling muxing.  It knows
it has a bank of four registers, each register containing one nibble
per pad. It can provide functions to set these registers. However
plat-orion has no idea what these pads are used for. It does not know
what functions each pad has.

mach-kirkwood does know what selection of functions each pad can take.
PAD0 can be a GPIO, or a NAND FLASH data bit2, or an SPI chip select
line. Maybe more interestingly, PAD4 can be UART0 RXD, but also PAD11
can also be UART0 RXD. It can also know that the TWI interface 0 needs
PADs 8 and 9 and that there are no other alternatives. So it could
export this information. However TWI interface 1 has two pads for SDA
and two pads for SCL. It could in theory export all four combinations
which make valid TWI configurations.

bubba3 board code knows that the board uses UART0 RXD from PAD 10. It
also knows that its a simple 3 wire UART, so the flow control signals
on various pads are not needed etc.

Now lets map these different logical hierarchies onto a pinmux driver
that would be placed into drivers/pinmux/kirkwood.c. I'm making a big
assumption here. Each mach will provide a pinmux driver, not each
board. We are trying to reduce code bloat, so we want to minimize
repeated code. This is why i expect a pinmux driver per mach, not per
board, since i expect there is a lot of duplication between boards of
the same mach. We also want to keep maintenance simple, so what is
probably never going to be used outside of bubba3 should probably be
kept in the bubba3 board file.

foo_enable() and foo_disable() methods come from plat-orion. So i
#include <plat/orion> and use the functions. That works fine for all
the other systems based on orion.

Now to pins

The example driver in your Documentation has

static unsigned int i2c0_pins[] = { 24, 25 };

We can do the same for the TWI interface 0

static unsigned int twi_0_pins[] = { 8, 9};

but what about TWI interface 1? There are only four combinations, so i
could do:

static unsigned int twi_1_a_pins[] = { 12, 17};
static unsigned int twi_1_b_pins[] = { 12, 37};
static unsigned int twi_1_c_pins[] = { 36, 17};
static unsigned int twi_1_d_pins[] = { 36, 37;

static struct foo_pmx_func myfuncs[] = {
       {
               .name = "twi-1-a",
               .pins = twi-1-a,
               .num_pins = ARRAY_SIZE(twi_1_a_pins),
       },
       {
               .name = "twi-1-b",
               .pins = twi-1-b,
               .num_pins = ARRAY_SIZE(twi_1_b_pins),
       },
       {
               .name = "twi-1-c",
               .pins = twi-1-c,
               .num_pins = ARRAY_SIZE(twi_1_c_pins),
       },
       {
               .name = "twi-1-d",
               .pins = twi-1-d,
               .num_pins = ARRAY_SIZE(twi_1_d_pins),
       },
};

and let the board tell the Marvell TWI driver to use twi-1-c with:

       PINMUX_MAP("twi-1-c", "mv-spi.1")

Four combinations is not too bad. But SPI is worse, there are
something like 32 combinations, maybe more. I could look at all the
different kirkwood boards and find the subset used and add just those?
And when a new board is added, it might have to add yet another
combination. The same problem exists for the UART. Not just which pins
are used, but also, is HW flow control used, are modem lines used,
etc.

To me, it makes more sense that drivers/pinmux/kirkwood.c provides the
unique pin functions, where there are no alternatives, eg the twi
0. It could also provide the pin functions used my Marvell in the
kirkwood reference design. It is likely that many real boards will be
based on that. However, if my bubba3 SPI pins don't fit with the
Marvell RDK, i probably want to put them in my board file.

Looking at the API, i don't see how my board could provide the list of
SPI pins. So it looks like each kirkwood board will have to extend
drivers/pinmux/kirkwood.c with its own specific pin definitions. This
i don't like.

     Andrew

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-11  9:50       ` Andrew Lunn
@ 2011-05-12  0:41         ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-12  0:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: linux-kernel, Grant Likely, Martin Persson, Lee Jones, linux-arm-kernel

2011/5/11 Andrew Lunn <andrew@lunn.ch>:

> plat-orion knows about the registers for controlling muxing.  It knows
> it has a bank of four registers, each register containing one nibble
> per pad. It can provide functions to set these registers. However
> plat-orion has no idea what these pads are used for. It does not know
> what functions each pad has.

No thanks, no code in plat-orion or any plat-xxx. No register definitions
either. This is all part of the problem with arch/arm/*.

Any drivers and register definitions for the pinmux driver goes into
drivers/pinmux/*, the machine/board provide base address for where it
is if need be, any IRQ as struct resource and any platform data on
some kind of custom format.

If you absolutely think it is a bad idea to put data about what function
groups there are and which pins they have directly into drivers/pinmux/*
you need to define a platform data struct someplace like
<linux/pinmux/foo.h> and pass it in through that, in the future we may even
be able to pass it directly from the device tree and skip all of these
info passing mechanisms.

> mach-kirkwood does know what selection of functions each pad can take.
> PAD0 can be a GPIO, or a NAND FLASH data bit2, or an SPI chip select
> line.

I think the driver in drivers/pinmux/* need to know this. Whether the raw
data comes from the machine or resides directly in drivers/pinmux/* is
an implementation thing.

> Maybe more interestingly, PAD4 can be UART0 RXD, but also PAD11
> can also be UART0 RXD. It can also know that the TWI interface 0 needs
> PADs 8 and 9 and that there are no other alternatives. So it could
> export this information. However TWI interface 1 has two pads for SDA
> and two pads for SCL. It could in theory export all four combinations
> which make valid TWI configurations.

OK.

> bubba3 board code knows that the board uses UART0 RXD from PAD 10. It
> also knows that its a simple 3 wire UART, so the flow control signals
> on various pads are not needed etc.

I don't think the board code should "know" about anything related to
a specific indvidual pad. It will know that to use UART0 on this board,
it should select some function name like "uart0-1". The pinmux driver
knows which pads this name is connected to. Noone else. Maybe
it got some static table out of the mach dir when instantiating itself,
that's not "knowing" IMO, I don't know what term to use sorry.

> Now lets map these different logical hierarchies onto a pinmux driver
> that would be placed into drivers/pinmux/kirkwood.c. I'm making a big
> assumption here.

This sounds like a strange name, more like it would be
drivers/pinmux/pinmux-orion.c or so, then configured with different
functions and pad sets for different machines by a set of
array tables.

> Each mach will provide a pinmux driver, not each
> board.

No, the machines provide no drivers. We need to get driver out
of arch/arm/*. The drivers shall be in drivers/pinmux/*. The mach
or if you like package may provide data to the driver if you
need to.

> We are trying to reduce code bloat, so we want to minimize
> repeated code. This is why i expect a pinmux driver per mach, not per
> board, since i expect there is a lot of duplication between boards of
> the same mach.

I expect one pinmux driver for all machines covered by a plat-*,
unless they have totally different registers, different striding, different
features altogether etc, in case they are really totally different
hardware, then we need drivers/pinmux/pinmux-foo.c,
pinmux-bar.c etc to cover them unless we can consolidate the
code.

> We also want to keep maintenance simple, so what is
> probably never going to be used outside of bubba3 should probably be
> kept in the bubba3 board file.

Board data is to be kept there, and that means which functions shall
be mapped to what devices and nothing else IMO.

The package of a certain chip is something else than the board IMO,
and that is what the driver in drivers/pinmux/* is all about. The only
thing the board is about is how to map the muxable functions for this
specific board.

> foo_enable() and foo_disable() methods come from plat-orion. So i
> #include <plat/orion> and use the functions. That works fine for all
> the other systems based on orion.

No code in plat-orion. drivers/pinmux/pinmux-orion.c.

> Now to pins
>
> The example driver in your Documentation has
>
> static unsigned int i2c0_pins[] = { 24, 25 };
>
> We can do the same for the TWI interface 0
>
> static unsigned int twi_0_pins[] = { 8, 9};
>
> but what about TWI interface 1? There are only four combinations, so i
> could do:
>
> static unsigned int twi_1_a_pins[] = { 12, 17};
> static unsigned int twi_1_b_pins[] = { 12, 37};
> static unsigned int twi_1_c_pins[] = { 36, 17};
> static unsigned int twi_1_d_pins[] = { 36, 37};
>
> static struct foo_pmx_func myfuncs[] = {
>       {
>               .name = "twi-1-a",
>               .pins = twi-1-a,
>               .num_pins = ARRAY_SIZE(twi_1_a_pins),
>       },
>       {
>               .name = "twi-1-b",
>               .pins = twi-1-b,
>               .num_pins = ARRAY_SIZE(twi_1_b_pins),
>       },
>       {
>               .name = "twi-1-c",
>               .pins = twi-1-c,
>               .num_pins = ARRAY_SIZE(twi_1_c_pins),
>       },
>       {
>               .name = "twi-1-d",
>               .pins = twi-1-d,
>               .num_pins = ARRAY_SIZE(twi_1_d_pins),
>       },
> };
>
> and let the board tell the Marvell TWI driver to use twi-1-c with:
>
>       PINMUX_MAP("twi-1-c", "mv-spi.1")

Yes exactly.

> Four combinations is not too bad. But SPI is worse, there are
> something like 32 combinations, maybe more. I could look at all the
> different kirkwood boards and find the subset used and add just those?
> And when a new board is added, it might have to add yet another
> combination. The same problem exists for the UART. Not just which pins
> are used, but also, is HW flow control used, are modem lines used,
> etc.

How would you do it today? I am not trying to solve the problem that
the world needs to be enumerated somehow, I am trying to provide
a framework for doing so.

And yes, since this isn't about mathematics we only need to enumerate
that which is practically useful.

If you want a futureproof solution you will have to wait until we have
device tree, then you can encode all of these combinations in the
device tree passed to the kernel.

> To me, it makes more sense that drivers/pinmux/kirkwood.c provides the
> unique pin functions, where there are no alternatives, eg the twi
> 0. It could also provide the pin functions used my Marvell in the
> kirkwood reference design. It is likely that many real boards will be
> based on that. However, if my bubba3 SPI pins don't fit with the
> Marvell RDK, i probably want to put them in my board file.

Sorry I'm not following, what is an RDK?

Anyway you need a way to specify all the useful functions for some
chip in a package on that board, then the board need to select a
subset of these functions.

> Looking at the API, i don't see how my board could provide the list of
> SPI pins. So it looks like each kirkwood board will have to extend
> drivers/pinmux/kirkwood.c with its own specific pin definitions. This
> i don't like.

But I don't think this is about board data at all. I think the pads and
their function combinations are about package data, not board.

There are alternatives:

* If the number of pins and possible muxings are reasonably small
  you can encode it directly in the driver as done in the U300 example.

* If you feel this clutters the driver with tables and header files, you can
  pass it in from the platform data (assuming a platform device), i.e.
  a custom struct containing the same stuff that is encoded in the
  table in the example driver. (Or the U300.) This is no different from
  how you pass data to any platform driver today.

The fact that the data about a certain package (for example the
DB3350 in the U300 case) would come from some
mach-u300/package-db3350.c file or header does not make it a board
file, it's a package file.

Soon we need to get rid of that also to get data encoding out of the
kernel, then we will probably need to encode it in a device tree
that knows about both the board and the chip package in a hierarchical
manner.

I'll go over the documentation again and see if I can clarify further...

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-12  0:41         ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-12  0:41 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/11 Andrew Lunn <andrew@lunn.ch>:

> plat-orion knows about the registers for controlling muxing. ?It knows
> it has a bank of four registers, each register containing one nibble
> per pad. It can provide functions to set these registers. However
> plat-orion has no idea what these pads are used for. It does not know
> what functions each pad has.

No thanks, no code in plat-orion or any plat-xxx. No register definitions
either. This is all part of the problem with arch/arm/*.

Any drivers and register definitions for the pinmux driver goes into
drivers/pinmux/*, the machine/board provide base address for where it
is if need be, any IRQ as struct resource and any platform data on
some kind of custom format.

If you absolutely think it is a bad idea to put data about what function
groups there are and which pins they have directly into drivers/pinmux/*
you need to define a platform data struct someplace like
<linux/pinmux/foo.h> and pass it in through that, in the future we may even
be able to pass it directly from the device tree and skip all of these
info passing mechanisms.

> mach-kirkwood does know what selection of functions each pad can take.
> PAD0 can be a GPIO, or a NAND FLASH data bit2, or an SPI chip select
> line.

I think the driver in drivers/pinmux/* need to know this. Whether the raw
data comes from the machine or resides directly in drivers/pinmux/* is
an implementation thing.

> Maybe more interestingly, PAD4 can be UART0 RXD, but also PAD11
> can also be UART0 RXD. It can also know that the TWI interface 0 needs
> PADs 8 and 9 and that there are no other alternatives. So it could
> export this information. However TWI interface 1 has two pads for SDA
> and two pads for SCL. It could in theory export all four combinations
> which make valid TWI configurations.

OK.

> bubba3 board code knows that the board uses UART0 RXD from PAD 10. It
> also knows that its a simple 3 wire UART, so the flow control signals
> on various pads are not needed etc.

I don't think the board code should "know" about anything related to
a specific indvidual pad. It will know that to use UART0 on this board,
it should select some function name like "uart0-1". The pinmux driver
knows which pads this name is connected to. Noone else. Maybe
it got some static table out of the mach dir when instantiating itself,
that's not "knowing" IMO, I don't know what term to use sorry.

> Now lets map these different logical hierarchies onto a pinmux driver
> that would be placed into drivers/pinmux/kirkwood.c. I'm making a big
> assumption here.

This sounds like a strange name, more like it would be
drivers/pinmux/pinmux-orion.c or so, then configured with different
functions and pad sets for different machines by a set of
array tables.

> Each mach will provide a pinmux driver, not each
> board.

No, the machines provide no drivers. We need to get driver out
of arch/arm/*. The drivers shall be in drivers/pinmux/*. The mach
or if you like package may provide data to the driver if you
need to.

> We are trying to reduce code bloat, so we want to minimize
> repeated code. This is why i expect a pinmux driver per mach, not per
> board, since i expect there is a lot of duplication between boards of
> the same mach.

I expect one pinmux driver for all machines covered by a plat-*,
unless they have totally different registers, different striding, different
features altogether etc, in case they are really totally different
hardware, then we need drivers/pinmux/pinmux-foo.c,
pinmux-bar.c etc to cover them unless we can consolidate the
code.

> We also want to keep maintenance simple, so what is
> probably never going to be used outside of bubba3 should probably be
> kept in the bubba3 board file.

Board data is to be kept there, and that means which functions shall
be mapped to what devices and nothing else IMO.

The package of a certain chip is something else than the board IMO,
and that is what the driver in drivers/pinmux/* is all about. The only
thing the board is about is how to map the muxable functions for this
specific board.

> foo_enable() and foo_disable() methods come from plat-orion. So i
> #include <plat/orion> and use the functions. That works fine for all
> the other systems based on orion.

No code in plat-orion. drivers/pinmux/pinmux-orion.c.

> Now to pins
>
> The example driver in your Documentation has
>
> static unsigned int i2c0_pins[] = { 24, 25 };
>
> We can do the same for the TWI interface 0
>
> static unsigned int twi_0_pins[] = { 8, 9};
>
> but what about TWI interface 1? There are only four combinations, so i
> could do:
>
> static unsigned int twi_1_a_pins[] = { 12, 17};
> static unsigned int twi_1_b_pins[] = { 12, 37};
> static unsigned int twi_1_c_pins[] = { 36, 17};
> static unsigned int twi_1_d_pins[] = { 36, 37};
>
> static struct foo_pmx_func myfuncs[] = {
> ? ? ? {
> ? ? ? ? ? ? ? .name = "twi-1-a",
> ? ? ? ? ? ? ? .pins = twi-1-a,
> ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(twi_1_a_pins),
> ? ? ? },
> ? ? ? {
> ? ? ? ? ? ? ? .name = "twi-1-b",
> ? ? ? ? ? ? ? .pins = twi-1-b,
> ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(twi_1_b_pins),
> ? ? ? },
> ? ? ? {
> ? ? ? ? ? ? ? .name = "twi-1-c",
> ? ? ? ? ? ? ? .pins = twi-1-c,
> ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(twi_1_c_pins),
> ? ? ? },
> ? ? ? {
> ? ? ? ? ? ? ? .name = "twi-1-d",
> ? ? ? ? ? ? ? .pins = twi-1-d,
> ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(twi_1_d_pins),
> ? ? ? },
> };
>
> and let the board tell the Marvell TWI driver to use twi-1-c with:
>
> ? ? ? PINMUX_MAP("twi-1-c", "mv-spi.1")

Yes exactly.

> Four combinations is not too bad. But SPI is worse, there are
> something like 32 combinations, maybe more. I could look at all the
> different kirkwood boards and find the subset used and add just those?
> And when a new board is added, it might have to add yet another
> combination. The same problem exists for the UART. Not just which pins
> are used, but also, is HW flow control used, are modem lines used,
> etc.

How would you do it today? I am not trying to solve the problem that
the world needs to be enumerated somehow, I am trying to provide
a framework for doing so.

And yes, since this isn't about mathematics we only need to enumerate
that which is practically useful.

If you want a futureproof solution you will have to wait until we have
device tree, then you can encode all of these combinations in the
device tree passed to the kernel.

> To me, it makes more sense that drivers/pinmux/kirkwood.c provides the
> unique pin functions, where there are no alternatives, eg the twi
> 0. It could also provide the pin functions used my Marvell in the
> kirkwood reference design. It is likely that many real boards will be
> based on that. However, if my bubba3 SPI pins don't fit with the
> Marvell RDK, i probably want to put them in my board file.

Sorry I'm not following, what is an RDK?

Anyway you need a way to specify all the useful functions for some
chip in a package on that board, then the board need to select a
subset of these functions.

> Looking at the API, i don't see how my board could provide the list of
> SPI pins. So it looks like each kirkwood board will have to extend
> drivers/pinmux/kirkwood.c with its own specific pin definitions. This
> i don't like.

But I don't think this is about board data at all. I think the pads and
their function combinations are about package data, not board.

There are alternatives:

* If the number of pins and possible muxings are reasonably small
  you can encode it directly in the driver as done in the U300 example.

* If you feel this clutters the driver with tables and header files, you can
  pass it in from the platform data (assuming a platform device), i.e.
  a custom struct containing the same stuff that is encoded in the
  table in the example driver. (Or the U300.) This is no different from
  how you pass data to any platform driver today.

The fact that the data about a certain package (for example the
DB3350 in the U300 case) would come from some
mach-u300/package-db3350.c file or header does not make it a board
file, it's a package file.

Soon we need to get rid of that also to get data encoding out of the
kernel, then we will probably need to encode it in a device tree
that knows about both the board and the chip package in a hierarchical
manner.

I'll go over the documentation again and see if I can clarify further...

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-12  0:41         ` Linus Walleij
@ 2011-05-12  7:00           ` Andrew Lunn
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-12  7:00 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, linux-kernel, Grant Likely, Martin Persson,
	Lee Jones, linux-arm-kernel

> > To me, it makes more sense that drivers/pinmux/kirkwood.c provides the
> > unique pin functions, where there are no alternatives, eg the twi
> > 0. It could also provide the pin functions used my Marvell in the
> > kirkwood reference design. It is likely that many real boards will be
> > based on that. However, if my bubba3 SPI pins don't fit with the
> > Marvell RDK, i probably want to put them in my board file.
> 
> Sorry I'm not following, what is an RDK?

Reference Design Kit. 

A chip manufacture generally has a reference board which uses most of
the functionality of the chip. They generally make the board design
available, so that anybody wanting to design a new board around the
chip can start with the RDK and add/remove parts and customize it to
there own requirements. 

Marvell are a bit closed about there RDKs, so i picked an AT91 as an
example:

http://www.atmel.com/dyn/products/tools_card.asp?tool_id=4281&category_id=163&family_id=605&subfamily_id=1964

All the design files are there for download.

Many boards using an AT91SAM9XE will be similar to this. So if the
pinmux driver supports this board, it will be a good basis for all
other boards which use the AT91SAM9XE.

    Andrew

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-12  7:00           ` Andrew Lunn
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-12  7:00 UTC (permalink / raw)
  To: linux-arm-kernel

> > To me, it makes more sense that drivers/pinmux/kirkwood.c provides the
> > unique pin functions, where there are no alternatives, eg the twi
> > 0. It could also provide the pin functions used my Marvell in the
> > kirkwood reference design. It is likely that many real boards will be
> > based on that. However, if my bubba3 SPI pins don't fit with the
> > Marvell RDK, i probably want to put them in my board file.
> 
> Sorry I'm not following, what is an RDK?

Reference Design Kit. 

A chip manufacture generally has a reference board which uses most of
the functionality of the chip. They generally make the board design
available, so that anybody wanting to design a new board around the
chip can start with the RDK and add/remove parts and customize it to
there own requirements. 

Marvell are a bit closed about there RDKs, so i picked an AT91 as an
example:

http://www.atmel.com/dyn/products/tools_card.asp?tool_id=4281&category_id=163&family_id=605&subfamily_id=1964

All the design files are there for download.

Many boards using an AT91SAM9XE will be similar to this. So if the
pinmux driver supports this board, it will be a good basis for all
other boards which use the AT91SAM9XE.

    Andrew

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-02 19:16 ` Linus Walleij
@ 2011-05-12  7:44   ` Sascha Hauer
  -1 siblings, 0 replies; 48+ messages in thread
From: Sascha Hauer @ 2011-05-12  7:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-kernel, linux-arm-kernel, Grant Likely, Martin Persson,
	Linus Walleij, Lee Jones

Hi Linus,

On Mon, May 02, 2011 at 09:16:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This patchset creates a pinmux subsystem and switches U300 to use that new
> subsystem as an example. The problem is not that fantastically hard to
> solve in a general way, nobody got around to it because it requires some
> upfront code I believe, and this is my stab at it.

This is a good step in the right direction. It solves the problem of
potential conflicts between different mux functions. Also it gives a
well defined interface between drivers and board code.

What I'm missing though is a generic way a single pad/mux mode
combination can be described. Let me take a look around how the
different subarchs do this:

omap:

	_OMAP3_MUXENTRY(DSS_DATA21, 91,
		"dss_data21", NULL, "mcspi3_cs0", "dss_data3",
		"gpio_91", NULL, NULL, "safe_mode"),
	_OMAP3_MUXENTRY(DSS_DATA22, 92,
		"dss_data22", NULL, "mcspi3_cs1", "dss_data4",
		"gpio_92", NULL, NULL, "safe_mode"),

pxa:

#define GPIO16_FFUART_TXD       MFP_CFG_OUT(GPIO16, AF3, DRIVE_HIGH)
#define GPIO37_FFUART_TXD       MFP_CFG_OUT(GPIO37, AF3, DRIVE_HIGH)
#define GPIO39_FFUART_TXD       MFP_CFG_OUT(GPIO39, AF2, DRIVE_HIGH)
#define GPIO83_FFUART_TXD       MFP_CFG_OUT(GPIO83, AF2, DRIVE_HIGH)
#define GPIO99_FFUART_TXD       MFP_CFG_OUT(GPIO99, AF3, DRIVE_HIGH)

i.MX:

#define _MX51_PAD_UART3_RXD__CSI1_D0            IOMUX_PAD(0x630, 0x240, 2, 0x0000, 0, 0)
#define _MX51_PAD_UART3_RXD__GPIO1_22           IOMUX_PAD(0x630, 0x240, 3, 0x0000, 0, 0)
#define _MX51_PAD_UART3_RXD__UART1_DTR          IOMUX_PAD(0x630, 0x240, 0, 0x0000, 0, 0)
#define _MX51_PAD_UART3_RXD__UART3_RXD          IOMUX_PAD(0x630, 0x240, 1, 0x09f4, 4, 0)
#define _MX51_PAD_UART3_TXD__CSI1_D1            IOMUX_PAD(0x634, 0x244, 2, 0x0000, 0, 0)

These all basically describe the same thing: put pad x into one of modes
a, b, c and apply certain flags like drive strength on this.

the other class of pin muxing I know of is that a whole group of pads
can be switched to a particular mode using a mux register like I think
is used used in your ux300 driver.

I'd like to have a unified way to describe this. If we ever want to move
this into the device tree we need this anyway as I think it's not an
option to have completely different SoC specific descriptions in the
device tree.

Do you think it's possible to do some consolidation on this level
aswell? It would really bring different SoCs more together.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-12  7:44   ` Sascha Hauer
  0 siblings, 0 replies; 48+ messages in thread
From: Sascha Hauer @ 2011-05-12  7:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Linus,

On Mon, May 02, 2011 at 09:16:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This patchset creates a pinmux subsystem and switches U300 to use that new
> subsystem as an example. The problem is not that fantastically hard to
> solve in a general way, nobody got around to it because it requires some
> upfront code I believe, and this is my stab at it.

This is a good step in the right direction. It solves the problem of
potential conflicts between different mux functions. Also it gives a
well defined interface between drivers and board code.

What I'm missing though is a generic way a single pad/mux mode
combination can be described. Let me take a look around how the
different subarchs do this:

omap:

	_OMAP3_MUXENTRY(DSS_DATA21, 91,
		"dss_data21", NULL, "mcspi3_cs0", "dss_data3",
		"gpio_91", NULL, NULL, "safe_mode"),
	_OMAP3_MUXENTRY(DSS_DATA22, 92,
		"dss_data22", NULL, "mcspi3_cs1", "dss_data4",
		"gpio_92", NULL, NULL, "safe_mode"),

pxa:

#define GPIO16_FFUART_TXD       MFP_CFG_OUT(GPIO16, AF3, DRIVE_HIGH)
#define GPIO37_FFUART_TXD       MFP_CFG_OUT(GPIO37, AF3, DRIVE_HIGH)
#define GPIO39_FFUART_TXD       MFP_CFG_OUT(GPIO39, AF2, DRIVE_HIGH)
#define GPIO83_FFUART_TXD       MFP_CFG_OUT(GPIO83, AF2, DRIVE_HIGH)
#define GPIO99_FFUART_TXD       MFP_CFG_OUT(GPIO99, AF3, DRIVE_HIGH)

i.MX:

#define _MX51_PAD_UART3_RXD__CSI1_D0            IOMUX_PAD(0x630, 0x240, 2, 0x0000, 0, 0)
#define _MX51_PAD_UART3_RXD__GPIO1_22           IOMUX_PAD(0x630, 0x240, 3, 0x0000, 0, 0)
#define _MX51_PAD_UART3_RXD__UART1_DTR          IOMUX_PAD(0x630, 0x240, 0, 0x0000, 0, 0)
#define _MX51_PAD_UART3_RXD__UART3_RXD          IOMUX_PAD(0x630, 0x240, 1, 0x09f4, 4, 0)
#define _MX51_PAD_UART3_TXD__CSI1_D1            IOMUX_PAD(0x634, 0x244, 2, 0x0000, 0, 0)

These all basically describe the same thing: put pad x into one of modes
a, b, c and apply certain flags like drive strength on this.

the other class of pin muxing I know of is that a whole group of pads
can be switched to a particular mode using a mux register like I think
is used used in your ux300 driver.

I'd like to have a unified way to describe this. If we ever want to move
this into the device tree we need this anyway as I think it's not an
option to have completely different SoC specific descriptions in the
device tree.

Do you think it's possible to do some consolidation on this level
aswell? It would really bring different SoCs more together.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-12  7:44   ` Sascha Hauer
@ 2011-05-12  9:40     ` Tony Lindgren
  -1 siblings, 0 replies; 48+ messages in thread
From: Tony Lindgren @ 2011-05-12  9:40 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Linus Walleij, linux-kernel, linux-arm-kernel, Grant Likely,
	Martin Persson, Linus Walleij, Lee Jones

* Sascha Hauer <s.hauer@pengutronix.de> [110512 00:40]:
> 
> These all basically describe the same thing: put pad x into one of modes
> a, b, c and apply certain flags like drive strength on this.
> 
> the other class of pin muxing I know of is that a whole group of pads
> can be switched to a particular mode using a mux register like I think
> is used used in your ux300 driver.
> 
> I'd like to have a unified way to describe this. If we ever want to move
> this into the device tree we need this anyway as I think it's not an
> option to have completely different SoC specific descriptions in the
> device tree.

Me too. Otherwise we'll have multiple different device tree implementations.

We might as well have the data in suitable format for device tree to
start with, and then have common access functions that can be replaced
for various platforms if necessary.

Regards,

Tony

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-12  9:40     ` Tony Lindgren
  0 siblings, 0 replies; 48+ messages in thread
From: Tony Lindgren @ 2011-05-12  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

* Sascha Hauer <s.hauer@pengutronix.de> [110512 00:40]:
> 
> These all basically describe the same thing: put pad x into one of modes
> a, b, c and apply certain flags like drive strength on this.
> 
> the other class of pin muxing I know of is that a whole group of pads
> can be switched to a particular mode using a mux register like I think
> is used used in your ux300 driver.
> 
> I'd like to have a unified way to describe this. If we ever want to move
> this into the device tree we need this anyway as I think it's not an
> option to have completely different SoC specific descriptions in the
> device tree.

Me too. Otherwise we'll have multiple different device tree implementations.

We might as well have the data in suitable format for device tree to
start with, and then have common access functions that can be replaced
for various platforms if necessary.

Regards,

Tony

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-12  7:44   ` Sascha Hauer
@ 2011-05-12 14:02     ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-12 14:02 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-kernel, Grant Likely, Martin Persson, Lee Jones, linux-arm-kernel

2011/5/12 Sascha Hauer <s.hauer@pengutronix.de>:

> What I'm missing though is a generic way a single pad/mux mode
> combination can be described. Let me take a look around how the
> different subarchs do this:
>
> omap:
>
>        _OMAP3_MUXENTRY(DSS_DATA21, 91,
>                "dss_data21", NULL, "mcspi3_cs0", "dss_data3",
>                "gpio_91", NULL, NULL, "safe_mode"),
>        _OMAP3_MUXENTRY(DSS_DATA22, 92,
>                "dss_data22", NULL, "mcspi3_cs1", "dss_data4",
>                "gpio_92", NULL, NULL, "safe_mode"),
>
> pxa:
>
> #define GPIO16_FFUART_TXD       MFP_CFG_OUT(GPIO16, AF3, DRIVE_HIGH)
> #define GPIO37_FFUART_TXD       MFP_CFG_OUT(GPIO37, AF3, DRIVE_HIGH)
> #define GPIO39_FFUART_TXD       MFP_CFG_OUT(GPIO39, AF2, DRIVE_HIGH)
> #define GPIO83_FFUART_TXD       MFP_CFG_OUT(GPIO83, AF2, DRIVE_HIGH)
> #define GPIO99_FFUART_TXD       MFP_CFG_OUT(GPIO99, AF3, DRIVE_HIGH)
>
> i.MX:
>
> #define _MX51_PAD_UART3_RXD__CSI1_D0            IOMUX_PAD(0x630, 0x240, 2, 0x0000, 0, 0)
> #define _MX51_PAD_UART3_RXD__GPIO1_22           IOMUX_PAD(0x630, 0x240, 3, 0x0000, 0, 0)
> #define _MX51_PAD_UART3_RXD__UART1_DTR          IOMUX_PAD(0x630, 0x240, 0, 0x0000, 0, 0)
> #define _MX51_PAD_UART3_RXD__UART3_RXD          IOMUX_PAD(0x630, 0x240, 1, 0x09f4, 4, 0)
> #define _MX51_PAD_UART3_TXD__CSI1_D1            IOMUX_PAD(0x634, 0x244, 2, 0x0000, 0, 0)
>
> These all basically describe the same thing: put pad x into one of modes
> a, b, c and apply certain flags like drive strength on this.
>
> the other class of pin muxing I know of is that a whole group of pads
> can be switched to a particular mode using a mux register like I think
> is used used in your ux300 driver.
>
> I'd like to have a unified way to describe this.

Hm, so some of the structure I currently have inside the specific U300 driver
need to become generic, in such way that say by activating 8 different
padmux functions at the same, this can boil down to a single register
write instead of 8 different register writes?

> Do you think it's possible to do some consolidation on this level
> aswell? It would really bring different SoCs more together.

I am thinking on the abstract level, now we would have:

Board:
static struct pinmux_map pmx_map[] = {
        PINMUX_MAP("foo0", "foo0-1"),
        PINMUX_MAP("bar0", "bar0-1"),
};
pinmux_register_mappings(pmx_map, ARRAY_SIZE(pmx_map));

Driver:
pmx = pinmux_get("foo0", NULL);
pinmux_enable(pmx);

For each of the mux functions. Now we need a grouping of
these functions.

So if I invent say pinmux function groups and add an argument
to pinmux_register_mappings() so it takes an optional groupname
arg, you can add several mappings and group them:

pinmux_register_mappings("foogrp", pmx_map1, ARRAY_SIZE(pmx_map1));
pinmux_register_mappings("bargrp", pmx_map2, ARRAY_SIZE(pmx_map2));

The pinmux core remember this association and add a
new API:

struct pinmux_group *pmxgrp = pinmux_group_get("foogrp");
pinmux_group_enable(pmxgrp);
pinmux_group_control(pmxgrp, N, N);
pinmux_group_disable(pmxgrp);
pinmux_group_put(pmxgrp);

If then the driver API adds an optional hook like this:
int (*enable_group) (struct pinmux_dev *pmxdev, unsigned *selector,
unsigned num_selectors);
void (*disable_group) (struct pinmux_dev *pmxdev, unsigned *selector,
unsigned num_selectors);

This way the driver can provide each muxing individually, the core keep
track of any grouping and making sure group sets does not clash,
and the driver can implement optional enable/disable calls that write entire
groups of pins at once and the driver knows how to combine these into a
few single register writes.

Would this work?

I see it as a clear, backwards-compatible superset to the current
patchset, so can go in a separate add-on if the current stuff go in first,
so we don't try to drink the entire ocean at once.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-12 14:02     ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-12 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/12 Sascha Hauer <s.hauer@pengutronix.de>:

> What I'm missing though is a generic way a single pad/mux mode
> combination can be described. Let me take a look around how the
> different subarchs do this:
>
> omap:
>
> ? ? ? ?_OMAP3_MUXENTRY(DSS_DATA21, 91,
> ? ? ? ? ? ? ? ?"dss_data21", NULL, "mcspi3_cs0", "dss_data3",
> ? ? ? ? ? ? ? ?"gpio_91", NULL, NULL, "safe_mode"),
> ? ? ? ?_OMAP3_MUXENTRY(DSS_DATA22, 92,
> ? ? ? ? ? ? ? ?"dss_data22", NULL, "mcspi3_cs1", "dss_data4",
> ? ? ? ? ? ? ? ?"gpio_92", NULL, NULL, "safe_mode"),
>
> pxa:
>
> #define GPIO16_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO16, AF3, DRIVE_HIGH)
> #define GPIO37_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO37, AF3, DRIVE_HIGH)
> #define GPIO39_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO39, AF2, DRIVE_HIGH)
> #define GPIO83_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO83, AF2, DRIVE_HIGH)
> #define GPIO99_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO99, AF3, DRIVE_HIGH)
>
> i.MX:
>
> #define _MX51_PAD_UART3_RXD__CSI1_D0 ? ? ? ? ? ?IOMUX_PAD(0x630, 0x240, 2, 0x0000, 0, 0)
> #define _MX51_PAD_UART3_RXD__GPIO1_22 ? ? ? ? ? IOMUX_PAD(0x630, 0x240, 3, 0x0000, 0, 0)
> #define _MX51_PAD_UART3_RXD__UART1_DTR ? ? ? ? ?IOMUX_PAD(0x630, 0x240, 0, 0x0000, 0, 0)
> #define _MX51_PAD_UART3_RXD__UART3_RXD ? ? ? ? ?IOMUX_PAD(0x630, 0x240, 1, 0x09f4, 4, 0)
> #define _MX51_PAD_UART3_TXD__CSI1_D1 ? ? ? ? ? ?IOMUX_PAD(0x634, 0x244, 2, 0x0000, 0, 0)
>
> These all basically describe the same thing: put pad x into one of modes
> a, b, c and apply certain flags like drive strength on this.
>
> the other class of pin muxing I know of is that a whole group of pads
> can be switched to a particular mode using a mux register like I think
> is used used in your ux300 driver.
>
> I'd like to have a unified way to describe this.

Hm, so some of the structure I currently have inside the specific U300 driver
need to become generic, in such way that say by activating 8 different
padmux functions at the same, this can boil down to a single register
write instead of 8 different register writes?

> Do you think it's possible to do some consolidation on this level
> aswell? It would really bring different SoCs more together.

I am thinking on the abstract level, now we would have:

Board:
static struct pinmux_map pmx_map[] = {
        PINMUX_MAP("foo0", "foo0-1"),
        PINMUX_MAP("bar0", "bar0-1"),
};
pinmux_register_mappings(pmx_map, ARRAY_SIZE(pmx_map));

Driver:
pmx = pinmux_get("foo0", NULL);
pinmux_enable(pmx);

For each of the mux functions. Now we need a grouping of
these functions.

So if I invent say pinmux function groups and add an argument
to pinmux_register_mappings() so it takes an optional groupname
arg, you can add several mappings and group them:

pinmux_register_mappings("foogrp", pmx_map1, ARRAY_SIZE(pmx_map1));
pinmux_register_mappings("bargrp", pmx_map2, ARRAY_SIZE(pmx_map2));

The pinmux core remember this association and add a
new API:

struct pinmux_group *pmxgrp = pinmux_group_get("foogrp");
pinmux_group_enable(pmxgrp);
pinmux_group_control(pmxgrp, N, N);
pinmux_group_disable(pmxgrp);
pinmux_group_put(pmxgrp);

If then the driver API adds an optional hook like this:
int (*enable_group) (struct pinmux_dev *pmxdev, unsigned *selector,
unsigned num_selectors);
void (*disable_group) (struct pinmux_dev *pmxdev, unsigned *selector,
unsigned num_selectors);

This way the driver can provide each muxing individually, the core keep
track of any grouping and making sure group sets does not clash,
and the driver can implement optional enable/disable calls that write entire
groups of pins at once and the driver knows how to combine these into a
few single register writes.

Would this work?

I see it as a clear, backwards-compatible superset to the current
patchset, so can go in a separate add-on if the current stuff go in first,
so we don't try to drink the entire ocean at once.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* RE : [PATCH 0/4] Pinmux subsystem
  2011-05-12 14:02     ` Linus Walleij
@ 2011-05-12 21:17       ` Matthieu Castet
  -1 siblings, 0 replies; 48+ messages in thread
From: Matthieu Castet @ 2011-05-12 21:17 UTC (permalink / raw)
  To: Linus Walleij, Sascha Hauer
  Cc: Grant Likely, Martin Persson, Lee Jones, linux-kernel, linux-arm-kernel

+In this 8x8 BGA package the pins { A8, A7, A6, A5 } can be used as an SPI port
+(these are four pins: CLK, RXD, TXD, FRM). In that case, pin B5 can be used as
+some general-purpose GPIO pin. However, in another setting, pins { A5, B5 } can
+be used as an I2C port (these are just two pins: SCL, SDA). Needless to say,
+we cannot use the SPI port and I2C port at the same time. However in the inside
+of the package the silicon performing the SPI logic can alternatively be routed
+out on pins { G4, G3, G2, G1 }.

Enumerating all possible case will be impossible because of the number of possible cases 
(and hardware guys can be very creative).

If spi can be in  { A8, A7, A6, A5 } and 
{ G4, G3, G2, G1 }, Then you can output the spi on :
- { A8, A7, A6, A5 }
- { A8, A7, A6, G1 }
- { A8, A7, G2, A5 }
[...]
- { G4, G3, G2, A5 }
- { G4, G3, G2, G1 }
You have 2^4 = 16 cases

Now RXD (MISO) or/and FRM (CS) can be not connected and used as a gpio. You have 4 * 16 cases = 64 cases.

Now take a complex chip, 200 balls 4 mux per ball and you can have up to 4^200 configurations...

Pin muxing is really board specific  and shouldn't be in a "generic" driver.

But what you could abstract is a way to select a configuration of a pin, not a group of pin
 for the board files.

Matthieu

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-12 21:17       ` Matthieu Castet
  0 siblings, 0 replies; 48+ messages in thread
From: Matthieu Castet @ 2011-05-12 21:17 UTC (permalink / raw)
  To: linux-arm-kernel

+In this 8x8 BGA package the pins { A8, A7, A6, A5 } can be used as an SPI port
+(these are four pins: CLK, RXD, TXD, FRM). In that case, pin B5 can be used as
+some general-purpose GPIO pin. However, in another setting, pins { A5, B5 } can
+be used as an I2C port (these are just two pins: SCL, SDA). Needless to say,
+we cannot use the SPI port and I2C port at the same time. However in the inside
+of the package the silicon performing the SPI logic can alternatively be routed
+out on pins { G4, G3, G2, G1 }.

Enumerating all possible case will be impossible because of the number of possible cases 
(and hardware guys can be very creative).

If spi can be in  { A8, A7, A6, A5 } and 
{ G4, G3, G2, G1 }, Then you can output the spi on :
- { A8, A7, A6, A5 }
- { A8, A7, A6, G1 }
- { A8, A7, G2, A5 }
[...]
- { G4, G3, G2, A5 }
- { G4, G3, G2, G1 }
You have 2^4 = 16 cases

Now RXD (MISO) or/and FRM (CS) can be not connected and used as a gpio. You have 4 * 16 cases = 64 cases.

Now take a complex chip, 200 balls 4 mux per ball and you can have up to 4^200 configurations...

Pin muxing is really board specific  and shouldn't be in a "generic" driver.

But what you could abstract is a way to select a configuration of a pin, not a group of pin
 for the board files.

Matthieu

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: RE : [PATCH 0/4] Pinmux subsystem
  2011-05-12 21:17       ` Matthieu Castet
@ 2011-05-13  7:05         ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-13  7:05 UTC (permalink / raw)
  To: Matthieu Castet
  Cc: Sascha Hauer, Grant Likely, Martin Persson, Lee Jones,
	linux-kernel, linux-arm-kernel

2011/5/12 Matthieu Castet <matthieu.castet@parrot.com>:

> Enumerating all possible case will be impossible because of the number of possible cases
> (and hardware guys can be very creative).

As mentioned in the document the subsystem is not about discrete
mathematics, i.e
we have no intent on enumerating every possible mux setting, only the ones that
are relevant for your electronics at hand.

> If spi can be in  { A8, A7, A6, A5 } and
> { G4, G3, G2, G1 }, Then you can output the spi on :
> - { A8, A7, A6, A5 }
> - { A8, A7, A6, G1 }
> - { A8, A7, G2, A5 }
> [...]
> - { G4, G3, G2, A5 }
> - { G4, G3, G2, G1 }
> You have 2^4 = 16 cases

Do you use all of them in practice?

> Pin muxing is really board specific  and shouldn't be in a "generic" driver.

It is rather package (PGA/BGA etc) specific than board specific. The board
is about what of the packaging options you actually use. As
mentioned in previous discussions you can pass in the actual configuration
of the mux settings from platform data, if we have device tree we can let the
board file dts inherit a package descriptor. All of this outside the
kernel tree.

So we define the function groups for the package that will actually be used
by the devices in the board files that we have.

And my first assumption is that those really aren't that many, and my second
assumption is that you would still have to have board-specific code to handle
every individual pin somewhere under mach-xxxx and this is what we're
currently trying to get away from.

> But what you could abstract is a way to select a configuration of a pin,
> not a group of pin for the board files.

The groups of pins are used when you're muxing devices, usually these use
more than one pin. And that is why we connect them to the devices
themselves with a mapping.

You can still allocate and mux individual pins for GPIO. Are there other
single-pin usecases?

Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-13  7:05         ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-13  7:05 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/12 Matthieu Castet <matthieu.castet@parrot.com>:

> Enumerating all possible case will be impossible because of the number of possible cases
> (and hardware guys can be very creative).

As mentioned in the document the subsystem is not about discrete
mathematics, i.e
we have no intent on enumerating every possible mux setting, only the ones that
are relevant for your electronics at hand.

> If spi can be in ?{ A8, A7, A6, A5 } and
> { G4, G3, G2, G1 }, Then you can output the spi on :
> - { A8, A7, A6, A5 }
> - { A8, A7, A6, G1 }
> - { A8, A7, G2, A5 }
> [...]
> - { G4, G3, G2, A5 }
> - { G4, G3, G2, G1 }
> You have 2^4 = 16 cases

Do you use all of them in practice?

> Pin muxing is really board specific ?and shouldn't be in a "generic" driver.

It is rather package (PGA/BGA etc) specific than board specific. The board
is about what of the packaging options you actually use. As
mentioned in previous discussions you can pass in the actual configuration
of the mux settings from platform data, if we have device tree we can let the
board file dts inherit a package descriptor. All of this outside the
kernel tree.

So we define the function groups for the package that will actually be used
by the devices in the board files that we have.

And my first assumption is that those really aren't that many, and my second
assumption is that you would still have to have board-specific code to handle
every individual pin somewhere under mach-xxxx and this is what we're
currently trying to get away from.

> But what you could abstract is a way to select a configuration of a pin,
> not a group of pin for the board files.

The groups of pins are used when you're muxing devices, usually these use
more than one pin. And that is why we connect them to the devices
themselves with a mapping.

You can still allocate and mux individual pins for GPIO. Are there other
single-pin usecases?

Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-12 14:02     ` Linus Walleij
@ 2011-05-13  9:59       ` Sascha Hauer
  -1 siblings, 0 replies; 48+ messages in thread
From: Sascha Hauer @ 2011-05-13  9:59 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Grant Likely, Martin Persson, Lee Jones, linux-kernel, linux-arm-kernel

On Thu, May 12, 2011 at 04:02:35PM +0200, Linus Walleij wrote:
> 2011/5/12 Sascha Hauer <s.hauer@pengutronix.de>:
> 
> > What I'm missing though is a generic way a single pad/mux mode
> > combination can be described. Let me take a look around how the
> > different subarchs do this:
> >
> > omap:
> >
> >        _OMAP3_MUXENTRY(DSS_DATA21, 91,
> >                "dss_data21", NULL, "mcspi3_cs0", "dss_data3",
> >                "gpio_91", NULL, NULL, "safe_mode"),
> >        _OMAP3_MUXENTRY(DSS_DATA22, 92,
> >                "dss_data22", NULL, "mcspi3_cs1", "dss_data4",
> >                "gpio_92", NULL, NULL, "safe_mode"),
> >
> > pxa:
> >
> > #define GPIO16_FFUART_TXD       MFP_CFG_OUT(GPIO16, AF3, DRIVE_HIGH)
> > #define GPIO37_FFUART_TXD       MFP_CFG_OUT(GPIO37, AF3, DRIVE_HIGH)
> > #define GPIO39_FFUART_TXD       MFP_CFG_OUT(GPIO39, AF2, DRIVE_HIGH)
> > #define GPIO83_FFUART_TXD       MFP_CFG_OUT(GPIO83, AF2, DRIVE_HIGH)
> > #define GPIO99_FFUART_TXD       MFP_CFG_OUT(GPIO99, AF3, DRIVE_HIGH)
> >
> > i.MX:
> >
> > #define _MX51_PAD_UART3_RXD__CSI1_D0            IOMUX_PAD(0x630, 0x240, 2, 0x0000, 0, 0)
> > #define _MX51_PAD_UART3_RXD__GPIO1_22           IOMUX_PAD(0x630, 0x240, 3, 0x0000, 0, 0)
> > #define _MX51_PAD_UART3_RXD__UART1_DTR          IOMUX_PAD(0x630, 0x240, 0, 0x0000, 0, 0)
> > #define _MX51_PAD_UART3_RXD__UART3_RXD          IOMUX_PAD(0x630, 0x240, 1, 0x09f4, 4, 0)
> > #define _MX51_PAD_UART3_TXD__CSI1_D1            IOMUX_PAD(0x634, 0x244, 2, 0x0000, 0, 0)
> >
> > These all basically describe the same thing: put pad x into one of modes
> > a, b, c and apply certain flags like drive strength on this.
> >
> > the other class of pin muxing I know of is that a whole group of pads
> > can be switched to a particular mode using a mux register like I think
> > is used used in your ux300 driver.
> >
> > I'd like to have a unified way to describe this.
> 
> Hm, so some of the structure I currently have inside the specific U300 driver
> need to become generic, in such way that say by activating 8 different
> padmux functions at the same, this can boil down to a single register
> write instead of 8 different register writes?
> 
> > Do you think it's possible to do some consolidation on this level
> > aswell? It would really bring different SoCs more together.
> 
> I am thinking on the abstract level, now we would have:
> 
> Board:
> static struct pinmux_map pmx_map[] = {
>         PINMUX_MAP("foo0", "foo0-1"),
>         PINMUX_MAP("bar0", "bar0-1"),
> };
> pinmux_register_mappings(pmx_map, ARRAY_SIZE(pmx_map));
> 
> Driver:
> pmx = pinmux_get("foo0", NULL);
> pinmux_enable(pmx);
> 
> For each of the mux functions. Now we need a grouping of
> these functions.

No, the driver API is fine. pinmux_enable can enable multiple pins in
the background and in fact it already does in your U300 example since
the hardware does not allow handling single pins.

What I'm asking for is to get rid of the differing definitions for
pin muxing. That is more an extension to your patches rather than
a change to your patches.

What omap/i.MX/pxa.. need is something like this:

/* describe all functions of a single pad, SoC or package specific */
struct padmux {
	int padno;
	const char *function; /* "uart1-txd", "spi3-mosi", "gpio127" */
	int num_functions;
};

/* select a particular function and additional settings */
struct function_selector {
	int padno;
	int function;	/* function to select from struct padmux above
			 * (could be const char * to match the function name
			 * instead of the number.
			 */
	unsigned long flags; /* drive strength, open drain, ... */
};

/*
 * map multiple pads to a device. Can be passed by the board or
 * filled from device tree.
 */
struct padgroup {
	const char *devname; /* imxspi.0, atmeluart.1,... */
	struct function_selector *functions;
	int num_functions;
};

(I don't know whether we can agree on a common set of flags, at least I
hope so)

I hope that we can agree on this or a similar set of structs to share
between different architectures.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-13  9:59       ` Sascha Hauer
  0 siblings, 0 replies; 48+ messages in thread
From: Sascha Hauer @ 2011-05-13  9:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 12, 2011 at 04:02:35PM +0200, Linus Walleij wrote:
> 2011/5/12 Sascha Hauer <s.hauer@pengutronix.de>:
> 
> > What I'm missing though is a generic way a single pad/mux mode
> > combination can be described. Let me take a look around how the
> > different subarchs do this:
> >
> > omap:
> >
> > ? ? ? ?_OMAP3_MUXENTRY(DSS_DATA21, 91,
> > ? ? ? ? ? ? ? ?"dss_data21", NULL, "mcspi3_cs0", "dss_data3",
> > ? ? ? ? ? ? ? ?"gpio_91", NULL, NULL, "safe_mode"),
> > ? ? ? ?_OMAP3_MUXENTRY(DSS_DATA22, 92,
> > ? ? ? ? ? ? ? ?"dss_data22", NULL, "mcspi3_cs1", "dss_data4",
> > ? ? ? ? ? ? ? ?"gpio_92", NULL, NULL, "safe_mode"),
> >
> > pxa:
> >
> > #define GPIO16_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO16, AF3, DRIVE_HIGH)
> > #define GPIO37_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO37, AF3, DRIVE_HIGH)
> > #define GPIO39_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO39, AF2, DRIVE_HIGH)
> > #define GPIO83_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO83, AF2, DRIVE_HIGH)
> > #define GPIO99_FFUART_TXD ? ? ? MFP_CFG_OUT(GPIO99, AF3, DRIVE_HIGH)
> >
> > i.MX:
> >
> > #define _MX51_PAD_UART3_RXD__CSI1_D0 ? ? ? ? ? ?IOMUX_PAD(0x630, 0x240, 2, 0x0000, 0, 0)
> > #define _MX51_PAD_UART3_RXD__GPIO1_22 ? ? ? ? ? IOMUX_PAD(0x630, 0x240, 3, 0x0000, 0, 0)
> > #define _MX51_PAD_UART3_RXD__UART1_DTR ? ? ? ? ?IOMUX_PAD(0x630, 0x240, 0, 0x0000, 0, 0)
> > #define _MX51_PAD_UART3_RXD__UART3_RXD ? ? ? ? ?IOMUX_PAD(0x630, 0x240, 1, 0x09f4, 4, 0)
> > #define _MX51_PAD_UART3_TXD__CSI1_D1 ? ? ? ? ? ?IOMUX_PAD(0x634, 0x244, 2, 0x0000, 0, 0)
> >
> > These all basically describe the same thing: put pad x into one of modes
> > a, b, c and apply certain flags like drive strength on this.
> >
> > the other class of pin muxing I know of is that a whole group of pads
> > can be switched to a particular mode using a mux register like I think
> > is used used in your ux300 driver.
> >
> > I'd like to have a unified way to describe this.
> 
> Hm, so some of the structure I currently have inside the specific U300 driver
> need to become generic, in such way that say by activating 8 different
> padmux functions at the same, this can boil down to a single register
> write instead of 8 different register writes?
> 
> > Do you think it's possible to do some consolidation on this level
> > aswell? It would really bring different SoCs more together.
> 
> I am thinking on the abstract level, now we would have:
> 
> Board:
> static struct pinmux_map pmx_map[] = {
>         PINMUX_MAP("foo0", "foo0-1"),
>         PINMUX_MAP("bar0", "bar0-1"),
> };
> pinmux_register_mappings(pmx_map, ARRAY_SIZE(pmx_map));
> 
> Driver:
> pmx = pinmux_get("foo0", NULL);
> pinmux_enable(pmx);
> 
> For each of the mux functions. Now we need a grouping of
> these functions.

No, the driver API is fine. pinmux_enable can enable multiple pins in
the background and in fact it already does in your U300 example since
the hardware does not allow handling single pins.

What I'm asking for is to get rid of the differing definitions for
pin muxing. That is more an extension to your patches rather than
a change to your patches.

What omap/i.MX/pxa.. need is something like this:

/* describe all functions of a single pad, SoC or package specific */
struct padmux {
	int padno;
	const char *function; /* "uart1-txd", "spi3-mosi", "gpio127" */
	int num_functions;
};

/* select a particular function and additional settings */
struct function_selector {
	int padno;
	int function;	/* function to select from struct padmux above
			 * (could be const char * to match the function name
			 * instead of the number.
			 */
	unsigned long flags; /* drive strength, open drain, ... */
};

/*
 * map multiple pads to a device. Can be passed by the board or
 * filled from device tree.
 */
struct padgroup {
	const char *devname; /* imxspi.0, atmeluart.1,... */
	struct function_selector *functions;
	int num_functions;
};

(I don't know whether we can agree on a common set of flags, at least I
hope so)

I hope that we can agree on this or a similar set of structs to share
between different architectures.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: RE : [PATCH 0/4] Pinmux subsystem
  2011-05-13  7:05         ` Linus Walleij
@ 2011-05-13 16:03           ` Matthieu CASTET
  -1 siblings, 0 replies; 48+ messages in thread
From: Matthieu CASTET @ 2011-05-13 16:03 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sascha Hauer, Grant Likely, Martin Persson, Lee Jones,
	linux-kernel, linux-arm-kernel

Linus Walleij a écrit :
> 2011/5/12 Matthieu Castet <matthieu.castet@parrot.com>:
> 
>> Enumerating all possible case will be impossible because of the number of possible cases
>> (and hardware guys can be very creative).
> 
> As mentioned in the document the subsystem is not about discrete
> mathematics, i.e
> we have no intent on enumerating every possible mux setting, only the ones that
> are relevant for your electronics at hand.
> 
>> If spi can be in  { A8, A7, A6, A5 } and
>> { G4, G3, G2, G1 }, Then you can output the spi on :
>> - { A8, A7, A6, A5 }
>> - { A8, A7, A6, G1 }
>> - { A8, A7, G2, A5 }
>> [...]
>> - { G4, G3, G2, A5 }
>> - { G4, G3, G2, G1 }
>> You have 2^4 = 16 cases
> 
> Do you use all of them in practice?
Of course not all, but more than the 2 { A8, A7, A6, A5 } or { G4, G3, G2, G1 }.

Also when we doing the bsp for a processor, it is better to allow flexibility
for future board.

So with your pin mux how to you handle the fact that on spi you can have some
signal not connected ?
For { A8, A7, A6, A5 } spi, some board want all 4 spi wire, other want 3 (CS,
MOSI, CLK) or (MISO, CS, CLK), other want 2.

This is board specific not package specific.

And that's doesn't apply only to spi. That's the same problem for uart (no
rts/cts), sdcard (one data vs 4 data), ...



> 
>> Pin muxing is really board specific  and shouldn't be in a "generic" driver.
> 
> It is rather package (PGA/BGA etc) specific than board specific. The board
> is about what of the packaging options you actually use. As
> mentioned in previous discussions you can pass in the actual configuration
> of the mux settings from platform data, if we have device tree we can let the
> board file dts inherit a package descriptor. All of this outside the
> kernel tree.
> 
> So we define the function groups for the package that will actually be used
> by the devices in the board files that we have.
> 
> And my first assumption is that those really aren't that many, and my second
> assumption is that you would still have to have board-specific code to handle
> every individual pin somewhere under mach-xxxx and this is what we're
> currently trying to get away from.
> 
>> But what you could abstract is a way to select a configuration of a pin,
>> not a group of pin for the board files.
> 
> The groups of pins are used when you're muxing devices, usually these use
> more than one pin. And that is why we connect them to the devices
> themselves with a mapping.
I believe there should be 2 different things :
- something for select pin. Omap stuff is nice : omap3_mux_init,
omap_mux_init_gpio, omap_mux_init_signal, ...
- something for grouping pins, but the board can add new group of pin if it
doesn't exist.


Also what i don't like in your system is the naming :
> +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 };
> +static unsigned int i2c0_pins[] = { 24, 25 };
> +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 };
What's 0, 8, 16, .... It should be define.

> +static struct foo_pmx_func myfuncs[] = {
> +       {
> +               .name = "spi0-0",
> +               .pins = spi0_0_pins,
> +               .num_pins = ARRAY_SIZE(spi0_1_pins),
> +       },
> +       {
> +               .name = "i2c0",
> +               .pins = i2c0_pins,
> +               .num_pins = ARRAY_SIZE(i2c0_pins),
> +       },
> +       {
> +               .name = "spi0-1",
> +               .pins = spi0_1_pins,
> +               .num_pins = ARRAY_SIZE(spi0_1_pins),
> +       },
> +};
How I am supposed to know what's spi0-0, i2c0, spi0-1 without reading the code ?

> +foo_probe()
> +{
> +       /* Allocate a state holder named "state" etc */
> +       struct pinmux pmx;
> +
> +       pmx = pinmux_get(&device, NULL);
> +       if IS_ERR(pmx)
> +               return PTR_ERR(pmx);
> +       pinmux_enable(pmx);
> +
> +       state->pmx = pmx;
> +}
> +If you want a specific mux setting and not just the first one found for this
> +device you can specify a specific mux setting, for example in the above example
> +the second i2c0 setting: pinmux_get(&device, "spi0-2");

How a driver that is generic for example sdchi, mmci, ... is supposed to know
the pinmux name ?
How this work if the board want a special mux ? I have to modify also the driver ?


Matthieu



^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-13 16:03           ` Matthieu CASTET
  0 siblings, 0 replies; 48+ messages in thread
From: Matthieu CASTET @ 2011-05-13 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

Linus Walleij a ?crit :
> 2011/5/12 Matthieu Castet <matthieu.castet@parrot.com>:
> 
>> Enumerating all possible case will be impossible because of the number of possible cases
>> (and hardware guys can be very creative).
> 
> As mentioned in the document the subsystem is not about discrete
> mathematics, i.e
> we have no intent on enumerating every possible mux setting, only the ones that
> are relevant for your electronics at hand.
> 
>> If spi can be in  { A8, A7, A6, A5 } and
>> { G4, G3, G2, G1 }, Then you can output the spi on :
>> - { A8, A7, A6, A5 }
>> - { A8, A7, A6, G1 }
>> - { A8, A7, G2, A5 }
>> [...]
>> - { G4, G3, G2, A5 }
>> - { G4, G3, G2, G1 }
>> You have 2^4 = 16 cases
> 
> Do you use all of them in practice?
Of course not all, but more than the 2 { A8, A7, A6, A5 } or { G4, G3, G2, G1 }.

Also when we doing the bsp for a processor, it is better to allow flexibility
for future board.

So with your pin mux how to you handle the fact that on spi you can have some
signal not connected ?
For { A8, A7, A6, A5 } spi, some board want all 4 spi wire, other want 3 (CS,
MOSI, CLK) or (MISO, CS, CLK), other want 2.

This is board specific not package specific.

And that's doesn't apply only to spi. That's the same problem for uart (no
rts/cts), sdcard (one data vs 4 data), ...



> 
>> Pin muxing is really board specific  and shouldn't be in a "generic" driver.
> 
> It is rather package (PGA/BGA etc) specific than board specific. The board
> is about what of the packaging options you actually use. As
> mentioned in previous discussions you can pass in the actual configuration
> of the mux settings from platform data, if we have device tree we can let the
> board file dts inherit a package descriptor. All of this outside the
> kernel tree.
> 
> So we define the function groups for the package that will actually be used
> by the devices in the board files that we have.
> 
> And my first assumption is that those really aren't that many, and my second
> assumption is that you would still have to have board-specific code to handle
> every individual pin somewhere under mach-xxxx and this is what we're
> currently trying to get away from.
> 
>> But what you could abstract is a way to select a configuration of a pin,
>> not a group of pin for the board files.
> 
> The groups of pins are used when you're muxing devices, usually these use
> more than one pin. And that is why we connect them to the devices
> themselves with a mapping.
I believe there should be 2 different things :
- something for select pin. Omap stuff is nice : omap3_mux_init,
omap_mux_init_gpio, omap_mux_init_signal, ...
- something for grouping pins, but the board can add new group of pin if it
doesn't exist.


Also what i don't like in your system is the naming :
> +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 };
> +static unsigned int i2c0_pins[] = { 24, 25 };
> +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 };
What's 0, 8, 16, .... It should be define.

> +static struct foo_pmx_func myfuncs[] = {
> +       {
> +               .name = "spi0-0",
> +               .pins = spi0_0_pins,
> +               .num_pins = ARRAY_SIZE(spi0_1_pins),
> +       },
> +       {
> +               .name = "i2c0",
> +               .pins = i2c0_pins,
> +               .num_pins = ARRAY_SIZE(i2c0_pins),
> +       },
> +       {
> +               .name = "spi0-1",
> +               .pins = spi0_1_pins,
> +               .num_pins = ARRAY_SIZE(spi0_1_pins),
> +       },
> +};
How I am supposed to know what's spi0-0, i2c0, spi0-1 without reading the code ?

> +foo_probe()
> +{
> +       /* Allocate a state holder named "state" etc */
> +       struct pinmux pmx;
> +
> +       pmx = pinmux_get(&device, NULL);
> +       if IS_ERR(pmx)
> +               return PTR_ERR(pmx);
> +       pinmux_enable(pmx);
> +
> +       state->pmx = pmx;
> +}
> +If you want a specific mux setting and not just the first one found for this
> +device you can specify a specific mux setting, for example in the above example
> +the second i2c0 setting: pinmux_get(&device, "spi0-2");

How a driver that is generic for example sdchi, mmci, ... is supposed to know
the pinmux name ?
How this work if the board want a special mux ? I have to modify also the driver ?


Matthieu

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: RE : [PATCH 0/4] Pinmux subsystem
  2011-05-13 16:03           ` Matthieu CASTET
@ 2011-05-14  7:57             ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-14  7:57 UTC (permalink / raw)
  To: Matthieu CASTET
  Cc: Sascha Hauer, linux-kernel, Grant Likely, Martin Persson,
	Lee Jones, linux-arm-kernel

2011/5/13 Matthieu CASTET <matthieu.castet@parrot.com>:
> Linus Walleij a écrit :
>> 2011/5/12 Matthieu Castet <matthieu.castet@parrot.com>:
>>> You have 2^4 = 16 cases
>>
>> Do you use all of them in practice?
> Of course not all, but more than the 2 { A8, A7, A6, A5 } or { G4, G3, G2, G1 }.
>
> Also when we doing the bsp for a processor, it is better to allow flexibility
> for future board.

Sorry I'm not following, what is inflexible in this case?

> So with your pin mux how to you handle the fact that on spi you can have some
> signal not connected ?
> For { A8, A7, A6, A5 } spi, some board want all 4 spi wire, other want 3 (CS,
> MOSI, CLK) or (MISO, CS, CLK), other want 2.

It's what I define as different functions from the pinmux:

Say if this is SPI0 that can use either 4, 3 or 2 wires you *can* call these
just "spi0-4wire", "spi0-3wire", "spi0-2wire". The only thing the pinmux
subsystem deals with it groups of pins, this is what is called functions.

It does not assume anything about how these functions are defined and
used, it just makes sure they do not overlap.

So with pinmux you can activate function "spi0-2wire" and then use the
two remaining pins as GPIO, no problem, it won't conflict since the
two free pins aren't part of that function.

> This is board specific not package specific.

It relates to how the stuff is connected to the packages I think, and whether
you call that package or board specific is no big issue. What I can say for
sure is if the package didn't have 4 wires wherof you could use only
two, you couldn't do any of this.

> And that's doesn't apply only to spi. That's the same problem for uart (no
> rts/cts), sdcard (one data vs 4 data), ...

No problem at all. You can define the functions you want.

>> The groups of pins are used when you're muxing devices, usually these use
>> more than one pin. And that is why we connect them to the devices
>> themselves with a mapping.
>
> I believe there should be 2 different things :
> - something for select pin. Omap stuff is nice : omap3_mux_init,
> omap_mux_init_gpio, omap_mux_init_signal, ...
> - something for grouping pins, but the board can add new group of pin if it
> doesn't exist.

Now, the framework does not deal with how the groups of pins, i.e. the
functions, are defined. Currently that is done by the drivers. The only
thing it deals with is handling conflicts among pins, and selecting
such groups.

If you want to create these groups from board data (what I call package
data) or say device tree, that is perfectly fine.

> Also what i don't like in your system is the naming :
>> +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 };
>> +static unsigned int i2c0_pins[] = { 24, 25 };
>> +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 };
> What's 0, 8, 16, .... It should be define.

Again this is an example of a simple driver, it's entirely up to your
driver to do. There are many similar example in Documentation/*
where we don't use #define to simplify examples.

>> +static struct foo_pmx_func myfuncs[] = {
>> +       {
>> +               .name = "spi0-0",
>> +               .pins = spi0_0_pins,
>> +               .num_pins = ARRAY_SIZE(spi0_1_pins),
>> +       },
>> +       {
>> +               .name = "i2c0",
>> +               .pins = i2c0_pins,
>> +               .num_pins = ARRAY_SIZE(i2c0_pins),
>> +       },
>> +       {
>> +               .name = "spi0-1",
>> +               .pins = spi0_1_pins,
>> +               .num_pins = ARRAY_SIZE(spi0_1_pins),
>> +       },
>> +};
>
> How I am supposed to know what's spi0-0, i2c0, spi0-1 without reading the code ?

Again this is an example. If this data came from the device tree for
example, or from arch/arm/mach-foo/package-db3350.c  in some
struct db3350_pin_platform_data{} that I come up with does not
matter to the framework.

It's no different from how say your MMC driver knows how to handle
which GPIO pin is card detect, you have to tell it what pin it is, likewise
you can tell your pinmux driver what pins to handle.

>> +foo_probe()
>> +{
>> +       /* Allocate a state holder named "state" etc */
>> +       struct pinmux pmx;
>> +
>> +       pmx = pinmux_get(&device, NULL);
>> +       if IS_ERR(pmx)
>> +               return PTR_ERR(pmx);
>> +       pinmux_enable(pmx);
>> +
>> +       state->pmx = pmx;
>> +}
>> +If you want a specific mux setting and not just the first one found for this
>> +device you can specify a specific mux setting, for example in the above example
>> +the second i2c0 setting: pinmux_get(&device, "spi0-2");
>
> How a driver that is generic for example sdchi, mmci, ... is supposed to know
> the pinmux name ?

Since the board define function mappings such as:

PINMUX_MAP("sdi0-my-group", "sdi0.0"),

The driver can reference that one association:

pinmux_get(dev, NULL);

If you have different muxings you can also use an optional function
name. This is no different to how you get clocks with clk_get(dev, NULL)
or regulators.

> How this work if the board want a special mux ? I have to modify also the driver ?

No I don't think so. If you mean specify new functions, you write your driver
so that your platform data/package data/device tree/etc can pass muxes in to
it. Don't lock on to the fact that the example driver does all that inside the
driver, it is an example only.

Imagine the same question for regulators. No different, just that the regulator
framework has some standard structure for how to pass in platform data
and I have not defined that. The clk framework has no such standard (yet)
and is comparable, if you had you clock driver in drivers/clk/foo.c you would
have the same problem.

If what you mean is that you want another muxing device, that is fully
supported in the same manner as we support several gpio_chip:s.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-14  7:57             ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-14  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/13 Matthieu CASTET <matthieu.castet@parrot.com>:
> Linus Walleij a ?crit :
>> 2011/5/12 Matthieu Castet <matthieu.castet@parrot.com>:
>>> You have 2^4 = 16 cases
>>
>> Do you use all of them in practice?
> Of course not all, but more than the 2 { A8, A7, A6, A5 } or { G4, G3, G2, G1 }.
>
> Also when we doing the bsp for a processor, it is better to allow flexibility
> for future board.

Sorry I'm not following, what is inflexible in this case?

> So with your pin mux how to you handle the fact that on spi you can have some
> signal not connected ?
> For { A8, A7, A6, A5 } spi, some board want all 4 spi wire, other want 3 (CS,
> MOSI, CLK) or (MISO, CS, CLK), other want 2.

It's what I define as different functions from the pinmux:

Say if this is SPI0 that can use either 4, 3 or 2 wires you *can* call these
just "spi0-4wire", "spi0-3wire", "spi0-2wire". The only thing the pinmux
subsystem deals with it groups of pins, this is what is called functions.

It does not assume anything about how these functions are defined and
used, it just makes sure they do not overlap.

So with pinmux you can activate function "spi0-2wire" and then use the
two remaining pins as GPIO, no problem, it won't conflict since the
two free pins aren't part of that function.

> This is board specific not package specific.

It relates to how the stuff is connected to the packages I think, and whether
you call that package or board specific is no big issue. What I can say for
sure is if the package didn't have 4 wires wherof you could use only
two, you couldn't do any of this.

> And that's doesn't apply only to spi. That's the same problem for uart (no
> rts/cts), sdcard (one data vs 4 data), ...

No problem at all. You can define the functions you want.

>> The groups of pins are used when you're muxing devices, usually these use
>> more than one pin. And that is why we connect them to the devices
>> themselves with a mapping.
>
> I believe there should be 2 different things :
> - something for select pin. Omap stuff is nice : omap3_mux_init,
> omap_mux_init_gpio, omap_mux_init_signal, ...
> - something for grouping pins, but the board can add new group of pin if it
> doesn't exist.

Now, the framework does not deal with how the groups of pins, i.e. the
functions, are defined. Currently that is done by the drivers. The only
thing it deals with is handling conflicts among pins, and selecting
such groups.

If you want to create these groups from board data (what I call package
data) or say device tree, that is perfectly fine.

> Also what i don't like in your system is the naming :
>> +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 };
>> +static unsigned int i2c0_pins[] = { 24, 25 };
>> +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 };
> What's 0, 8, 16, .... It should be define.

Again this is an example of a simple driver, it's entirely up to your
driver to do. There are many similar example in Documentation/*
where we don't use #define to simplify examples.

>> +static struct foo_pmx_func myfuncs[] = {
>> + ? ? ? {
>> + ? ? ? ? ? ? ? .name = "spi0-0",
>> + ? ? ? ? ? ? ? .pins = spi0_0_pins,
>> + ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(spi0_1_pins),
>> + ? ? ? },
>> + ? ? ? {
>> + ? ? ? ? ? ? ? .name = "i2c0",
>> + ? ? ? ? ? ? ? .pins = i2c0_pins,
>> + ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(i2c0_pins),
>> + ? ? ? },
>> + ? ? ? {
>> + ? ? ? ? ? ? ? .name = "spi0-1",
>> + ? ? ? ? ? ? ? .pins = spi0_1_pins,
>> + ? ? ? ? ? ? ? .num_pins = ARRAY_SIZE(spi0_1_pins),
>> + ? ? ? },
>> +};
>
> How I am supposed to know what's spi0-0, i2c0, spi0-1 without reading the code ?

Again this is an example. If this data came from the device tree for
example, or from arch/arm/mach-foo/package-db3350.c  in some
struct db3350_pin_platform_data{} that I come up with does not
matter to the framework.

It's no different from how say your MMC driver knows how to handle
which GPIO pin is card detect, you have to tell it what pin it is, likewise
you can tell your pinmux driver what pins to handle.

>> +foo_probe()
>> +{
>> + ? ? ? /* Allocate a state holder named "state" etc */
>> + ? ? ? struct pinmux pmx;
>> +
>> + ? ? ? pmx = pinmux_get(&device, NULL);
>> + ? ? ? if IS_ERR(pmx)
>> + ? ? ? ? ? ? ? return PTR_ERR(pmx);
>> + ? ? ? pinmux_enable(pmx);
>> +
>> + ? ? ? state->pmx = pmx;
>> +}
>> +If you want a specific mux setting and not just the first one found for this
>> +device you can specify a specific mux setting, for example in the above example
>> +the second i2c0 setting: pinmux_get(&device, "spi0-2");
>
> How a driver that is generic for example sdchi, mmci, ... is supposed to know
> the pinmux name ?

Since the board define function mappings such as:

PINMUX_MAP("sdi0-my-group", "sdi0.0"),

The driver can reference that one association:

pinmux_get(dev, NULL);

If you have different muxings you can also use an optional function
name. This is no different to how you get clocks with clk_get(dev, NULL)
or regulators.

> How this work if the board want a special mux ? I have to modify also the driver ?

No I don't think so. If you mean specify new functions, you write your driver
so that your platform data/package data/device tree/etc can pass muxes in to
it. Don't lock on to the fact that the example driver does all that inside the
driver, it is an example only.

Imagine the same question for regulators. No different, just that the regulator
framework has some standard structure for how to pass in platform data
and I have not defined that. The clk framework has no such standard (yet)
and is comparable, if you had you clock driver in drivers/clk/foo.c you would
have the same problem.

If what you mean is that you want another muxing device, that is fully
supported in the same manner as we support several gpio_chip:s.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-10 21:42     ` Linus Walleij
@ 2011-05-15 13:33       ` Andrew Lunn
  -1 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-15 13:33 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, linux-kernel, Grant Likely, Martin Persson,
	Lee Jones, linux-arm-kernel

Hi Folks

There was a question, how many different combinations would end up in
drivers/pinmux/pinmux-foo.c given the flexibility different devices
have?

It has been suggested there would be a drivers/pinmux/pinmux-orion.c
for all devices based around orion. That would be mach-kirkwood,
mach-dove, mach-loki, mach-mv78xx0, mach-orion5x. Currently loki and
dove have pin muxing code, but no board makes use of this. However
kirkwood, orion5x and mv78xx0 do have pin muxing in use.

With a bit of scripting, came to the following results. Each line is
on pinmux function description what would be needed in the driver. The
number at the front says how many boards would use it. In total there
are 33 descriptions needed. Of these, 18 are used only once. This to
me suggests the board needs to be able to specify a pin description
when there is no reuse with other boards.

      1 KIRKWOOD_MPP0_NF_IO2,KIRKWOOD_MPP1_NF_IO3,KIRKWOOD_MPP2_NF_IO4,KIRKWOOD_MPP3_NF_IO5,KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,KIRKWOOD_MPP18_NF_IO0,KIRKWOOD_MPP19_NF_IO1,
      1 KIRKWOOD_MPP0_NF_IO2,KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,KIRKWOOD_MPP18_NF_IO0,KIRKWOOD_MPP19_NF_IO1,
      1 KIRKWOOD_MPP12_SD_CLK,
      1 KIRKWOOD_MPP12_SD_CLK,KIRKWOOD_MPP13_SD_CMD,KIRKWOOD_MPP14_SD_D0,KIRKWOOD_MPP15_SD_D1,KIRKWOOD_MPP16_SD_D2,KIRKWOOD_MPP17_SD_D3,
      1 KIRKWOOD_MPP15_SATA0_ACTn,
      1 KIRKWOOD_MPP15_SATA0_ACTn,KIRKWOOD_MPP17_SATA0_PRESENTn,
      1 KIRKWOOD_MPP1_SPI_MOSI,KIRKWOOD_MPP2_SPI_SCK,KIRKWOOD_MPP3_SPI_MISO,KIRKWOOD_MPP7_SPI_SCn,
      1 KIRKWOOD_MPP33_GE1_TXCTL,
      1 KIRKWOOD_MPP39_AU_I2SBCLK,KIRKWOOD_MPP40_AU_I2SDO,KIRKWOOD_MPP43_AU_I2SDI,KIRKWOOD_MPP41_AU_I2SLRCLK,KIRKWOOD_MPP42_AU_I2SMCLK,
      1 KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,
      1 KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,KIRKWOOD_MPP18_NF_IO0,KIRKWOOD_MPP19_NF_IO1,
      1 KIRKWOOD_MPP4_SATA1_ACTn,
      1 KIRKWOOD_MPP5_SATA0_ACTn,
      1 MV78xx0_MPP0_GE1_TXCLK,MV78xx0_MPP1_GE1_TXCTL,MV78xx0_MPP2_GE1_RXCTL,MV78xx0_MPP3_GE1_RXCLK,MV78xx0_MPP4_GE1_TXD0,MV78xx0_MPP5_GE1_TXD1,MV78xx0_MPP6_GE1_TXD2,MV78xx0_MPP7_GE1_TXD3,MV78xx0_MPP8_GE1_RXD0,MV78xx0_MPP9_GE1_RXD1,MV78xx0_MPP10_GE1_RXD2,MV78xx0_MPP11_GE1_RXD3,
      1 MV78xx0_MPP13_SYSRST_OUTn,MV78xx0_MPP29_SYSRST_OUTn,
      1 MV78xx0_MPP14_SATA1_ACTn,MV78xx0_MPP48_SATA1_ACTn,
      1 MV78xx0_MPP15_SATA0_ACTn,MV78xx0_MPP49_SATA0_ACTn,
      1 ORION5X_MPP2_PCI_ARB,ORION5X_MPP3_PCI_ARB,ORION5X_MPP4_PCI_ARB,ORION5X_MPP5_PCI_ARB,
      2 KIRKWOOD_MPP16_SATA1_ACTn,
      2 KIRKWOOD_MPP20_SATA1_ACTn,
      2 KIRKWOOD_MPP7_PEX_RST_OUTn,
      2 ORION5X_MPP0_PCIE_RST_OUTn,
      2 ORION5X_MPP6_PCI_CLK,ORION5X_MPP7_PCI_CLK,
      3 KIRKWOOD_MPP13_UART1_TXD,KIRKWOOD_MPP14_UART1_RXD,
      3 KIRKWOOD_MPP20_GE1_TXD0,KIRKWOOD_MPP21_GE1_TXD1,KIRKWOOD_MPP22_GE1_TXD2,KIRKWOOD_MPP23_GE1_TXD3,KIRKWOOD_MPP24_GE1_RXD0,KIRKWOOD_MPP25_GE1_RXD1,KIRKWOOD_MPP26_GE1_RXD2,KIRKWOOD_MPP27_GE1_RXD3,KIRKWOOD_MPP30_GE1_RXCTL,KIRKWOOD_MPP31_GE1_RXCLK,KIRKWOOD_MPP32_GE1_TCLKOUT,KIRKWOOD_MPP33_GE1_TXCTL,
      3 KIRKWOOD_MPP21_SATA0_ACTn,
      3 ORION5X_MPP14_SATA_LED,ORION5X_MPP15_SATA_LED,
      7 KIRKWOOD_MPP0_SPI_SCn,KIRKWOOD_MPP1_SPI_MOSI,KIRKWOOD_MPP2_SPI_SCK,KIRKWOOD_MPP3_SPI_MISO,
      7 KIRKWOOD_MPP6_SYSRST_OUTn,
      7 ORION5X_MPP12_SATA_LED,ORION5X_MPP13_SATA_LED,ORION5X_MPP14_SATA_LED,ORION5X_MPP15_SATA_LED,
      8 KIRKWOOD_MPP10_UART0_TXD,KIRKWOOD_MPP11_UART0_RXD,
      8 KIRKWOOD_MPP8_TW0_SDA,KIRKWOOD_MPP9_TW0_SCK

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-15 13:33       ` Andrew Lunn
  0 siblings, 0 replies; 48+ messages in thread
From: Andrew Lunn @ 2011-05-15 13:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Folks

There was a question, how many different combinations would end up in
drivers/pinmux/pinmux-foo.c given the flexibility different devices
have?

It has been suggested there would be a drivers/pinmux/pinmux-orion.c
for all devices based around orion. That would be mach-kirkwood,
mach-dove, mach-loki, mach-mv78xx0, mach-orion5x. Currently loki and
dove have pin muxing code, but no board makes use of this. However
kirkwood, orion5x and mv78xx0 do have pin muxing in use.

With a bit of scripting, came to the following results. Each line is
on pinmux function description what would be needed in the driver. The
number at the front says how many boards would use it. In total there
are 33 descriptions needed. Of these, 18 are used only once. This to
me suggests the board needs to be able to specify a pin description
when there is no reuse with other boards.

      1 KIRKWOOD_MPP0_NF_IO2,KIRKWOOD_MPP1_NF_IO3,KIRKWOOD_MPP2_NF_IO4,KIRKWOOD_MPP3_NF_IO5,KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,KIRKWOOD_MPP18_NF_IO0,KIRKWOOD_MPP19_NF_IO1,
      1 KIRKWOOD_MPP0_NF_IO2,KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,KIRKWOOD_MPP18_NF_IO0,KIRKWOOD_MPP19_NF_IO1,
      1 KIRKWOOD_MPP12_SD_CLK,
      1 KIRKWOOD_MPP12_SD_CLK,KIRKWOOD_MPP13_SD_CMD,KIRKWOOD_MPP14_SD_D0,KIRKWOOD_MPP15_SD_D1,KIRKWOOD_MPP16_SD_D2,KIRKWOOD_MPP17_SD_D3,
      1 KIRKWOOD_MPP15_SATA0_ACTn,
      1 KIRKWOOD_MPP15_SATA0_ACTn,KIRKWOOD_MPP17_SATA0_PRESENTn,
      1 KIRKWOOD_MPP1_SPI_MOSI,KIRKWOOD_MPP2_SPI_SCK,KIRKWOOD_MPP3_SPI_MISO,KIRKWOOD_MPP7_SPI_SCn,
      1 KIRKWOOD_MPP33_GE1_TXCTL,
      1 KIRKWOOD_MPP39_AU_I2SBCLK,KIRKWOOD_MPP40_AU_I2SDO,KIRKWOOD_MPP43_AU_I2SDI,KIRKWOOD_MPP41_AU_I2SLRCLK,KIRKWOOD_MPP42_AU_I2SMCLK,
      1 KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,
      1 KIRKWOOD_MPP4_NF_IO6,KIRKWOOD_MPP5_NF_IO7,KIRKWOOD_MPP18_NF_IO0,KIRKWOOD_MPP19_NF_IO1,
      1 KIRKWOOD_MPP4_SATA1_ACTn,
      1 KIRKWOOD_MPP5_SATA0_ACTn,
      1 MV78xx0_MPP0_GE1_TXCLK,MV78xx0_MPP1_GE1_TXCTL,MV78xx0_MPP2_GE1_RXCTL,MV78xx0_MPP3_GE1_RXCLK,MV78xx0_MPP4_GE1_TXD0,MV78xx0_MPP5_GE1_TXD1,MV78xx0_MPP6_GE1_TXD2,MV78xx0_MPP7_GE1_TXD3,MV78xx0_MPP8_GE1_RXD0,MV78xx0_MPP9_GE1_RXD1,MV78xx0_MPP10_GE1_RXD2,MV78xx0_MPP11_GE1_RXD3,
      1 MV78xx0_MPP13_SYSRST_OUTn,MV78xx0_MPP29_SYSRST_OUTn,
      1 MV78xx0_MPP14_SATA1_ACTn,MV78xx0_MPP48_SATA1_ACTn,
      1 MV78xx0_MPP15_SATA0_ACTn,MV78xx0_MPP49_SATA0_ACTn,
      1 ORION5X_MPP2_PCI_ARB,ORION5X_MPP3_PCI_ARB,ORION5X_MPP4_PCI_ARB,ORION5X_MPP5_PCI_ARB,
      2 KIRKWOOD_MPP16_SATA1_ACTn,
      2 KIRKWOOD_MPP20_SATA1_ACTn,
      2 KIRKWOOD_MPP7_PEX_RST_OUTn,
      2 ORION5X_MPP0_PCIE_RST_OUTn,
      2 ORION5X_MPP6_PCI_CLK,ORION5X_MPP7_PCI_CLK,
      3 KIRKWOOD_MPP13_UART1_TXD,KIRKWOOD_MPP14_UART1_RXD,
      3 KIRKWOOD_MPP20_GE1_TXD0,KIRKWOOD_MPP21_GE1_TXD1,KIRKWOOD_MPP22_GE1_TXD2,KIRKWOOD_MPP23_GE1_TXD3,KIRKWOOD_MPP24_GE1_RXD0,KIRKWOOD_MPP25_GE1_RXD1,KIRKWOOD_MPP26_GE1_RXD2,KIRKWOOD_MPP27_GE1_RXD3,KIRKWOOD_MPP30_GE1_RXCTL,KIRKWOOD_MPP31_GE1_RXCLK,KIRKWOOD_MPP32_GE1_TCLKOUT,KIRKWOOD_MPP33_GE1_TXCTL,
      3 KIRKWOOD_MPP21_SATA0_ACTn,
      3 ORION5X_MPP14_SATA_LED,ORION5X_MPP15_SATA_LED,
      7 KIRKWOOD_MPP0_SPI_SCn,KIRKWOOD_MPP1_SPI_MOSI,KIRKWOOD_MPP2_SPI_SCK,KIRKWOOD_MPP3_SPI_MISO,
      7 KIRKWOOD_MPP6_SYSRST_OUTn,
      7 ORION5X_MPP12_SATA_LED,ORION5X_MPP13_SATA_LED,ORION5X_MPP14_SATA_LED,ORION5X_MPP15_SATA_LED,
      8 KIRKWOOD_MPP10_UART0_TXD,KIRKWOOD_MPP11_UART0_RXD,
      8 KIRKWOOD_MPP8_TW0_SDA,KIRKWOOD_MPP9_TW0_SCK

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-15 13:33       ` Andrew Lunn
@ 2011-05-15 17:50         ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-15 17:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: linux-kernel, Grant Likely, Martin Persson, Lee Jones, linux-arm-kernel

2011/5/15 Andrew Lunn <andrew@lunn.ch>:

> With a bit of scripting, came to the following results. Each line is
> on pinmux function description what would be needed in the driver. The
> number at the front says how many boards would use it. In total there
> are 33 descriptions needed. Of these, 18 are used only once. This to
> me suggests the board needs to be able to specify a pin description
> when there is no reuse with other boards.

As indicated elsewhere in this thread, a pinmux driver can pass in
platform/package/board/system data just like any other platform
driver in the kernel tree can.

You will have to specify it somehow in a custom header file, just
like with any other platform driver.

Example from:
arch/arm/mach-orion5x/ls-chl-setup.c:

static struct gpio_led lschl_led_pins[] = {
        {
                .name = "alarm:red",
                .gpio = LSCHL_GPIO_LED_ALARM,
                .active_low = 1,
        }, {
                .name = "info:amber",
                .gpio = LSCHL_GPIO_LED_INFO,
                .active_low = 1,
        }, {
                .name = "func:blue:top",
                .gpio = LSCHL_GPIO_LED_FUNC,
                .active_low = 1,
        }, {
                .name = "power:blue:bottom",
                .gpio = LSCHL_GPIO_LED_PWR,
        },
};

static struct gpio_led_platform_data lschl_led_data = {
        .leds = lschl_led_pins,
        .num_leds = ARRAY_SIZE(lschl_led_pins),
};

static struct platform_device lschl_leds = {
        .name = "leds-gpio",
        .id = -1,
        .dev = {
                .platform_data = &lschl_led_data,
        },
};

Still, the driver for these LEDs is in
drivers/leds/leds-gpio.c and the above structure is
described in include/linux/leds.h

There is no difference between this and letting
drivers/pinmux/pinmux-orion.c have a header file in
include/linux/pinmux/orion.h and pass in necessary board
data that way.

Are we now in agreement that this will work just fine?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-15 17:50         ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-15 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/15 Andrew Lunn <andrew@lunn.ch>:

> With a bit of scripting, came to the following results. Each line is
> on pinmux function description what would be needed in the driver. The
> number at the front says how many boards would use it. In total there
> are 33 descriptions needed. Of these, 18 are used only once. This to
> me suggests the board needs to be able to specify a pin description
> when there is no reuse with other boards.

As indicated elsewhere in this thread, a pinmux driver can pass in
platform/package/board/system data just like any other platform
driver in the kernel tree can.

You will have to specify it somehow in a custom header file, just
like with any other platform driver.

Example from:
arch/arm/mach-orion5x/ls-chl-setup.c:

static struct gpio_led lschl_led_pins[] = {
        {
                .name = "alarm:red",
                .gpio = LSCHL_GPIO_LED_ALARM,
                .active_low = 1,
        }, {
                .name = "info:amber",
                .gpio = LSCHL_GPIO_LED_INFO,
                .active_low = 1,
        }, {
                .name = "func:blue:top",
                .gpio = LSCHL_GPIO_LED_FUNC,
                .active_low = 1,
        }, {
                .name = "power:blue:bottom",
                .gpio = LSCHL_GPIO_LED_PWR,
        },
};

static struct gpio_led_platform_data lschl_led_data = {
        .leds = lschl_led_pins,
        .num_leds = ARRAY_SIZE(lschl_led_pins),
};

static struct platform_device lschl_leds = {
        .name = "leds-gpio",
        .id = -1,
        .dev = {
                .platform_data = &lschl_led_data,
        },
};

Still, the driver for these LEDs is in
drivers/leds/leds-gpio.c and the above structure is
described in include/linux/leds.h

There is no difference between this and letting
drivers/pinmux/pinmux-orion.c have a header file in
include/linux/pinmux/orion.h and pass in necessary board
data that way.

Are we now in agreement that this will work just fine?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-15 17:50         ` Linus Walleij
@ 2011-05-17  1:57           ` Kyungmin Park
  -1 siblings, 0 replies; 48+ messages in thread
From: Kyungmin Park @ 2011-05-17  1:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, linux-kernel, Grant Likely, Martin Persson,
	Lee Jones, linux-arm-kernel

Hi,

In real scenario, we can set the full pins at once, but sometimes it
needs to set the function only one,

e.g.,

static void __init goni_radio_init(void)
{
        int gpio;

        gpio = S5PV210_GPJ2(4);                 /* XMSMDATA_4 */
        gpio_request(gpio, "FM_INT");
        s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
        i2c1_devs[0].irq = gpio_to_irq(gpio);

        gpio = S5PV210_GPJ2(5);                 /* XMSMDATA_5 */
        gpio_request(gpio, "FM_RST");
        gpio_direction_output(gpio, 1);
}

In this case we only need to set the function at interrupt by like
s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
So pinmux function provides this feature also.

How do you think?

Thank you,
Kyungmin Park


On Mon, May 16, 2011 at 2:50 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2011/5/15 Andrew Lunn <andrew@lunn.ch>:
>
>> With a bit of scripting, came to the following results. Each line is
>> on pinmux function description what would be needed in the driver. The
>> number at the front says how many boards would use it. In total there
>> are 33 descriptions needed. Of these, 18 are used only once. This to
>> me suggests the board needs to be able to specify a pin description
>> when there is no reuse with other boards.
>
> As indicated elsewhere in this thread, a pinmux driver can pass in
> platform/package/board/system data just like any other platform
> driver in the kernel tree can.
>
> You will have to specify it somehow in a custom header file, just
> like with any other platform driver.
>
> Example from:
> arch/arm/mach-orion5x/ls-chl-setup.c:
>
> static struct gpio_led lschl_led_pins[] = {
>        {
>                .name = "alarm:red",
>                .gpio = LSCHL_GPIO_LED_ALARM,
>                .active_low = 1,
>        }, {
>                .name = "info:amber",
>                .gpio = LSCHL_GPIO_LED_INFO,
>                .active_low = 1,
>        }, {
>                .name = "func:blue:top",
>                .gpio = LSCHL_GPIO_LED_FUNC,
>                .active_low = 1,
>        }, {
>                .name = "power:blue:bottom",
>                .gpio = LSCHL_GPIO_LED_PWR,
>        },
> };
>
> static struct gpio_led_platform_data lschl_led_data = {
>        .leds = lschl_led_pins,
>        .num_leds = ARRAY_SIZE(lschl_led_pins),
> };
>
> static struct platform_device lschl_leds = {
>        .name = "leds-gpio",
>        .id = -1,
>        .dev = {
>                .platform_data = &lschl_led_data,
>        },
> };
>
> Still, the driver for these LEDs is in
> drivers/leds/leds-gpio.c and the above structure is
> described in include/linux/leds.h
>
> There is no difference between this and letting
> drivers/pinmux/pinmux-orion.c have a header file in
> include/linux/pinmux/orion.h and pass in necessary board
> data that way.
>
> Are we now in agreement that this will work just fine?
>
> Yours,
> Linus Walleij
> --
> 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/
>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-17  1:57           ` Kyungmin Park
  0 siblings, 0 replies; 48+ messages in thread
From: Kyungmin Park @ 2011-05-17  1:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

In real scenario, we can set the full pins at once, but sometimes it
needs to set the function only one,

e.g.,

static void __init goni_radio_init(void)
{
        int gpio;

        gpio = S5PV210_GPJ2(4);                 /* XMSMDATA_4 */
        gpio_request(gpio, "FM_INT");
        s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
        i2c1_devs[0].irq = gpio_to_irq(gpio);

        gpio = S5PV210_GPJ2(5);                 /* XMSMDATA_5 */
        gpio_request(gpio, "FM_RST");
        gpio_direction_output(gpio, 1);
}

In this case we only need to set the function at interrupt by like
s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
So pinmux function provides this feature also.

How do you think?

Thank you,
Kyungmin Park


On Mon, May 16, 2011 at 2:50 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2011/5/15 Andrew Lunn <andrew@lunn.ch>:
>
>> With a bit of scripting, came to the following results. Each line is
>> on pinmux function description what would be needed in the driver. The
>> number at the front says how many boards would use it. In total there
>> are 33 descriptions needed. Of these, 18 are used only once. This to
>> me suggests the board needs to be able to specify a pin description
>> when there is no reuse with other boards.
>
> As indicated elsewhere in this thread, a pinmux driver can pass in
> platform/package/board/system data just like any other platform
> driver in the kernel tree can.
>
> You will have to specify it somehow in a custom header file, just
> like with any other platform driver.
>
> Example from:
> arch/arm/mach-orion5x/ls-chl-setup.c:
>
> static struct gpio_led lschl_led_pins[] = {
> ? ? ? ?{
> ? ? ? ? ? ? ? ?.name = "alarm:red",
> ? ? ? ? ? ? ? ?.gpio = LSCHL_GPIO_LED_ALARM,
> ? ? ? ? ? ? ? ?.active_low = 1,
> ? ? ? ?}, {
> ? ? ? ? ? ? ? ?.name = "info:amber",
> ? ? ? ? ? ? ? ?.gpio = LSCHL_GPIO_LED_INFO,
> ? ? ? ? ? ? ? ?.active_low = 1,
> ? ? ? ?}, {
> ? ? ? ? ? ? ? ?.name = "func:blue:top",
> ? ? ? ? ? ? ? ?.gpio = LSCHL_GPIO_LED_FUNC,
> ? ? ? ? ? ? ? ?.active_low = 1,
> ? ? ? ?}, {
> ? ? ? ? ? ? ? ?.name = "power:blue:bottom",
> ? ? ? ? ? ? ? ?.gpio = LSCHL_GPIO_LED_PWR,
> ? ? ? ?},
> };
>
> static struct gpio_led_platform_data lschl_led_data = {
> ? ? ? ?.leds = lschl_led_pins,
> ? ? ? ?.num_leds = ARRAY_SIZE(lschl_led_pins),
> };
>
> static struct platform_device lschl_leds = {
> ? ? ? ?.name = "leds-gpio",
> ? ? ? ?.id = -1,
> ? ? ? ?.dev = {
> ? ? ? ? ? ? ? ?.platform_data = &lschl_led_data,
> ? ? ? ?},
> };
>
> Still, the driver for these LEDs is in
> drivers/leds/leds-gpio.c and the above structure is
> described in include/linux/leds.h
>
> There is no difference between this and letting
> drivers/pinmux/pinmux-orion.c have a header file in
> include/linux/pinmux/orion.h and pass in necessary board
> data that way.
>
> Are we now in agreement that this will work just fine?
>
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-17  1:57           ` Kyungmin Park
@ 2011-05-18 20:02             ` Linus Walleij
  -1 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-18 20:02 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Andrew Lunn, linux-kernel, Grant Likely, Martin Persson,
	Lee Jones, linux-arm-kernel

2011/5/17 Kyungmin Park <kmpark@infradead.org>:

> In this case we only need to set the function at interrupt by like
> s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
> So pinmux function provides this feature also.

This function:
s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));

Isn't part of the gpiolib I can see, I am discussing with Grant
how to handle custom GPIO configuration. Looking in
archa/arm/plat-samsung/include/plat/gpio-core.h
it looks like you have your own reimplementation of the
entire gpiolib in order to get these special configuration
functions... (So hopefully if I can add custom GPIO
configs to gpiolib, all of this can move to drivers/gpio/*)

So I don't know exactly what this means, if it means that you want to
mux pin 0xf to become GPIO, then yes, in the pinmux framework
you would:

pinmux_request_gpio(pinno, gpiono);

(As you see pins and GPIOs are now in different address spaces
and that is why two number have to be given, but if you manage
it the number space it can basically be the same number if
the platform so permits.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-18 20:02             ` Linus Walleij
  0 siblings, 0 replies; 48+ messages in thread
From: Linus Walleij @ 2011-05-18 20:02 UTC (permalink / raw)
  To: linux-arm-kernel

2011/5/17 Kyungmin Park <kmpark@infradead.org>:

> In this case we only need to set the function at interrupt by like
> s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
> So pinmux function provides this feature also.

This function:
s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));

Isn't part of the gpiolib I can see, I am discussing with Grant
how to handle custom GPIO configuration. Looking in
archa/arm/plat-samsung/include/plat/gpio-core.h
it looks like you have your own reimplementation of the
entire gpiolib in order to get these special configuration
functions... (So hopefully if I can add custom GPIO
configs to gpiolib, all of this can move to drivers/gpio/*)

So I don't know exactly what this means, if it means that you want to
mux pin 0xf to become GPIO, then yes, in the pinmux framework
you would:

pinmux_request_gpio(pinno, gpiono);

(As you see pins and GPIOs are now in different address spaces
and that is why two number have to be given, but if you manage
it the number space it can basically be the same number if
the platform so permits.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH 0/4] Pinmux subsystem
  2011-05-18 20:02             ` Linus Walleij
@ 2011-05-18 21:21               ` Mark Brown
  -1 siblings, 0 replies; 48+ messages in thread
From: Mark Brown @ 2011-05-18 21:21 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Kyungmin Park, Andrew Lunn, linux-kernel, Grant Likely,
	Martin Persson, Lee Jones, linux-arm-kernel

On Wed, May 18, 2011 at 10:02:47PM +0200, Linus Walleij wrote:
> 2011/5/17 Kyungmin Park <kmpark@infradead.org>:

> > In this case we only need to set the function at interrupt by like
> > s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
> > So pinmux function provides this feature also.

> This function:
> s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));

> Isn't part of the gpiolib I can see, I am discussing with Grant
> how to handle custom GPIO configuration. Looking in
> archa/arm/plat-samsung/include/plat/gpio-core.h

> it looks like you have your own reimplementation of the
> entire gpiolib in order to get these special configuration
> functions... (So hopefully if I can add custom GPIO
> configs to gpiolib, all of this can move to drivers/gpio/*)

This API predates the generic GPIO API - it's a reimplementation because
at that time the various platforms all had their own custom APIs.  The
Samsung specific API is pretty much only used for the pin mux and mode
configuration these days.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [PATCH 0/4] Pinmux subsystem
@ 2011-05-18 21:21               ` Mark Brown
  0 siblings, 0 replies; 48+ messages in thread
From: Mark Brown @ 2011-05-18 21:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 18, 2011 at 10:02:47PM +0200, Linus Walleij wrote:
> 2011/5/17 Kyungmin Park <kmpark@infradead.org>:

> > In this case we only need to set the function at interrupt by like
> > s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
> > So pinmux function provides this feature also.

> This function:
> s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));

> Isn't part of the gpiolib I can see, I am discussing with Grant
> how to handle custom GPIO configuration. Looking in
> archa/arm/plat-samsung/include/plat/gpio-core.h

> it looks like you have your own reimplementation of the
> entire gpiolib in order to get these special configuration
> functions... (So hopefully if I can add custom GPIO
> configs to gpiolib, all of this can move to drivers/gpio/*)

This API predates the generic GPIO API - it's a reimplementation because
at that time the various platforms all had their own custom APIs.  The
Samsung specific API is pretty much only used for the pin mux and mode
configuration these days.

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2011-05-18 21:21 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-02 19:16 [PATCH 0/4] Pinmux subsystem Linus Walleij
2011-05-02 19:16 ` Linus Walleij
2011-05-02 22:57 ` Russell King - ARM Linux
2011-05-02 22:57   ` Russell King - ARM Linux
2011-05-10 21:25   ` Linus Walleij
2011-05-10 21:25     ` Linus Walleij
2011-05-10 21:45     ` Russell King - ARM Linux
2011-05-10 21:45       ` Russell King - ARM Linux
2011-05-10 23:15       ` Linus Walleij
2011-05-10 23:15         ` Linus Walleij
2011-05-03 17:27 ` Andrew Lunn
2011-05-03 17:27   ` Andrew Lunn
2011-05-03 19:29   ` Valdis.Kletnieks
2011-05-03 19:29     ` Valdis.Kletnieks at vt.edu
2011-05-10 21:42   ` Linus Walleij
2011-05-10 21:42     ` Linus Walleij
2011-05-11  9:50     ` Andrew Lunn
2011-05-11  9:50       ` Andrew Lunn
2011-05-12  0:41       ` Linus Walleij
2011-05-12  0:41         ` Linus Walleij
2011-05-12  7:00         ` Andrew Lunn
2011-05-12  7:00           ` Andrew Lunn
2011-05-15 13:33     ` Andrew Lunn
2011-05-15 13:33       ` Andrew Lunn
2011-05-15 17:50       ` Linus Walleij
2011-05-15 17:50         ` Linus Walleij
2011-05-17  1:57         ` Kyungmin Park
2011-05-17  1:57           ` Kyungmin Park
2011-05-18 20:02           ` Linus Walleij
2011-05-18 20:02             ` Linus Walleij
2011-05-18 21:21             ` Mark Brown
2011-05-18 21:21               ` Mark Brown
2011-05-12  7:44 ` Sascha Hauer
2011-05-12  7:44   ` Sascha Hauer
2011-05-12  9:40   ` Tony Lindgren
2011-05-12  9:40     ` Tony Lindgren
2011-05-12 14:02   ` Linus Walleij
2011-05-12 14:02     ` Linus Walleij
2011-05-12 21:17     ` RE : " Matthieu Castet
2011-05-12 21:17       ` Matthieu Castet
2011-05-13  7:05       ` RE : " Linus Walleij
2011-05-13  7:05         ` Linus Walleij
2011-05-13 16:03         ` RE : " Matthieu CASTET
2011-05-13 16:03           ` Matthieu CASTET
2011-05-14  7:57           ` RE : " Linus Walleij
2011-05-14  7:57             ` Linus Walleij
2011-05-13  9:59     ` Sascha Hauer
2011-05-13  9:59       ` Sascha Hauer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.