Re: [PATCH net-next v4 4/4] net: ftgmac100: Add RGMII delay support for AST2600

[Andrew Lunn <andrew@lunn.ch>]

> Based on the above information, I have attempted to outline my understanding.
> 1. 'rgmii' + MAC delay:
> Add warming, keep MAC delay and pass "rgmii-id" to PHY driver.

Think about that. What delays do you get as a result?

> 2. 'rgmii-id' + MAC delay:
> disable MAC delay and pass "rgmii-id" to PHY driver
> 
> 3. 'rgmii-id' + no MAC delay:
> Keep disabling MAC delay and pass "rgmii-id" to PHY driver
> 
> 4. 'rgmii-txid' or 'rgmii-rxid':
> Keep original setting

> I have some idea to discuss with you.
> 1. On 'rgmii', I want to add warming and directly disable MAC delay and pass 'rgmii-id' 
> to PHY driver.

Yep.

> 
> 2. On 'rgmii-id', ignore if enabling MAC delay, all disable MAC delay and pass ' rgmii-id'
> to PHY driver.
> 
> 3. On 'rgmii-txid' or 'rgmii-rxid', keep the above item 4.
> 
> Actually, it's difficult for the driver to determine whether the MAC delay is enabled or not.
> Our design doesn't use a single bit to indicate the delay state. Instead, the delay setting is 
> derived from the user's configured delay value.

But you can turn it back to ps. I would say, if it is > 1000, the
intention is it provides the 2ns delay. If it is < 1000 it is just a
minor tuning value because of bad board design.

Do you have experience from the field, what do real boards use? Are
they all inserting the same 2ns? Or is each customer tuning their
bootloader to configure the hardware differently per board design?

You might even need a more complex solution. If the bootloader is
adding a delay between say 200ps and 1600ps, it suggests a poorly
designed board, and the PHY adding 2ns is not going to work. There is
a need for rx-internal-delay-ps or tx-internal-delay-ps in DT. You can
give a warning, and indicate what values are needed, but leave the
hardware alone. If the bootloader is setting the delay > 1600, passing
_RGMII_ID to the PHY, and disabling MAC delays is likely to work, so
you just need to warn about phy-mode being wrong. If the bootloader is
inserting < 200ps, and phy-mode is rgmii-id, that is just fine
tuning. Maybe suggest rx-internal-delay-ps or tx-internal-delay-ps be
added in DT, but leave it alone.

Whatever you do, you need a lot of comments in the code and the commit
message to help developers understand why they are seeing warnings and
what they need to do.

	Andrew