On Wed, Aug 24, 2011 at 8:29 PM, Stephen Warren swarren@nvidia.com wrote:
Linus Walleij wrote at Friday, August 19, 2011 3:54 AM:
This creates a subsystem for handling of pin control devices. These are devices that control different aspects of package pins.
Sorry to keep harping on about this, but I'm still not convinced the data model is correct.
Don't feel sorry! I'm getting very much done and much important research and thinking is happening thanks to you valuable feedback. Keep doing this!
What I notice is that the review comments have changed focus from the earlier contention point about having multiple functions per map entry to something else, which to me seems like it comes from device tree work, and is about describing how each and every pin is wired to its function. Does this correspond to your recent pinmux experience?
Here are the two possible models:
Model 1 (as implemented in the pin control subsystem):
- Each pinmux chip defines a number of functions.
- Each function describes what functionality is mux'd onto the pins.
- Each function describes which pins are affected by the function.
This is correct.
Result:
If there are a couple of controllers that can each be mux'd out two places, you get four functions:
Function i2c0-0: Controller i2c0 is mux'd onto pins 0 and 1 Function i2c0-1: Controller i2c0 is mux'd onto pins 2 and 3 Function spi0-0: Controller spi0 is mux'd onto pins 0 and 1 Function spi0-1: Controller spi0 is mux'd onto pins 4 and 5
Yes, if and only if the pinmux driver find it useful to expose these two ports on the two sets of pins each.
For example: there really *is* a bus soldered to pin {0,1} and another bus soldered to pin {2,3}, and each has devices on it. But I have only one single i2c controller i2c0.
If pins {2,3} were not soldered or used for something else it doesn't make sense for the pin controller to expose two functions like this.
So this is dependent on platform/board data. We need to know how the chip has been deployed in this very machine to know what functions to expose.
Model 2 (what I'd like to see)
- Each pinmux chip defines a number of functions.
- Each function describes the functionality that can be mux'd out.
- Each pinmux chip defines a number of pins.
- Each pinmux chip defines which functions are legal on which pins.
Result:
With the same controllers and pins above, you get:
Function i2c0 Function spi0 Pins 0, 1, 2, 3, 4, 5 Pins 0, 1 accept functions i2c0, spi0 Pins 2, 3 accept functions i2c0 Pins 4, 5 accept functions spi0
We'd still have a single controller-wide namespace for functions, it's just that functions are something you mux onto pins, not the combination of mux'd functionality and the pins it's been mux'd onto.
I think I understand this. Maybe.
I read it like this: Legend: 1..* = one to many 1..1 = one to one
- Pins has a 1..* relation to functions - Functions in general have a 1..* relation to pins, - Device drivers in general have a 1..* relation to functions - Functions with 1..1 relation to pins and device drives with a 1..1 relation to functions is not the norm
So this is radically different in that it requires the pin control system to assume basically that any one pin can be used for any one function.
I refer to this as a phone-exchange model, like any one pin can map to any one function, like any phone can call any other phone.
The current model is not based on that assumption at all. The current model is derived from some available pinmux controllers and described below.
- I believe this model is much more directly maps to the way most HW
works; there's a register for each pin where you write a value to select which functionality to mux onto that pin. Assuming that's true, using this data model might lead to simpler pinmux chip implementations; they'd require fewer mapping tables.
I understand this statement as you assume al pinmuxes are phone-exachange-type, where any pin can be tied to any function at runtime. So for example the CLK pin of spi0 can be muxed onto any pin basically, not a predetermined set of pins.
I think that data model does not take into account the fact that all or most pinmuxes are inherently restricted and not at all that open-ended. That model would impose a phone-exchange type of data model on hardware that is much more limited.
So the data model I'm assuming is:
- Pins has a 1..* relation to functions - Functions in general have a 1..1 relation to pins, - Device drivers in general have a 1..1 relation to functions - Functions with 1..* relation to pins is uncommon as is 1..* realtions between device drivers and functions.
The latter is the crucial point where I think we have different assumptions.
If it holds, it leads to the fact that a pinmux driver will present a few functions for say i2s0 usually only one, maybe two, three, never hundreds.
Survey of some systems
So this is where we have to determine what is really the way such hardware at large works. I have done some research in data sheets and code:
What I have found is that the typical layout is that:
- Each pin has a number of mux control bits in some register, often 2 or 3 bits.
- These bits select a mux setting out of 2 to 8 possible settings.
- It is very uncommon to be able to map the same function to some other pin - so often say UART0 CTS can only come out on one pin, not five different pins, let alone any pin
- The need to present a lot of different combinations of one function on different pins to the subsystem through the selectors is very limited.
mach-u300: ----------------
Configurability: each function can be mapped to one pin (pad) only - for example SPI is found on pads { 420 .. 425 } it can only be enabled on these pins. It's not possible to move the SPI to say pins { 100 ... 105 }, what you can do is turn the pins into GPIO:s instead. However you cannot map any arbitrary GPIO line to each one of them, but a *certain* GPIO line. So this muxing is pretty static.
Further the functions are selected groupwise, so you select all 6 pins to be used for GPIO or something else, you cannot select things on each pin individually at all. For example pins {166 ... 171, 176, 177} can be used for MMC *or* memory stick, *or* GPIOs. You cannot request to have your MMC on some other pins and you absolutely can never use MMC and memory stick at the same time, because they can only use these pins.
So there is a mux, and it is not like a phone exchange that can route anything anywhere, it's a few functions per pin.
mach-ux500: -----------------
Each muxable pin has a default function, which is GPIO, then it also can provide one of three "alternate functions" called A, B, C. Our map looks like this for pin 0:
#define GPIO0_GPIO PIN_CFG(0, GPIO) #define GPIO0_U0_CTSn PIN_CFG(0, ALT_A) #define GPIO0_TRIG_OUT PIN_CFG(0, ALT_B) #define GPIO0_IP_TDO PIN_CFG(0, ALT_C)
So pin 0 can be used for GPIO, the UART0 CTS signal, trigger output or TDO, whatever that is.
You cannot move the UART0 CTS singnal to some other pin, this is fixed muxing per pin, just like in the U300. If you want to use UART0 you *have* to use pin 0 for it's CTS signal, nothing else is possible.
If you want to use the other functions, you loose UART0.
However the Ux500 is not muxed in groups, so each pins function can be selected individually. Which makes it possible to configure a few useless combinations, like vital pins replaced by GPIO and whatever.
plat-omap: --------------
I have checked the TRMs for OMAP 3 & 4 but the pinmux stuff seems to sit in the system controller (at OMAP*_CTRL_BASE) which in turn does not appear to be documented in the public data sheet. (Help me if you have a reference...)
Looking at say arch/arm/mach-omap2/mux2420.c you see entries like this:
_OMAP2420_MUXENTRY(CAM_D0, 54, "cam_d0", "hw_dbg2", "sti_dout", "gpio_54", NULL, NULL, "etk_d2", NULL),
As you can see pin 54 (which does not seem to mean anything else than that this is the pin that can be used for GPIO channel 54) can theoretically be used for up to 8 different functions, in this case only 5 of them are actually valid. The selected function is apparently controlled by 3 bits per pin somewhere.
It's not like you can move "sti_dout" to some other pin than 54. This is a restriction of the electronics.
So OMAPs mux impose identical restrictions on pins as the ux500 mux - a pin can not hold *any* function, just a few select functions. It is not a phone-exchange type.
mach-msm: ----------------
Hard to tell how this works and what's available, support seems to be incomplete. Currently it seems to be wired to do either a dedicated function (like some UART pin) or GPIO, like each pin can be used for two specific things, and not phone-exchange type.
plat-mxc/mach-mx5: ----------------------------
Quite hard o make out, but checking the mux maps in arch/arm/plat-mxc/include/mach/iomux-*.h I get the impression that also these muxes are of the same type as the above for example:
/* PAD MUX ALT INPSE PATH PADCTRL */ #define _MX51_PAD_EIM_D16__AUD4_RXFS IOMUX_PAD(0x3f0, 0x5c, 5, 0x0000, 0, 0) #define _MX51_PAD_EIM_D16__AUD5_TXD IOMUX_PAD(0x3f0, 0x5c, 7, 0x08d8, 0, 0) #define _MX51_PAD_EIM_D16__EIM_D16 IOMUX_PAD(0x3f0, 0x5c, 0, 0x0000, 0, 0) #define _MX51_PAD_EIM_D16__GPIO2_0 IOMUX_PAD(0x3f0, 0x5c, 1, 0x0000, 0, 0) #define _MX51_PAD_EIM_D16__I2C1_SDA IOMUX_PAD(0x3f0, 0x5c, 0x14, 0x09b4, 0, 0) #define _MX51_PAD_EIM_D16__UART2_CTS IOMUX_PAD(0x3f0, 0x5c, 3, 0x0000, 0, 0) #define _MX51_PAD_EIM_D16__USBH2_DATA0 IOMUX_PAD(0x3f0, 0x5c, 2, 0x0000, 0, 0)
Pad 16 seems to be able to mux in AUD4 RXFS, AUD5 TXD, EIM, GPIO2 controller line 0, I2C1_SDA, UART2 CTS and USBH2 DATA0.
Thus these functions are restricted to this one pin. It's not like I could all of a sudden have GPIO2 line 0 on pad 45 instead. Or UART2 CTS on pad 96. Or similar. If I want to use UART2, it's CTS signal must go out on pad 16.
- Given that's the way Tegra HW works at least, the patches I recently
posted to initialize the Tegra pinmux from device-tree define bindings that use this model. I think it's important that such bindings use the same data model as the pinmux chip data model. This keeps consistency between boot-time initialization and the pinmux chip portion of any run-time pinmux modifications.
Isn't it better if that data model is kept as a Tegra thing as I guess it is right now?
Atleast until we find if this model really suits other machines...
My concern is that it may force all pin control drivers to conform to something pretty complex that is only useful for Tegra. And in that case that part of it (the 1..* relation between functions and pins) should reside in the tegra pinmux driver, not across the entire subsystem.
The intention of the pinmux part of the pin control subsystem is to model the subset of what is common functionality across all pin controllers, not the superset of everything that is possible to do with every pin controller, atleast not if it makes the API hard to use for others. So let's figure out if this is the case.
So that way in summary:
- Pin has a 1..* relation to functions - ALL SYSTEMS - Functions in general have a 1..1 relation to pins, the function can only come out on one pin - Functions with 1..* relation to pins is uncommon, Tegra is the only chip that has this. - Device drivers in general have a 1..1 relation to functions, 1..* is uncommon
More than anything else I think that it's up to the pin control/mux driver to present the useful functions for a system, what would change my mind is if one system would present - in practice, on a real board - a voluminous amount of functions for the same IP block say.
So the subsystem enumerates the pins, tie them to functions, and map functions to device drivers. Infering which functions should be available on a certain board is up to the device driver, say function "uart0" may be {0..3}, {100..103} or {265..268} on three different boards, but at runtime the device driver asks for the function "uart0" and don't really care much about what pins it gets, as long as the device driver does its work.
As an aside, I see your patches use the pinmux API at driver probe time to set up the pinmux, whereas my init-pinmux-driver-from-dt patches do a pinmux-controller-wide initialization at probe time. There was some discussion in response to earlier patches re: which way was better, and I got the impression that the pinmux API was mainly for runtime changes, rather than boot-time initial configuration. If that's not the case, then I guess we should drop my proposed init-pinmux-driver-from-dt patches and put all that code into your pinmux subsystem...
I think the individual pinmux driver should deal with initializing the system, by using its supplied platform data or device tree, then it can present the relevant available functions on that specific machine to the pinmux subsystem.
I think the structure if the supplied platform data for Tegra may be quite different from the data supplied by the pin controllers mentioned above, if it is as open-ended as it is described.
So the driver handling this should be in drivers/pinctrl but the subsystem should not (IMO) deal with setting up the registers and infering which mux functions are available and on which pins, that knowledge should be intrinsic to the specific driver.
And I assume the set of functions eventually presented to the subsystem will generally be of 1..1 type where one function maps to one device driver, not 1..*.
I don't know if this makes anything clearer, I feel I'm writing too much text :-/
Thanks, Linus Walleij