pinctrl: mvebu: reset pins to an UNKNOWN state on startup

Note: this patch is a *RFC*, it is not intended for merging, only to get a discussion started. The code is horrible, makes terrible assumptions and so on.

On many platforms, most of the pinmux initialization is done in the bootloader, and therefore persists when we boot the Linux kernel. This prevents us from making sure that the pinmux configuration in the board device trees is correct.

One idea to make sure our device trees are correct in terms of pinmuxing is to set the state of each pin to an unavailable function during the initialization of the pinctrl driver. This way, only pins that are explicitly configured through proper device tree attributes will actually be functional.

Remaining questions to solve:

* Is this useful?

* How to figure out what function number is a good function number to set all pins to at startup? It could be passed by the SoC-specific pinctrl drivers.

* Maybe some pins should be excluded for this, especially if the console serial port pins are muxed. On Armada XP, it's not the
case: UART0 RX/UART0 TX pins are not part of MPPs, so we can clear all pins. But on other mvebu platforms, it may be the case.

* If this sounds like an interesting thing, should we keep it only at the mvebu driver level, or make it something more generically available in the pinctrl subsystem?

---
 drivers/pinctrl/pinctrl-mvebu.c |    3 +++
 1 file changed, 3 insertions(+)

index 8e6266c..32a9cbe 100644
@@ -739,6 +739,9 @@ int __devinit mvebu_pinctrl_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "registered pinctrl driver\n");
 
+	for (n = 0; n < pctl->num_groups; n++)
+		mvebu_pinconf_group_set(pctl->pctldev, n, 7);
+
 	/* register gpio ranges */
 	for (n = 0; n < soc->ngpioranges; n++)
 		pinctrl_add_gpio_range(pctl->pctldev, &soc->gpioranges[n]);

1.7.9.5

Thomas,

how you make sure something you don't know about? The bootloader sets these pinmux settings for a reason and if the DT doesn't tell the kernel it should make no assumptions at all.

IMHO it isn't - but maybe I am missing the point here. What is it that you don't like in the bootloaders choice of configuring pinmux?

That's why you don't touch pins that you don't know about. The mvebu pinctrl is written to overwrite pinctrl settings only if someone requests a special function. Usually this is done by a developer that knows about the board.

Sebastian

Sebastian,

Apparently, the general feeling of the kernel people is that the kernel should reduce as much as possible its dependency on the hardware configuration set up by the bootloader. Therefore, even though your bootloader does a certain pinmux configuration, your kernel should make its own pinmux configuration and be fully independent from the one done by the bootloader. If we want to achieve this, then it seemed like a good idea to put all pins to an unknown state at boot time.

Just because the kernel should not depend on hardware configuration done by the bootloader, except for very core things like DRAM timings and al. At least, that's my understanding of where the ARM kernel people are trying to go. But I might have misunderstood, in which case of course, the entire purpose of this patch disappears.

Best regards,

Thomas

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

You can get a lot of information from /debug. eg,

