netdev - Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190805141100.GG24275@lunn.ch>
Date:   Mon, 5 Aug 2019 16:11:00 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Alexandru Ardelean <alexandru.ardelean@...log.com>
Cc:     netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, davem@...emloft.net,
        robh+dt@...nel.org, mark.rutland@....com, f.fainelli@...il.com,
        hkallweit1@...il.com
Subject: Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver

> +  adi,rx-internal-delay:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> +      is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> +      default value is 0 (which represents 2 ns)
> +    enum: [ 0, 1, 2, 6, 7 ]

We want these numbers to be in ns. So the default value would actually
be 2. The driver needs to convert the number in DT to a value to poke
into a PHY register. Please rename the property adi,rx-internal-delay-ns.

> +
> +  adi,tx-internal-delay:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> +      is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> +      default value is 0 (which represents 2 ns)
> +    enum: [ 0, 1, 2, 6, 7 ]

Same here.

> +
> +  adi,fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      When operating in RMII mode, this option configures the FIFO depth.
> +      See `dt-bindings/net/adin.h`.
> +    enum: [ 0, 1, 2, 3, 4, 5 ]

Units? You should probably rename this adi,fifo-depth-bits and list
the valid values in bits.

> +
> +  adi,eee-enabled:
> +    description: |
> +      Advertise EEE capabilities on power-up/init (default disabled)
> +    type: boolean

It is not the PHY which decides this. The MAC indicates if it is EEE
capable to phylib. phylib looks into the PHY registers to determine if
the PHY supports EEE. phylib will then enable EEE
advertisement. Please remove this, and ensure EEE is disabled by
default.

	Andrew