Re: [RFC 2/2] staging: jnx-connector: add device tree binding

From: Rob Herring
Date: Sat Oct 08 2016 - 12:06:13 EST


On Fri, Oct 7, 2016 at 10:16 AM, Pantelis Antoniou
<pantelis.antoniou@xxxxxxxxxxxx> wrote:
> From: Guenter Roeck <groeck@xxxxxxxxxxx>
>
> Add documentation for the Juniper connector driver.
>
> Signed-off-by: Alon Ronen <aronen@xxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <groeck@xxxxxxxxxxx>
> [Ported from Juniper kernel]
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
> ---
> .../devicetree/bindings/jnx/jnx-connector | 59 ++++++++++++++++++++++

Don't put vendor directories at top level. I assume this is not
stable, stick it in staging with the driver.

> 1 file changed, 59 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/jnx/jnx-connector
>
> diff --git a/Documentation/devicetree/bindings/jnx/jnx-connector b/Documentation/devicetree/bindings/jnx/jnx-connector
> new file mode 100644
> index 0000000..9262752
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/jnx/jnx-connector
> @@ -0,0 +1,59 @@
> +Device-Tree bindings for Juniper connector driver
> +
> +Required properties:
> + - compatible - Must be one of
> + - "jnx,pic-connector"
> + - "jnx,sib-connector"
> + - "jnx,fpc-connector"
> + - "jnx,cb-connector"
> + - "jnx,psm-connector"
> + - "jnx,fan-connector"
> + - "jnx,mp-connector"
> + - "jnx,fpm-connector"
> + - "jnx,spmb-connector"

I imagine all these have different signals. You need to be explicit
which properties below apply to which compatible.

> + "simple-bus" must also be specified to facilitate instantiation.

No. This looks like abuse of simple-bus.

> + - slot - slot number

Does reg work here instead?

> +
> +Optional properties:
> + - assembly-id - Static assembly ID to be used for all adapters.
> + If specified, it is assumed that no ID eeprom is
> + present, and the provided assembly ID will always be
> + used to identify the adapter and the devicetree overlay
> + to load.
> + - presence-detect-gpios - presence detect gpio pin
> + If not provided, card is assumed
> + to be always present.
> + - attention-button-gpios - request button gpio pin
> + - attention-button-ignore - set if kernel shall ignore
> + attention button events
> + - attention-button-holdtime - button hold time in milli-seconds
> + (3s default)
> + - activation-timeout - activation timeout in milli-seconds (10s default)
> + - debounce-interval - debounce interval in milli-seconds
> + (applies to presence detect pin).

unit suffixes on these. You should know this.

> + - power-enable-gpios - power enable gpio pin
> + - power-status-gpios - power status gpio pin (mandatory if
> + power-enable-gpios is provided)
> + - power-enable-timeout - power enable timeout in ms (1s default)
> + - poweron-reset-delay - delay to be applied after poweron in
> + milli-seconds (default none)
> + - reset-gpios - board reset gpio pin
> + - led-green - phandle pointing to green LED
> + - led-red - phandle pointing to red LED

Are all these gpios endpoints? i.e don't need some gpio translation
for downstream card devices? If so, then they mostly look fine.

> + - sysfs-linkname - name to be user for the symbolic link under jnx/card

Err, NAK.

> +
> +Example node:
> +
> + pic0 {
> + compatible = "jnx,pic-connector", "simple-bus";
> + slot = <0>;
> + presence-detect-gpios = <&gpio1 148 0x10013>;
> + attention-button-gpios = <&gpio20 150 0x10013>;
> + power-enable-gpios = <&gpio1 154 0x0>;
> + power-status-gpios = <&gpio1 151 0x3>;
> + reset-gpios = <&gpio1 153 0x10000>;
> + power-enable-timeout = <2000>;
> + debounce-interval = <1>;
> + led-green = <&pic0_green>;
> + led-red = <&pic0_red>;
> + };
> --
> 1.9.1
>