[lkml]   [2012]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] pinctrl: Add one-register-per-pin type device tree based pinctrl driver
    On 06/19/2012 07:56 AM, Tony Lindgren wrote:
    > Hi,
    > Below is the pinctrl-single patch updated with hopefully all the Stephen's
    > comments addressed. The binding still needs to be looked at, see relevant
    > parts of the discussion below.
    > From: Tony Lindgren <>
    > Date: Wed, 6 Jun 2012 04:18:18 -0700
    > Subject: [PATCH] pinctrl: Add one-register-per-pin type device tree based pinctrl driver
    > Add one-register-per-pin type device tree based pinctrl driver.
    > Currently this driver only works on omap2+ series of processors,
    > where there is either an 8 or 16-bit padconf register for each pin.
    > Support for other similar pinmux controllers can be added.
    > Signed-off-by: Tony Lindgren <>
    > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
    > new file mode 100644
    > index 0000000..929254c
    > --- /dev/null
    > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
    > @@ -0,0 +1,106 @@
    > +One-register-per-pin type device tree based pinctrl driver
    > +
    > +Required properties:
    > +- compatible : "pinctrl-single"
    > +
    > +- reg : offset and length of the register set for the mux registers
    > +
    > +- pinctrl-single,register-width : pinmux register access width in bits
    > +
    > +- pinctrl-single,function-mask : mask of allowed pinmux function bits
    > + in the pinmux register
    > +
    > +- pinctrl-single,pinconf-mask : mask of allowed pinconf bits in the
    > + pinmux register; this gets combined with pinconf mask but is a separate
    > + mask to allow the option of setting pinconf separatately from the
    > + function

    Given that this binding doesn't allow describing pin configuration at
    present, I would simply remove all mention of that property in the
    binding documentation. It can be added back if/when that feature is
    added. Any future driver using this binding can refuse to allow pin
    configuration if that property is missing.

    > +- pinctrl-single,function-off : function off mode for disabled state if
    > + available and same for all registers; if not, use a value larger than
    > + function-mask to ignore disabling of registers

    Rather than requiring an invalid value in this property, shouldn't the
    lack of a valid function-off value be represented by the property not
    being present in the DT?

    > +This driver assumes that there is only one register for each pin,
    > +and uses the common pinctrl bindings as specified in the pinctrl-bindings.txt
    > +document in this directory.

    At this point in the file, I think you need to mention that you're
    switching from describing the top-level device node to describing pin
    configuration nodes.

    > +The pinctrl register offsets and default values are specified as pairs

    I thought we were going to remove "default" here?

    > +using pinctrl-single,pins. For example, setting a pin for a device
    > +could be done with:
    > +
    > + pinctrl-single,pins = <0xdc 0x118>;
    > +
    > +Where 0xdc is the offset from the pinctrl register base address for the
    > +device pinctrl register, and 0x118 contains the desired value of the
    > +pinctrl register. See the device example and static board pins example
    > +below for more information.

    There should be some explanation only the portion of this value covered
    by the pinctrl-single,function-mask value is updated in the register.

    > +This driver tries to avoid understanding pin and function names because of
    > +the extra bloat they would cause especially in the case of a large number
    > +of pins. This driver just sets what is specified for the board in the .dts file.
    > +Further user space debugging tools can be developed to decipher the pin and
    > +function names using debugfs.

    There shouldn't be any discussion of a driver here; the binding is a HW

    > +Example:

    I only reviewed the binding document, not the code.

     \ /
      Last update: 2012-06-22 00:41    [W:0.057 / U:40.980 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site