diff mbox

[v2,01/14] Documentation: dt/bindings: Document pinctrl-ingenic

Message ID 20170122144947.16158-2-paul@crapouillou.net (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Cercueil Jan. 22, 2017, 2:49 p.m. UTC
This commit adds documentation for the devicetree bidings of the
pinctrl-ingenic driver, which handles pin configuration and pin
muxing of the Ingenic SoCs currently supported by the Linux kernel.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 .../bindings/pinctrl/ingenic,pinctrl.txt           | 77 ++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt

v2: Rewrote the documentation for the new pinctrl-ingenic driver

Comments

Linus Walleij Jan. 27, 2017, 11:18 a.m. UTC | #1
On Sun, Jan 22, 2017 at 3:49 PM, Paul Cercueil <paul@crapouillou.net> wrote:


> +Required properties:
> +- compatible: One of:
> +  - "ingenic,jz4740-pinctrl"
> +  - "ingenic,jz4780-pinctrl"
> +
> +Optional properties:
> +- ingenic,pull-ups: A list of 32-bit bit fields, where each bit set tells the
> +  driver that a pull-up resistor is available for this pin.
> +  By default, the driver considers that all pins feature a pull-up resistor.
> +- ingenic,pull-downs: A list of 32-bit bit fields, where each bit set tells
> +  the driver that a pull-down resistor is available for this pin.
> +  By default, the driver considers that all pins feature a pull-down
> +  resistor.

So I still don't understand these properties.

Does this mean that there is a pull-up *inside* the SoC or *outside*
on the board where it is mounted?

If it is *outside* the SoC, on the board, the pulls should be set on
the consumers, not on the controller.

In the former case, if this pertains to the *inside* of the SoC:
is it just different between jz4740 and jz4780?
In that case, just code this into the driver as .data to the .compatible
in the DT match. No special DT properties needed.

If it is different between e.g. different versions of jz4740, why not
make different compatible-properties? If there are different versions
of the circuit, with different hard-wired hardware configuration, they
are different devices and should have different compatible strings.

> +                               /* CLK, CMD, D0 */
> +                               ingenic,pins = <0x69 0 0x68 0 0x6a 0>;

Standard bindings use just "pins". Why the custom ingenic,
prefix?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Cercueil Jan. 27, 2017, 3:27 p.m. UTC | #2
Hi,

> So I still don't understand these properties.
> 
> Does this mean that there is a pull-up *inside* the SoC or *outside*
> on the board where it is mounted?

The pull-up resistors are inside the SoCs.

> In the former case, if this pertains to the *inside* of the SoC:
> is it just different between jz4740 and jz4780?
> In that case, just code this into the driver as .data to the 
> .compatible
> in the DT match. No special DT properties needed.

Well, I've been taught that devicetree is for describing the hardware, 
and
the driver code is for functionality. So that's why I put it in 
devicetree.

That's also the reason why I put the list of functions and groups in
devicetree and not in the driver code.

> Standard bindings use just "pins". Why the custom ingenic,
> prefix?

I can change that.

Regards,
-Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Jan. 31, 2017, 12:59 p.m. UTC | #3
On Fri, Jan 27, 2017 at 4:27 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> [Me]:
>> In the former case, if this pertains to the *inside* of the SoC:
>> is it just different between jz4740 and jz4780?
>> In that case, just code this into the driver as .data to the .compatible
>> in the DT match. No special DT properties needed.
>
> Well, I've been taught that devicetree is for describing the hardware, and
> the driver code is for functionality. So that's why I put it in devicetree.

This is a gray area.

But as GPIO maintainer I'm not happy about encoding information
about the hardware, that can be deducted from the compatible-string,
into the devicetree.

I prefer to encode per-compatible hardware information tables into
the driver, as structs, and pick the right table based on the compatible
string as .data in the of match entry.

It's simple to retrieve into the driver using of_device_get_match_data()
these days.

> That's also the reason why I put the list of functions and groups in
> devicetree and not in the driver code.

I'm not super-happy about that either, and it's not the way I would
have done it but the argument has been made
that it is a lot of opaque data and people prefer to maintain it
in the device tree.

I accept it for functions and groups, but I don't like it.

I will not fold to any consistency arguments of the type "now
you allowed this one thing, so you must allow this other thing
so you are consistent", just no. I didn't like it the first time, so I'm
not liking it anymore the second time.

I guess if the DT people tell me it has to be done this way I would
accept it. After a lot of discussion. But noone has.

Please make it a table and put it in the driver instead.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
new file mode 100644
index 000000000000..ead5b01ad471
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
@@ -0,0 +1,77 @@ 
+Ingenic jz47xx pin controller
+
+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".
+
+For the jz47xx SoCs, pin control is tightly bound with GPIO ports. All pins may
+be used as GPIOs, multiplexed device functions are configured within the
+GPIO port configuration registers and it is typical to refer to pins using the
+naming scheme "PxN" where x is a character identifying the GPIO port with
+which the pin is associated and N is an integer from 0 to 31 identifying the
+pin within that GPIO port. For example PA0 is the first pin in GPIO port A, and
+PB31 is the last pin in GPIO port B. The jz4740 contains 4 GPIO ports, PA to
+PD, for a total of 128 pins. The jz4780 contains 6 GPIO ports, PA to PF, for a
+total of 192 pins.
+
+
+Pin controller node
+===================
+
+Required properties:
+- compatible: One of:
+  - "ingenic,jz4740-pinctrl"
+  - "ingenic,jz4780-pinctrl"
+
+Optional properties:
+- ingenic,pull-ups: A list of 32-bit bit fields, where each bit set tells the
+  driver that a pull-up resistor is available for this pin.
+  By default, the driver considers that all pins feature a pull-up resistor.
+- ingenic,pull-downs: A list of 32-bit bit fields, where each bit set tells
+  the driver that a pull-down resistor is available for this pin.
+  By default, the driver considers that all pins feature a pull-down
+  resistor.
+
+
+'functions' sub-node
+====================
+
+The 'functions' node will contain sub-nodes that correspond to pin function
+nodes, and no properties. Pin function nodes will contain sub-nodes that
+correspond to pin groups, and no properties.
+
+The names of the pin function nodes will end up being the available functions
+provided by the pinctrl driver.
+The names of the pin group nodes will end up being the available groups
+provided by the pinctrl driver.
+
+Required properties for pin groups:
+- ingenic,pins: <pin mode [pin mode ...]>;
+  where 'pin' is the number of the pin, and 'mode' is the function mode of the
+  pin that should be enabled for this group.
+
+
+Example:
+=======
+
+pinctrl: ingenic-pinctrl@10010000 {
+	compatible = "ingenic,jz4740-pinctrl";
+	reg = <0x10010000 0x400>;
+
+	ingenic,pull-ups   = <0xffffffff 0xffffffff 0xffffffff 0xdfffffff>;
+	ingenic,pull-downs = <0x00000000 0x00000000 0x00000000 0x00000000>;
+
+	functions {
+		mmc {
+			mmc-1bit {
+				/* CLK, CMD, D0 */
+				ingenic,pins = <0x69 0 0x68 0 0x6a 0>;
+			};
+
+			mmc-4bit {
+				/* D1, D2, D3 */
+				ingenic,pins = <0x6b 0 0x6c 0 0x6d 0>;
+			};
+		};
+	};
+};