On 08/05/2013 01:33 AM, Laxman Dewangan wrote: > TI Palmas series Power Management IC have multiple pins which can be > configured for different functionality. This pins can be configured > for different function. Also their properties like pull up/down, > open drain enable/disable are configurable. > > Add support for pincontrol driver Palmas series device like TPS65913, > TPS80036. The driver supports to be register from DT only. > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt > +Required subnode-properties: > +- pinctrl-pins: An array of strings. Each string contains the name of a pin. > + Valid values for these names are listed below. s/pinctrl-pins/pins/ I think. > +Optional subnode-properties: > +- pinctrl-function: string containing the name of the function to mux to the > + pin. Valid values for function names are listed below. Please refer the > + datasheet of the device to determine which are valid for each pin. s/pinctrl-function/function/ I think. > +The properties "pinctrl-pins" and "pinctrl-function" ara generic pincontrol > +properties and described in pinctrl-bindings.txt. > + > +Following are other valid propeties for Palams pinctrol dt node which are > +described in pinctrl-bindings.txt. > +bias-disable, bias-pull-up, bias-pull-down, bias-pull-pin-default > +drive-open-drain: Value will represent the open drain to be enable/disable > + of pins. > + 0: Disable, 1: Enable. I'd be tempted to replace everything I quoted above with something a little simpler to avoid any redundancy with pinctrl-bindings.txt: ========== This binding uses the following generic properties as defined in pinctrl-bindings.txt: Required: pins Options: function, bias-disable, bias-pull-up, bias-pull-down, bias-pin-default, drive-open-drain. ========== > +Note that many of these properties are only valid for certain specific pins. > +See the Palmas device datasheet for complete details regarding which pins > +support which functionality. > + > +Valid values for pin names are: > +pins: gpio0, gpio1, gpio2, gpio3, gpio4, gpio5, gpio6, gpio7, gpio8, gpio9, "pins:" is redundant here. > + gpio10, gpio11, gpio12, gpio13, gpio14, gpio15, vac, powergood, > + nreswarm, pwrdown, gpadc_start, reset_in, nsleep, enable1, enable2, > + int. > + > +function: gpio, led, pwm, regen, sysen, clk32kgaudio, id, vbus_det, chrg_det, I think you would want to structure this list the same as the pins list, so prefix it with "Valid values for function names are:", and delete "function:" at the start of the list. > +There is 4 special function: opt0, opt1, opt2 and opt3. If this functions are s/this/any of these/ > +selected then directly pins register will be written with 0, 1, 2 and 3 s/and/or/ > +repectively if it is valid for that pins. s/repectively/respectively/ Hmm. I'm not sure I like mixing those functions with the more "semantic" function names above. I'd prefer the driver support either the semantic names or the raw names, but not both. It's not really a big deal though I suppose. I didn't review the code, just the binding. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html