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
