On 11/11/2013 11:01 AM, Mark Rutland wrote: > On Mon, Nov 11, 2013 at 05:41:58PM +0000, Ashwini Ghuge wrote: >> The driver uses the common Tegra pinctrl driver utility functions to >> implement the majority of the driver. It is based on the similar Tegra114 >> pinctrl >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-pinctrl.txt >> @@ -0,0 +1,120 @@ >> +NVIDIA Tegra124 pinmux controller >> + >> +The Tegra124 pinctrl binding is very similar to the Tegra114 >> +pinctrl binding, as described in nvidia,tegra114-pinmux.txt. In fact, this >> +document assumes that binding as a baseline, and only documents the >> +differences between the two bindings. >> + >> +Required properties: >> +- compatible: "nvidia,tegra124-pinmux" >> +- reg: Should contain the register physical address and length for each of >> + the pad control and mux registers. The first bank of address must be the >> + driver strength pad control register address and second bank address must >> + be pinmux register address. > > It might be worth using reg-names here, in case a future hardware > variant builds atop of this. This binding is essentially identical to the existing Tegra20/30/114 bindings, it'd just that the HW has different sets of valid values for pin/group/function names. I'd rather keep it consistent with the other bindings. It's quite unlikely some future binding will build on top of this; if the HW changes at all, we'll almost certainly have a completely new binding definition to cater for the differing pin/group/function names. >> +Tegra124 has the following optional properties for pin configuration subnodes: >> +- nvidia,enable-input: Integer. Enable the pin's input path. 0: no, 1: yes. > > Why are the properties representing boolean values not empty / boolean > properties (as can be handled in the kernel with of_property_read_bool)? They aren't Boolean, they're tri-state: Set off, set on, don't change the setting. The "pinctrl state" nodes each list a set of changes to make to the HW to enter/set-up that state, and those sets of changes may not need to alter all values, hence it's important to be able to differentiate between not changing a value, and the value to change it to. >> +- nvidia,open-drain: Integer. Enable open drain mode. 0: no, 1: yes. >> +- nvidia,lock: Integer. Lock the pin configuration against further changes >> + until reset. 0: no, 1: yes. >> +- nvidia,io-reset: Integer. Reset the IO path. 0: no, 1: yes. >> +- nvidia,rcv-sel: Integer. Select VIL/VIH receivers. 0: normal, 1: high. > > Why not just have nvidia,rcv-high as a boolean property, and default to > normal? Same argument. >> +- nvidia,drive-type: Integer. Valid range 0...3. > > What do these values mean? They're the raw HW values. We probably should add something like, "See the TRM's definition of field XXX for further details". >> +As with Tegra114, see the Tegra TRM for complete details regarding which >> +groups support which functionality. > > Where can I find this TRM? The TRM for earlier Tegras is available at e.g.: https://developer.nvidia.com/tegra-3-technical-reference-manual We probably haven't published the Tegra124 TRM yet:-( However, the overall structure of the Tegra124 pinmux is the same as for the Tegra30 pinmux documented at that link. The difference is the set of pins/groups/functions. > [...] > >> +Example: >> + >> + pinmux: pinmux { >> + compatible = "nvidia,tegra124-pinmux"; >> + reg = <0x70000868 0x148 /* Pad control registers */ >> + 0x70003000 0x40c>; /* PinMux registers */ > > For consistency, please bracket entries in lists individually: > > reg = <0x70000868 0x148>, /* Pad control registers */ > <0x70003000 0x40c>; /* PinMux registers */ I had pointed this out to Ashwini in internal review, along with a few other minor formatting/wording issues elsewhere. Hopefully that'll all be rolled into v2... -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html