netdev - Re: dp83867: Why does ti,fifo-depth set only TX, and why is it mandatory?

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: <190bd4d3-4bbd-3684-da31-2335b7c34c2a@ti.com>
Date:   Thu, 14 Nov 2019 11:53:36 -0600
From:   Dan Murphy <dmurphy@...com>
To:     Adrian Bunk <bunk@...nel.org>
CC:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        <netdev@...r.kernel.org>
Subject: Re: dp83867: Why does ti,fifo-depth set only TX, and why is it
 mandatory?

Adrian

On 11/14/19 10:24 AM, Adrian Bunk wrote:
> Hi,
>
> looking at the ti,fifo-depth property to set the TX FIFO Depth in the
> dp83867 driver I was wondering:
>
> 1. Why does it set only TX?
> Is there a reason why TX needs setting but RX does not?
> (RX FIFO Depth is SGMII-only, but that's what I am using)

There was no RX fifo depth setting for this device only TX fifo depth 
setting at the original submission.

See 8.6.14 PHY Control Register (PHYCR) only defines tx

> 2. Why is it a mandatory property?
> Perhaps I am missing something obvious, but why can't the driver either
> leave the value untouched or set the maximum when nothing is configured?

When the driver was originally written it was written only for RGMII 
interfaces as that is the MII that the data sheet references and does 
not reference SGMII.  We did not have SGMII samples available at that 
time. According to the HW guys setting the FIFO depth is required for 
RGMII interfaces.  When SGMII support was added in commit 
507ddd5c0d47ad869f361c71d700ffe7f12d1dd6 the rx fifo-depth DT property 
should have been added and both tx and rx should have been made 
optional.  We should probably deprecate the ti,fifo-depth in favor of 
the standard rx-fifo-depth and tx-fifo-depth common properties.

Dan