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

[Biao Huang (黄彪)]

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

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