# cat /debug/pinctrl/f1010000.pinctrl/pinconf-groups 
Pin config settings per pin group
Format: group (name): configs
0 (mpp0):current: spi(cs), available = [ gpio(io) nand(io2) ]
1 (mpp1):current: spi(mosi), available = [ gpo(o) nand(io3) ]
2 (mpp2):current: spi(sck), available = [ gpo(o) nand(io4) ]
3 (mpp3):current: spi(miso), available = [ gpo(o) nand(io5) ]
4 (mpp4):current: sata1(act), available = [ gpio(io) nand(io6) uart0(rxd) lcd(hsync) ]
5 (mpp5):current: sata0(act), available = [ gpo(o) nand(io7) uart0(txd) lcd(vsync) ]
6 (mpp6):current: spi(mosi), available = [ sysrst(out) ]
7 (mpp7):current: gpo(o), available = [ spi(cs) lcd(pwm) ]
8 (mpp8):current: twsi0(sda), available = [ gpio(io) uart0(rts) uart1(rts) mii-1(rxerr) sata1(prsnt) mii(col) ]
9 (mpp9):current: twsi0(sck), available = [ gpio(io) uart0(cts) uart1(cts) sata0(prsnt) mii(crs) ]
10 (mpp10):current: uart0(txd), available = [ gpo(o) spi(sck) sata1(act) ]
11 (mpp11):current: uart0(rxd), available = [ gpio(io) spi(miso) sata0(act) ]
12 (mpp12):current: gpo(o), available = [ sdio(clk) audio(spdifo) spi(mosi) twsi1(sda) ]
13 (mpp13):current: uart1(txd), available = [ gpio(io) sdio(cmd) audio(rmclk) lcd(pwm) ]
14 (mpp14):current: uart1(rxd), available = [ gpio(io) sdio(d0) sata1(prsnt) audio(spdifi) audio-1(sdi) mii(col) ]
15 (mpp15):current: sata0(act), available = [ gpio(io) sdio(d1) uart0(rts) uart1(txd) spi(cs) ]
16 (mpp16):current: gpio(io), available = [ sdio(d2) uart0(cts) uart1(rxd) sata1(act) lcd(extclk) mii(crs) ]
17 (mpp17):current: gpio(io), available = [ sdio(d3) sata0(prsnt) sata1(act) twsi1(sck) ]
18 (mpp18):current: gpo(o), available = [ nand(io0) pex(clkreq) ]
19 (mpp19):current: gpo(o), available = [ nand(io1) ]
20 (mpp20):current: sata1(act), available = [ gpio(io) ts(mp0) tdm(tx0ql) ge1(txd0) audio(spdifi) lcd(d0) ]
21 (mpp21):current: sata0(act), available = [ gpio(io) ts(mp1) tdm(rx0ql) ge1(txd1) audio(spdifo) lcd(d1) ]
22 (mpp22):current: sata1(prsnt), available = [ gpio(io) ts(mp2) tdm(tx2ql) ge1(txd2) audio(rmclk) lcd(d2) ]
23 (mpp23):current: sata0(prsnt), available = [ gpio(io) ts(mp3) tdm(rx2ql) ge1(txd3) audio(bclk) lcd(d3) ]
24 (mpp24):current: gpio(io), available = [ ts(mp4) tdm(spi-cs0) ge1(rxd0) audio(sdo) lcd(d4) ]
25 (mpp25):current: gpio(io), available = [ ts(mp5) tdm(spi-sck) ge1(rxd1) audio(lrclk) lcd(d5) ]
26 (mpp26):current: gpio(io), available = [ ts(mp6) tdm(spi-miso) ge1(rxd2) audio(mclk) lcd(d6) ]
27 (mpp27):current: gpio(io), available = [ ts(mp7) tdm(spi-mosi) ge1(rxd3) audio(sdi) lcd(d7) ]
28 (mpp28):current: gpio(io), available = [ ts(mp8) tdm(int) ge1(col) audio(extclk) lcd(d8) ]
29 (mpp29):current: gpio(io), available = [ ts(mp9) tdm(rst) ge1(txclk) lcd(d9) ]
30 (mpp30):current: gpio(io), available = [ ts(mp10) tdm(pclk) ge1(rxctl) lcd(d10) ]
31 (mpp31):current: gpio(io), available = [ ts(mp11) tdm(fs) ge1(rxclk) lcd(d11) ]
32 (mpp32):current: gpio(io), available = [ ts(mp12) tdm(drx) ge1(txclko) lcd(d12) ]
33 (mpp33):current: gpo(o), available = [ tdm(dtx) ge1(txctl) lcd(d13) ]
34 (mpp34):current: gpio(io), available = [ tdm(spi-cs1) ge1(txen) sata1(act) lcd(d14) ]
35 (mpp35):current: gpio(io), available = [ tdm(tx0ql) ge1(rxerr) sata0(act) lcd(d15) mii(rxerr) ]
36 (mpp36):current: gpio(io), available = [ ts(mp0) tdm(spi-cs1) audio(spdifi) twsi1(sda) ]
37 (mpp37):current: gpio(io), available = [ ts(mp1) tdm(tx2ql) audio(spdifo) twsi1(sck) ]
38 (mpp38):current: gpio(io), available = [ ts(mp2) tdm(rx2ql) audio(rmclk) lcd(d18) ]
39 (mpp39):current: gpio(io), available = [ ts(mp3) tdm(spi-cs0) audio(bclk) lcd(d19) ]
40 (mpp40):current: gpio(io), available = [ ts(mp4) tdm(spi-sck) audio(sdo) lcd(d20) ]
41 (mpp41):current: gpio(io), available = [ ts(mp5) tdm(spi-miso) audio(lrclk) lcd(d21) ]
42 (mpp42):current: gpio(io), available = [ ts(mp6) tdm(spi-mosi) audio(mclk) lcd(d22) ]
43 (mpp43):current: gpio(io), available = [ ts(mp7) tdm(int) audio(sdi) lcd(d23) ]
44 (mpp44):current: gpio(io), available = [ ts(mp8) tdm(rst) audio(extclk) lcd(clk) ]
45 (mpp45):current: gpio(io), available = [ ts(mp9) tdm(pclk) lcd(e) ]
46 (mpp46):current: gpio(io), available = [ ts(mp10) tdm(fs) lcd(hsync) ]
47 (mpp47):current: gpio(io), available = [ ts(mp11) tdm(drx) lcd(vsync) ]
48 (mpp48):current: gpio(io), available = [ ts(mp12) tdm(dtx) lcd(d16) ]
49 (mpp49):current: gpo(o), available = [ tdm(rx0ql) pex(clkreq) lcd(d17) ]

shows the current configuration of each pin.

# cat /debug/pinctrl/f1010000.pinctrl/pinmux-pins 
Pinmux settings per pin
Format: pin (name): mux_owner gpio_owner hog?
pin 0 (PIN0): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function spi group mpp0
pin 1 (PIN1): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function spi group mpp1
pin 2 (PIN2): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function spi group mpp2
pin 3 (PIN3): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function spi group mpp3
pin 4 (PIN4): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function sata1 group mpp4
pin 5 (PIN5): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function sata0 group mpp5
pin 6 (PIN6): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 7 (PIN7): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 8 (PIN8): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function twsi0 group mpp8
pin 9 (PIN9): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function twsi0 group mpp9
pin 10 (PIN10): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function uart0 group mpp10
pin 11 (PIN11): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function uart0 group mpp11
pin 12 (PIN12): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 13 (PIN13): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function uart1 group mpp13
pin 14 (PIN14): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function uart1 group mpp14
pin 15 (PIN15): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 16 (PIN16): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 17 (PIN17): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 18 (PIN18): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 19 (PIN19): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 20 (PIN20): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function sata1 group mpp20
pin 21 (PIN21): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function sata0 group mpp21
pin 22 (PIN22): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function sata1 group mpp22
pin 23 (PIN23): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function sata0 group mpp23
pin 24 (PIN24): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 25 (PIN25): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 26 (PIN26): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 27 (PIN27): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 28 (PIN28): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 29 (PIN29): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 30 (PIN30): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 31 (PIN31): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 32 (PIN32): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 33 (PIN33): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 34 (PIN34): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 35 (PIN35): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 36 (PIN36): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function gpio group mpp36
pin 37 (PIN37): f1010000.pinctrl mvebu-gpio:37 (HOG) function gpio group mpp37
pin 38 (PIN38): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 39 (PIN39): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 40 (PIN40): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 41 (PIN41): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 42 (PIN42): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 43 (PIN43): f1010000.pinctrl mvebu-gpio:43 (HOG) function gpio group mpp43
pin 44 (PIN44): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function gpio group mpp44
pin 45 (PIN45): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 46 (PIN46): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 47 (PIN47): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 48 (PIN48): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 49 (PIN49): (MUX UNCLAIMED) (GPIO UNCLAIMED)

This shows how pins have been configured via DT.

If you compare the two, you can see that pin 6 has probably been set by uboot, but not by DT.

Andrew

Dear Andrew Lunn,

Yes, I was using that one...

... but not that one.

Indeed, by correlating the two files, you can get a good view of which pins are configured even though no driver has claimed them. I don't think it's as clear as having a non-functional device due to the pin not being muxed at all, but if it is thought as being sufficient, then fair enough.

Best regards,

Thomas

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

If it is useful, please consider introducing new debugfs files, that thing is supposed to be helpful.

For example: if a <debugfs>/pinctrl/pinctrl-foo/pins-probe-state caching and providing the power-on-state of all pins is useful, then just add that mechanism to the pinctrl core I'd say.

Yours,
Linus Walleij

Now I don't know which kernel senior being it was that told me never to screw around with the defaults from the boot loader if not really needed. It is better if the driver reads the hardware to figure out what state it's in and move on from there.

There may be cases where it's still needed, such as to save power, if pins (in this case) are set up such that they waste power unless the default setting is overridden.

So I think it'd be nice if the actual reasons for doing this movement to a known state were known? Else the design pattern should be to discover the current state from the actual hardware registers.

Depending on complexity the above may be a bit utopic...

Yours,
Linus Walleij

This depends on the platform quite a bit - there's two schools of thought on what bootloaders should do, and typically platforms decide on a per platform basis which they'll use:

1. The bootloader does enough to load the kernel and nothing more, the kernel should ignore anything the bootloader did. Some people prefer this as it places minimal reliance on the bootloader which may be of uncertain quality and minimises the need to upgrade the bootloader which is often highly risky.

2. The bootloader does all the pin setup and so on. This provides a place for board specific stuff and avoids the kernel having to know about lots of device variants.

For example AIUI OMAP tends towards the second view (partly due to
number of device variants) but for example Samsung platforms typically
use the first method (their pinmux is pretty simple).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121025/f30f4e79/attachment-0001.sig>

Well, it's easy enough to determine that the kernel's configuration is correct as far as it goes, since it's applied, and if everything still works, the pinmux configuration is presumably correct. The issue is more that the kernel's configuration may be missing some entries, and hence relying on the bootloader having already set up some pins/groups.

On Tegra at least, and I imagine many SoCs, there is not an "unavailable" function for each pin/group. Hence, this would be impossible to implement. For pins/groups where there is a mux function that is reserved or actually does do nothing, the value of that mux function is potentially different per pin/group. At least I suppose we could force tristate on for all pingroups, so no output signals would work.

I can certainly see the use of this, as an explicitly enabled option used for testing at least. On Tegra, we've certainly had issues where the kernel accidentally relies on the bootloader having set things up, so when switching to a different lighter-weight bootloader, things stop working. That was more w.r.t. clocks, but it could easily happen for pinmux too.

Yes, this would have to be either implemented by individual drivers, or use data passed in by the individual drivers.

I don't think excluding the serial port pins would be useful. Presumably, if you enable this option, you're doing so explicitly to test the pinmux setup after you've tested that the kernel otherwise works correctly. As such, if the serial port stops working when you flip on this option, it's a pretty good clue that the serial port pinmux setup isn't correct. Presumably, you might also enable earlyprintk over the UART when debugging with this new option.

Making this generic is probably complex for reasons I outlined above. I'd tend towards a common kernel Kconfig or cmdline option to trigger this, but have each driver use that option as it sees fit in its own custom code. I imagine that will reduce the overall amount of code it takes to implement this. If using a cmdline option, the pinmux core should parse it, and set up some variable or function for the drivers to query.