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

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

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-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html