| Message ID | 1351724661-29050-6-git-send-email-haojian.zhuang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/31/2012 05:04 PM, Haojian Zhuang wrote: > Add comments with pinconf & gpio range in the document of > pinctrl-single. I'd tend to suggest separating the series to add GPIO range support and pinconf support, especially since didn't Tony suggest a separate driver for pinconf? Perhaps that was just for non-generic properties. > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt > +- pinctrl-single,gpio-ranges : gpio range list of phandles. > + Must be present if gpio range phandle is specified. > + This property should be existing in .dtsi files for those silicons. > + > +- pinctrl-single,gpio : array with gpio range start, size & register > + offset. Must be present if gpio range phandle is specified. > + This property should be existing in .dts files for those boards. > + > +- pinctrl-single,gpio-func : gpio function value in the pinmux register. > + Must be present if gpio range phandle is specified. > + This property should be existing in .dts files for those boards. I don't see any reason why pinctrl-single,gpio or pinctrl-single,gpio-func are board-specific rather than SoC-specific. Surely it's the Soc HW construction that defines how the pinctrl and GPIO modules relate to each-other? I would simply remove the last sentence from each of the three descriptions above. Also, isn't this list a list of properties for the main pinctrl-single node itself? I think you should split the list up as follows: 1) A description of the main node 2) A list of required and optional properties for the main node 3) A description of what a GPIO range node is 4) A list of required and optional properties for a GPIO range node. I think that would explain the whole structure a lot more clearly. Oh, and why not just get rid of the pinctrl-single,gpio-ranges property entirely, and simply make the GPIO range definitions be child nodes of the pinctrl-single node? That would require them to either be named according to some scheme or have some compatible property to differentiate them from any other nodes, but I think that's workable. > @@ -42,6 +77,15 @@ 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. > > +In case pinctrl device supports gpio function, it needs to define gpio range. > +All the phandles of gpio range list should be set in below: > + > + pinctrl-single,gpio-ranges = <[phandle of gpio range]>; > + > + [phandle of gpio range]: { That's a label, not a phandle. The reference to the label gets compiled to a phandle in the DTB. The node name is missing. you should probably just use an example label and re-write the last 3 lines as: pinctrl-single,gpio-ranges = <&range0>; range0: range0 {
* Stephen Warren <swarren@wwwdotorg.org> [121101 11:06]: > On 10/31/2012 05:04 PM, Haojian Zhuang wrote: > > Add comments with pinconf & gpio range in the document of > > pinctrl-single. > > I'd tend to suggest separating the series to add GPIO range support and > pinconf support, especially since didn't Tony suggest a separate driver > for pinconf? Perhaps that was just for non-generic properties. Well I was just thinking setting them up as separate driver instances for the register ranges supporting pinconf and not supporting pinconf. This combined with some pinconf property parsed during pinctrl-single.c probe time allows saving lots of unnecessary parsing of properties for register ranges that don't support pinconf. At least for omaps we have few hundred registers that don't support pinconf, and then some separate random register that do. Regards, TOny
On Fri, Nov 2, 2012 at 2:04 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 10/31/2012 05:04 PM, Haojian Zhuang wrote: >> Add comments with pinconf & gpio range in the document of >> pinctrl-single. > > I'd tend to suggest separating the series to add GPIO range support and > pinconf support, especially since didn't Tony suggest a separate driver > for pinconf? Perhaps that was just for non-generic properties. > >> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt > >> +- pinctrl-single,gpio-ranges : gpio range list of phandles. >> + Must be present if gpio range phandle is specified. >> + This property should be existing in .dtsi files for those silicons. >> + >> +- pinctrl-single,gpio : array with gpio range start, size & register >> + offset. Must be present if gpio range phandle is specified. >> + This property should be existing in .dts files for those boards. >> + >> +- pinctrl-single,gpio-func : gpio function value in the pinmux register. >> + Must be present if gpio range phandle is specified. >> + This property should be existing in .dts files for those boards. > > I don't see any reason why pinctrl-single,gpio or > pinctrl-single,gpio-func are board-specific rather than SoC-specific. Oh, I wrote it with error. It should be SoC-specific. > Surely it's the Soc HW construction that defines how the pinctrl and > GPIO modules relate to each-other? I would simply remove the last > sentence from each of the three descriptions above. > > Also, isn't this list a list of properties for the main pinctrl-single > node itself? I think you should split the list up as follows: > > 1) A description of the main node > 2) A list of required and optional properties for the main node > 3) A description of what a GPIO range node is > 4) A list of required and optional properties for a GPIO range node. > > I think that would explain the whole structure a lot more clearly. > > Oh, and why not just get rid of the pinctrl-single,gpio-ranges property > entirely, and simply make the GPIO range definitions be child nodes of > the pinctrl-single node? That would require them to either be named > according to some scheme or have some compatible property to > differentiate them from any other nodes, but I think that's workable. > Em. Removing pinctrl-single,gpio-ranges should be better. Now I define range as sub node. There're two properties in this sub-node: reg & pinctrl-single,gpio. >> @@ -42,6 +77,15 @@ 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. >> >> +In case pinctrl device supports gpio function, it needs to define gpio range. >> +All the phandles of gpio range list should be set in below: >> + >> + pinctrl-single,gpio-ranges = <[phandle of gpio range]>; >> + >> + [phandle of gpio range]: { > > That's a label, not a phandle. The reference to the label gets compiled > to a phandle in the DTB. The node name is missing. you should probably > just use an example label and re-write the last 3 lines as: > > pinctrl-single,gpio-ranges = <&range0>; > > range0: range0 { > Updated.
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt index 2c81e45..15f4dae 100644 --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt @@ -17,6 +17,41 @@ Optional properties: - pinctrl-single,bit-per-mux : boolean to indicate that one register controls more than one pin +- pinctrl-single,gpio-ranges : gpio range list of phandles. + Must be present if gpio range phandle is specified. + This property should be existing in .dtsi files for those silicons. + +- pinctrl-single,gpio : array with gpio range start, size & register + offset. Must be present if gpio range phandle is specified. + This property should be existing in .dts files for those boards. + +- pinctrl-single,gpio-func : gpio function value in the pinmux register. + Must be present if gpio range phandle is specified. + This property should be existing in .dts files for those boards. + +- pinctrl-single,power-source-mask : mask of setting power source in + the pinmux register + +- pinctrl-single,power-source : value of setting power source field + in the pinmux register + +- pinctrl-single,bias-mask : mask of setting bias value in the pinmux + register + +- pinctrl-single,bias-disable : value of disabling bias in the pinmux + register + +- pinctrl-single,bias-pull-down : value of setting bias pull down in + the pinmux register + +- pinctrl-single,bias-pull-up : value of setting bias pull up in the + pinmux register + +- pinctrl-single,bias : value of setting bias in the pinmux register + +- pinctrl-single,input-schmitt-mask : mask of setting input schmitt + in the pinmux register + 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 +77,15 @@ 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. +In case pinctrl device supports gpio function, it needs to define gpio range. +All the phandles of gpio range list should be set in below: + + pinctrl-single,gpio-ranges = <[phandle of gpio range]>; + + [phandle of gpio range]: { + pinctrl-single,gpio = <0 55 0x0dc>; + pinctrl-single,gpio-func = <0>; + }; Example: /* SoC common file */ @@ -76,6 +120,28 @@ control_devconf0: pinmux@48002274 { pinctrl-single,function-mask = <0x5F>; }; +/* third controller instance for pins in gpio domain */ +pmx_gpio: pinmux@d401e000 { + compatible = "pinctrl-single"; + reg = <0xd401e000 0x0330>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <7>; + pinctrl-single,gpio-ranges = <&gpiorange0 &gpiorange1>; +}; + +gpiorange0: gpiorange@d401e0dc { + pinctrl-single,gpio = <0 55 0x0dc>; + pinctrl-single,gpio-func = <0>; +}; + +gpiorange1: gpiorange@d401e2f0 { + pinctrl-single,gpio = <55 5 0x2f0>; + pinctrl-single,gpio-func = <1>; +}; + + /* board specific .dts file */ &pmx_core {
Add comments with pinconf & gpio range in the document of pinctrl-single. Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com> --- .../devicetree/bindings/pinctrl/pinctrl-single.txt | 66 ++++++++++++++++++++ 1 file changed, 66 insertions(+)