Re: [PATCH v5 1/2] stmmac: dwmac-mediatek: enable 2ns delay only for special cases

Dear Andrew,
	Thanks for your comments.

On Fri, 2022-12-23 at 15:18 +0100, Andrew Lunn wrote:

On Fri, Dec 23, 2022 at 09:50:28AM +0800, Biao Huang wrote:

quoted

In current driver, MAC will always enable 2ns delay in RGMII mode,
but that will lead to transmission failures for "rgmii-id"/"rgmii-
txid"
cases.

Modify the implementation of fix_mac_speed() to ensure the 2ns
delay
will only take effect for "rgmii-rxid"/"rgmii" cases, then user can
choose phy-mode freely.

This does not seem correct. There are three ways the delays can be
added:

1) The MAC
2) The PHY
3) Extra long lines on the board.

What the four RGMII modes tell you is what is needed in addition to
whatever the board provides. So it describes the combination of 1)
and
2). Your board does not appear to be applying any delays, so you
should be using rgmii-id.

The MAC and PHY driver then need to decide how to add these delays,
and in most cases, the MAC does nothing, and passes phy-mode to the
PHY and the PHY adds the delay.

The MAC can add delays, but if it does, it need to mask out the
delays
it added to the value passed to the PHY. Otherwise the PHY will add
the delay as well.

in the ethernet-controller.yaml,
 77       # RX and TX delays are added by the MAC when required
 78       - rgmii
 79
 80       # RGMII with internal RX and TX delays provided by the PHY,
 81       # the MAC should not add the RX or TX delays in this case
 82       - rgmii-id
 83
 84       # RGMII with internal RX delay provided by the PHY, the MAC
 85       # should not add an RX delay in this case
 86       - rgmii-rxid
 87
 88       # RGMII with internal TX delay provided by the PHY, the MAC
 89       # should not add an TX delay in this case
 90       - rgmii-txid

so, why don't MAC set delay according to rgmii-** ?
rgmii-rxid   --> mac set tx, but not set rx delay
rgmii-txid   --> mac set rx, but not set tx delay
...

and PHY seems set delay according to rgmii-** in their dirver.

quoted

 
-	if ((phy_interface_mode_is_rgmii(priv_plat->phy_mode))) {
+	switch (priv_plat->phy_mode) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
 		/* prefer 2ns fixed delay which is controlled by
TXC_PHASE_CTRL,
 		 * when link speed is 1Gbps with RGMII interface,
 		 * Fall back to delay macro circuit for 10/100Mbps link
speed.

So this is wrong. PHY_INTERFACE_MODE_RGMII means the board is adding
the delay via long lines. You should not be added any delay at all.

For PHY_INTERFACE_MODE_RGMII_RXID, you need to mask the RXID bit from
phy_mode when connecting the MAC to the PHY. Otherwise the PHY is
going to add this delay as well.

These lines only set tx delay when phy_mode is RGMII/RGMI_RXID, when
phy will not add tx delay internally.
We don't want to restrict phy-mode to rgmii-id only, so I don't think
we need mask RXID bit, then, User can use rgmii-rxid/-txid/-id as their
will.

If I misunderstood, please correct me. Thanks.

	 Andrew

Best Regards!
Biao
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel