linux-kernel - Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal delay for DP83869

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] [day] [month] [year] [list]

Message-ID: <ecee51d5-a69e-4f7e-2a33-ae7aae63fec8@ti.com>
Date:   Wed, 20 May 2020 15:55:14 -0500
From:   Dan Murphy <dmurphy@...com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Florian Fainelli <f.fainelli@...il.com>, <hkallweit1@...il.com>,
        <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal
 delay for DP83869

Andrew

On 5/20/20 3:44 PM, Andrew Lunn wrote:
>> I think adding it in the core would be a bit of a challenge.  I think each
>> PHY driver needs to handle parsing and validating this property on its own
>> (like fifo-depth).  It is a PHY specific setting.
> fifo-depth yes. But some delays follow a common pattern.
>
> e.g.
> Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
>
> Device Tree Value     Delay   Pad Skew Register Value
>    -----------------------------------------------------
>          0               -840ps          0000
>          200             -720ps          0001
>          400             -600ps          0010
>          600             -480ps          0011
>          800             -360ps          0100
>          1000            -240ps          0101
>          1200            -120ps          0110
>          1400               0ps          0111
>          1600             120ps          1000
>          1800             240ps          1001
>          2000             360ps          1010
>          2200             480ps          1011
>          2400             600ps          1100
>          2600             720ps          1101
>          2800             840ps          1110
>          3000             960ps          1111
>
> Documentation/devicetree/bindings/net/adi,adin.yaml
>
>   adi,rx-internal-delay-ps:
>      description: |
>        RGMII RX Clock Delay used only when PHY operates in RGMII mode with
>        internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
>      enum: [ 1600, 1800, 2000, 2200, 2400 ]
>      default: 2000
>
>    adi,tx-internal-delay-ps:
>      description: |
>        RGMII TX Clock Delay used only when PHY operates in RGMII mode with
>        internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
>      enum: [ 1600, 1800, 2000, 2200, 2400 ]
>      default: 2000
>
> Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>
> - tx-delay: Delay value for RGMII bridge TX clock.
>              Valid values are between 0 to 7, that maps to
>              417, 717, 1020, 1321, 1611, 1913, 2215, 2514 ps
>              Default value is 4, which corresponds to 1611 ps
> - rx-delay: Delay value for RGMII bridge RX clock.
>              Valid values are between 0 to 7, that maps to
>              273, 589, 899, 1222, 1480, 1806, 2147, 2464 ps
>              Default value is 2, which corresponds to 899 ps
>
> You could implement checking against a table of valid values, which is
> something you need for your PHY. You could even consider making it a
> 2D table, and return the register value, not the delay?

So provide a helper function that will just basically parse an array and 
return the indexed value.

The outlier here is the AD device since the index to value is not 1-1 
mapping.  Not sure we need a 2D table like the AD driver.

I actually implemented this in the dp83869 a bit ago and have done this 
in a few other non-PHY drivers.

I guess I can look at making it a utility function in the networking space.

Dan

>
> 	  Andrew