Messages in this thread | | | Date | Tue, 01 Oct 2013 14:30:26 -0600 | From | Stephen Warren <> | Subject | Re: [PATCH V4 2/3] pincntrl: add support for AMS AS3722 pin control driver |
| |
On 09/24/2013 11:47 AM, Laxman Dewangan wrote: > The AS3722 is a compact system PMU suitable for mobile phones, tablets etc. > > Add a driver to support accessing the GPIO, pinmux and pin configuration > of 8 GPIO pins found on the AMS AS3722 through pin control driver and > gpiolib. > > The driver will register itself as the pincontrol driver and gpio driver.
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-as3722.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-as3722.txt
> +AMS AS3722 Pincontrol
This binding document doesn't appear to define any compatible value. I think there should be one binding document per compatible value, rather than splitting up the documentation of a particular binding into lots of different files. As such, shouldn't this documentation all be part of Documentation/devicetree/bindings/regulator/as3722-regulator.txt?
> +DT node contains the different subnode for pin control to represent
Which "DT node"? This paragraph doesn't make much sense...
> +different states. Each of these subnodes represents some desired > +configuration for a list of pins. This configuration can include the > +mux function to select on those pin(s), and various pin configuration > +parameters, such as pull-up, open drain.
I think that paragraph is meant to say the following, as part of as3722-regulator.txt:
========== Optional sub-nodes:
- pinmux: Represents the pinmux configuration of the AS3722 GPIO pins. This node contains pin configuration nodes, as defined by pinctrl-bindings.txt ==========
(and then go on to describe the required/optional properties within the pin configuration nodes)
> +Valid values for pin names are:
I would say "pin property" rather than "pin names" here, to make it obvious where these values are used.
> + gpio0, gpio1, gpio2, gpio3, gpio4, gpio5, gpio6, gpio7 > + > +Valid value of function names are:
Similarly, "function property" here. (and values for not value of).
Other than that, this binding looks reasonable.
| |