On Tue, Jun 16, 2020 at 09:49:56AM +0200, Helmut Grohne wrote: > The macb driver does not support configuring rgmii delays. At least for > the Zynq GEM, delays are not supported by the hardware at all. However, > the driver happily accepts and ignores any such delays. > > When operating in a mac to phy connection, the delay setting applies to > the phy. Since the MAC does not support delays, the phy must provide > them and the only supported mode is rgmii-id. However, in a fixed mac > to mac connection, the delay applies to the mac itself. Therefore the > only supported rgmii mode is rgmii.
This seems incorrect - see the phy documentation in Documentation/networking/phy.rst: * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any internal delay by itself, it assumes that either the Ethernet MAC (if capable or the PCB traces) insert the correct 1.5-2ns delay * PHY_INTERFACE_MODE_RGMII_TXID: the PHY should insert an internal delay for the transmit data lines (TXD[3:0]) processed by the PHY device * PHY_INTERFACE_MODE_RGMII_RXID: the PHY should insert an internal delay for the receive data lines (RXD[3:0]) processed by the PHY device * PHY_INTERFACE_MODE_RGMII_ID: the PHY should insert internal delays for both transmit AND receive data lines from/to the PHY device Note that PHY_INTERFACE_MODE_RGMII, the delay can be added by _either_ the MAC or by PCB trace routing. The individual RGMII delay modes are more about what the PHY itself is asked to do with respect to inserting delays, so I don't think your patch makes sense. In any case... > Link: https://lore.kernel.org/netdev/20200610081236.GA31659@laureti-dev/ > Signed-off-by: Helmut Grohne <helmut.gro...@intenta.de> > --- > drivers/net/ethernet/cadence/macb_main.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c > b/drivers/net/ethernet/cadence/macb_main.c > index 5b9d7c60eebc..bee5bf65e8b3 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -514,7 +514,7 @@ static void macb_validate(struct phylink_config *config, > state->interface != PHY_INTERFACE_MODE_RMII && > state->interface != PHY_INTERFACE_MODE_GMII && > state->interface != PHY_INTERFACE_MODE_SGMII && > - !phy_interface_mode_is_rgmii(state->interface)) { > + state->interface != PHY_INTERFACE_MODE_RGMII_ID) { Here you reject everything except PHY_INTERFACE_MODE_RGMII_ID. > bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS); > return; > } > @@ -694,6 +694,13 @@ static int macb_phylink_connect(struct macb *bp) > struct phy_device *phydev; > int ret; > > + if (of_phy_is_fixed_link(dn) && > + phy_interface_mode_is_rgmii(bp->phy_interface) && > + bp->phy_interface != PHY_INTERFACE_MODE_RGMII) { but here you reject everything except PHY_INTERFACE_MODE_RGMII. These can't both be right. If you start with PHY_INTERFACE_MODE_RGMII, and have a fixed link, you'll have PHY_INTERFACE_MODE_RGMII passed into the validate function, which will then fail. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
