lkml.org 
[lkml]   [2013]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] pinctrl: Add documentation for pinctrl-msm8x74
On 11/23/2013 04:38 PM, Bjorn Andersson wrote:
> This adds initial documentation for the pinctrl-msm8x74 driver.

> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,msm8x74-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,msm8x74-pinctrl.txt

> +Qualcomm MSM8x74 TLMM block
> +
> +Required properties:
> +- compatible: "qcom,msm8x74-pinctrl"
> +- reg: Should be the base address of the TLMM block.

base address *and length*?

> +- interrupts: Should be the irq of the TLMM summary interrupt.

s/irq/IRQ/

> +- interrupt-controller: Marks the device node as an interrupt controller.
> +- #interrupt-cells: Should be two.
> +- gpio-controller: Marks the device node as a GPIO controller.
> +- #gpio-cells : Should be two.
> + The first cell is the gpio pin number and the
> + second cell is used for optional parameters.
> +
> +Please refer to ../gpio/gpio.txt for a general description of GPIO bindings.

It's probably worth linking to ../interrupt-controller/interrupts.txt too.

> +Please refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning of the
> +phrase "pin configuration node".
> +
> +Each subnode describes properties for the given pins.

Which subnodes? It's probably worth describing what subnodes can exist.
Perhaps try cribbing e.g. the 3 paragraphs in the Tegra binding that
appear right after the "Please refer to pinctrl-bindings.txt..." that's
there.

> +Required subnode-properties:
> +- qcom,pins: An array of strings, each matching a pin or group to be
> + configured. Possible values are listed below.

It isn't clear from this that these properties exist in a sub-sub-node,
not just a sub-node, of the main pinctrl node.

> +Optional subnode-properties:
> +- qcom,function: A name of the function to be muxed to the specified
> + pins. Possible values are listed below.

Are "pins" and "functions" generic enough now that they don't need a
vendor prefix? Both those property names are certainly mentioned in
pinctrl-bindings.txt.

> +- drive-strength: Configure the drive strength of the specified pins.

What units? I assume the definition matches that in
pinctrl-bindings.txt? Perhaps it'd be simpler to say "The following
generic properties as defined in pinctrl-bindings.txt: pins, function,
bias-disable, ...".

> +Valid values for qcom,function are:
> + blsp_i2c2, blsp_i2c6, blsp_i2c11, blsp_spi1, blsp_uart2, blsp_uart8, slimbus
> +
> + (Note that this is not yet the complete list of functions)

It'd be best if the complete list were defined from the start. That
said, I suppose adding new values into the list would be
backwards-compatible (if not forwards-compatible i.e. new DTs usable
with old kernels).

> +Example:
> +
> + msmgpio: pinctrl@fd510000 {
...
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_default>;
> +
> + uart2 {

What does that node represent? Other pinctrl bindings only have 1 or 2
levels of subnodes, not 3.

> + uart2_default: uart2_default {
> + mux {
> + qcom,pins = "gpio4", "gpio5";
> + qcom,function = "blsp_uart2";
> + };




\
 
 \ /
  Last update: 2013-11-26 01:21    [W:0.099 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site