| Message ID | 1356083118-18857-6-git-send-email-haojian.zhuang@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, * Haojian Zhuang <haojian.zhuang@linaro.org> [121221 01:48]: > > +- pinctrl-single,power-source : array of value that are used to configure > + power source in the pinmux register. They're value of power source field > + and power source mask. > + > + /* power source, mask */ > + pinctrl-single,power-source = <0x1000 0x1800>; > + > +- pinctrl-single,bias : array of value that are used to configure the input > + bias in the pinmux register. They're value of bias field, bias mask, > + bias disable value, bias pull down value & bias pull up value. > + > + /* bias, mask, disable, pull down, pull up */ > + pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>; > + > +- pinctrl-single,input-schmitt : array of value that are used to configure > + input schmitt in the pinmux register. They're value of input schmitt field, > + mask, & disable value. > + > + /* input schmitt value, mask, disable */ > + pinctrl-single,input-schmitt = <0x40 0x70 0x40>; > + Hmm we might be able to standardize on just few bindings if we break the bias into enable, pullup and pulldown. Then we should have the defval, enable and disable for each of them to allow setting the board specific config, and to enable and disable things using the generic pinconf api. So how about something like this: pinctrl-single,power-source = <defval regmask enableval disableval>; pinctrl-single,bias-enable = <defval regmask enableval disableval>; pinctrl-single,bias-pullup = <defval regmask enableval disableval>; pinctrl-single,bias-pulldown = <defval regmask enableval disableval>; pinctrl-single,input-schmitt = <defval regmask enableval disableval>; And then we can add support for other things like comparators too: pinctrl-single,comparator-enable = <defval regmask enableval disableval>; pinctrl-single,comparator-status = <regmask>; /* read only status bits */ I'll do some experiments on this with my bit-per-mux testcase, but please let me know if you see any issues using the format above meanwhile. Regards, Tony
On 22 December 2012 09:22, Tony Lindgren <tony@atomide.com> wrote: > Hi, > > * Haojian Zhuang <haojian.zhuang@linaro.org> [121221 01:48]: >> >> +- pinctrl-single,power-source : array of value that are used to configure >> + power source in the pinmux register. They're value of power source field >> + and power source mask. >> + >> + /* power source, mask */ >> + pinctrl-single,power-source = <0x1000 0x1800>; >> + >> +- pinctrl-single,bias : array of value that are used to configure the input >> + bias in the pinmux register. They're value of bias field, bias mask, >> + bias disable value, bias pull down value & bias pull up value. >> + >> + /* bias, mask, disable, pull down, pull up */ >> + pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>; >> + >> +- pinctrl-single,input-schmitt : array of value that are used to configure >> + input schmitt in the pinmux register. They're value of input schmitt field, >> + mask, & disable value. >> + >> + /* input schmitt value, mask, disable */ >> + pinctrl-single,input-schmitt = <0x40 0x70 0x40>; >> + > > Hmm we might be able to standardize on just few bindings if we > break the bias into enable, pullup and pulldown. Then we should > have the defval, enable and disable for each of them to allow > setting the board specific config, and to enable and disable > things using the generic pinconf api. > > So how about something like this: > > pinctrl-single,power-source = <defval regmask enableval disableval>; It seems that there's no requirement on disable power source. Is there any silicon that need to disable power source? I think that power source means drive strength at here. > pinctrl-single,bias-enable = <defval regmask enableval disableval>; > pinctrl-single,bias-pullup = <defval regmask enableval disableval>; > pinctrl-single,bias-pulldown = <defval regmask enableval disableval>; If bias-pullup or bias-pulldown, it means that bias-enable also. As my understanding, pin could be in any state of bias-disable, bias-pullup, bias-pulldown. In my v5 patches, pin couldn't switch state among these states. Now it's fixed in v6 patches. > pinctrl-single,input-schmitt = <defval regmask enableval disableval>; In Marvell silicons, input-schmitt trigger could be configured as high-edge, low-edge or both detect. So enableval can't cover this usage. > > And then we can add support for other things like comparators too: > > pinctrl-single,comparator-enable = <defval regmask enableval disableval>; > pinctrl-single,comparator-status = <regmask>; /* read only status bits */ > I'm OK on appending these properties. I would also add slew rate property later. Regards Haojian
* Haojian Zhuang <haojian.zhuang@linaro.org> [121221 22:35]: > On 22 December 2012 09:22, Tony Lindgren <tony@atomide.com> wrote: > > Hi, > > > > * Haojian Zhuang <haojian.zhuang@linaro.org> [121221 01:48]: > >> > >> +- pinctrl-single,power-source : array of value that are used to configure > >> + power source in the pinmux register. They're value of power source field > >> + and power source mask. > >> + > >> + /* power source, mask */ > >> + pinctrl-single,power-source = <0x1000 0x1800>; > >> + > >> +- pinctrl-single,bias : array of value that are used to configure the input > >> + bias in the pinmux register. They're value of bias field, bias mask, > >> + bias disable value, bias pull down value & bias pull up value. > >> + > >> + /* bias, mask, disable, pull down, pull up */ > >> + pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>; > >> + > >> +- pinctrl-single,input-schmitt : array of value that are used to configure > >> + input schmitt in the pinmux register. They're value of input schmitt field, > >> + mask, & disable value. > >> + > >> + /* input schmitt value, mask, disable */ > >> + pinctrl-single,input-schmitt = <0x40 0x70 0x40>; > >> + > > > > Hmm we might be able to standardize on just few bindings if we > > break the bias into enable, pullup and pulldown. Then we should > > have the defval, enable and disable for each of them to allow > > setting the board specific config, and to enable and disable > > things using the generic pinconf api. > > > > So how about something like this: > > > > pinctrl-single,power-source = <defval regmask enableval disableval>; > > It seems that there's no requirement on disable power source. Is there > any silicon that need > to disable power source? I think that power source means drive strength at here. Yes at least I have cases where the bias disable needs to be set when changing bias voltage between 1.8V and 3V. > > pinctrl-single,bias-enable = <defval regmask enableval disableval>; > > pinctrl-single,bias-pullup = <defval regmask enableval disableval>; > > pinctrl-single,bias-pulldown = <defval regmask enableval disableval>; > > If bias-pullup or bias-pulldown, it means that bias-enable also. > As my understanding, pin could be in any state of bias-disable, > bias-pullup, bias-pulldown. > In my v5 patches, pin couldn't switch state among these states. Now > it's fixed in v6 patches. Yes that's cool. I'm just worried that if we try to stuff all the bias settings into one array, it won't work for additional bias states. Looks like we have at least bias disable, bias voltage and bias high impedance mode in addition to what you're describing. > > pinctrl-single,input-schmitt = <defval regmask enableval disableval>; > In Marvell silicons, input-schmitt trigger could be configured as > high-edge, low-edge or both > detect. So enableval can't cover this usage. Hmm there too trying to stuff them into one array may not be flexible enough to cover all the cases. So that's why I'm suggesting we describe them separately. > > And then we can add support for other things like comparators too: > > > > pinctrl-single,comparator-enable = <defval regmask enableval disableval>; > > pinctrl-single,comparator-status = <regmask>; /* read only status bits */ > > > I'm OK on appending these properties. I would also add slew rate property later. OK thanks. BTW, we may not need the board configured "defval" in the array examples I posted above, we may have it already in the pinmux binding. Regards, Tony
* Tony Lindgren <tony@atomide.com> [121222 09:14]: > * Haojian Zhuang <haojian.zhuang@linaro.org> [121221 22:35]: > > > pinctrl-single,input-schmitt = <defval regmask enableval disableval>; > > In Marvell silicons, input-schmitt trigger could be configured as > > high-edge, low-edge or both > > detect. So enableval can't cover this usage. Hmm assuming that's two bits for configuring it, can you make both detect same as: pinctrl-single,input-schmitt-high-edge = <enableval disableval regmask>; pinctrl-single,input-schmitt-low-edge = <enableval disableval regmask>; Or does that not work for you? To me it looks like we should just standarize on the following: pinctrl-single,bias-enable = <enableval disableval regmask>; pinctrl-single,bias-pullup = <enableval disableval regmask>; pinctrl-single,bias-pulldown = <enableval disableval regmask>; pinctrl-single,input-schmitt = <enableval disableval regmask>; pinctrl-single,input-schmitt-high-edge = <enableval disableval regmask>; pinctrl-single,input-schmitt-low-edge = <enableval disableval regmask>; ... pinctrl-single,comparator-enable = <enableval disableval regmask>; pinctrl-single,comparator-status = <regmask>; /* read only status bits */ So no need to stuff the defval there AFAIK. Regards, Tony
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt index 2c81e45..7d1d4b2 100644 --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt @@ -1,7 +1,9 @@ One-register-per-pin type device tree based pinctrl driver Required properties: -- compatible : "pinctrl-single" +- compatible : "pinctrl-single" or "pinconf-single". + "pinctrl-single" means that pinconf isn't supported. + "pinconf-single" means that generic pinconf is supported. - reg : offset and length of the register set for the mux registers @@ -14,9 +16,31 @@ Optional properties: - pinctrl-single,function-off : function off mode for disabled state if available and same for all registers; if not specified, disabling of pin functions is ignored + - pinctrl-single,bit-per-mux : boolean to indicate that one register controls more than one pin +- pinctrl-single,power-source : array of value that are used to configure + power source in the pinmux register. They're value of power source field + and power source mask. + + /* power source, mask */ + pinctrl-single,power-source = <0x1000 0x1800>; + +- pinctrl-single,bias : array of value that are used to configure the input + bias in the pinmux register. They're value of bias field, bias mask, + bias disable value, bias pull down value & bias pull up value. + + /* bias, mask, disable, pull down, pull up */ + pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>; + +- pinctrl-single,input-schmitt : array of value that are used to configure + input schmitt in the pinmux register. They're value of input schmitt field, + mask, & disable value. + + /* input schmitt value, mask, disable */ + pinctrl-single,input-schmitt = <0x40 0x70 0x40>; + This driver assumes that there is only one register for each pin (unless the pinctrl-single,bit-per-mux is set), and uses the common pinctrl bindings as specified in the pinctrl-bindings.txt document in this directory. @@ -42,6 +66,24 @@ Where 0xdc is the offset from the pinctrl register base address for the device pinctrl register, 0x18 is the desired value, and 0xff is the sub mask to be used when applying this change to the register. + +Optional sub-node: In case some pins could be configured as GPIO in the pinmux +register, those pins could be defined as a GPIO range. The sub-node should +be defined in .dtsi files of those silicons. + +Required properties in sub-node: +- reg : offset and length of the GPIO range sub-node. + +- pinctrl-single,gpio : array of GPIO base number in the range and the GPIO + function in the pinmux register. + + range0: { + /* GPIO0 ~ GPIO54 */ + reg = <0xd401e0dc 55>; + pinctrl-single,gpio = <0 0>; + }; + + Example: /* SoC common file */ @@ -76,6 +118,26 @@ control_devconf0: pinmux@48002274 { pinctrl-single,function-mask = <0x5F>; }; +/* third controller instance for pins in gpio domain */ +pmx_gpio: pinmux@d401e000 { + compatible = "pinconf-single"; + reg = <0xd401e000 0x0330>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <7>; + + range0: range@d401e0dc { + /* GPIO0 ~ GPIO54 */ + reg = <0xd401e0dc 0xdc>; + /* gpio base & gpio func */ + pinctrl-single,gpio = <0 0>; + }; +}; + + /* board specific .dts file */ &pmx_core { @@ -96,6 +158,19 @@ control_devconf0: pinmux@48002274 { >; }; + uart1_pins: pinmux_uart1_pins { + pinctrl-single,pins = < + 0x198 0x6 /* GPIO47_UART1_RXD */ + 0x19c 0x6 /* GPIO48_UART1_TXD */ + >; + /* power source, mask */ + pinctrl-single,power-source = <0x1000 0x1800>; + /* bias, mask, disable, pull down, pull up */ + pinctrl-single,bias = <0xc000 0xe000 0 0xa000 0xc000>; + /* input schmitt, mask, disable */ + pinctrl-single,input-schmitt = <0x40 0x70 0x40>; + }; + /* map uart2 pins */ uart2_pins: pinmux_uart2_pins { pinctrl-single,pins = < @@ -122,6 +197,11 @@ control_devconf0: pinmux@48002274 { }; +&uart1 { + pinctrl-names = "default"; + pinctrl-0 = <&uart1_pins>; +}; + &uart2 { pinctrl-names = "default"; pinctrl-0 = <&uart2_pins>;