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

[Adrian Bunk <bunk@kernel.org>]

On Thu, Nov 14, 2019 at 11:53:36AM -0600, Dan Murphy wrote:
> Adrian

Hi Dan,

>...
> > 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.

My reading of the datasheets is that it isn't needed at all for RGMII,
only for SGMII and gigabit GMII.

Which makes it weird that it is only written in the RGMII case where it
is documented to be disabled.

And there is a documented default value so writing shouldn't be mandatory
in any case.

Perhaps I am looking at the wrong datasheets or there's a hardware errata?

> When SGMII support was added in commit
> 507ddd5c0d47ad869f361c71d700ffe7f12d1dd6

That's adding 6-wire mode support, the version of the driver I use with 
SGMII in 4.14 is much older and not far from the original submission.

Is there anything that might be missing for SGMII you are aware of?

> 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

Thanks
Adrian