Re: [RFC 13/13] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Andrew Lunn]

> > > > +	if (phy_if == PHY_INTERFACE_MODE_RGMII_ID ||
> > > > +	    phy_if == PHY_INTERFACE_MODE_RGMII_TXID)
> > > > +		rgmii_tx_id |= ICSSG_CTRL_RGMII_ID_MODE;
> > > > +
> > > > +	regmap_update_bits(ctrl_mmr, icssgctrl_reg, ICSSG_CTRL_RGMII_ID_MODE, rgmii_tx_id);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > 
> > > 
> > > O.K, so this does not do what i initially thought it was doing. I was
> > > thinking it was to fine tune the delay, ti,syscon-rgmii-delay would be
> > > a small pico second value to allow the 2ns delay to be tuned to the
> > > board.
> > > 
> > > But now i think this is actually inserting the full 2ns delay?
> > > 
> > > The problem is, you also pass phy_if to of_phy_connect() so the PHY
> > > will also insert the delay if requested. So you end up with double
> > > delays for rgmii_id and rgmii_txid.
> 
> It's misunderstanding here. The bit field name in TRM is RGMII0_ID_MODE
> and meaning:
> 0h - Internal transmit delay is enabled
> 1h - Internal transmit delay is not enabled.
> 
> So here internal delay will be disabled for RGMII_ID/RGMII_TXID.

And enabled for the others?

Why don't you always disable the delays and let the PHY do it? That is
what pretty much every other MAC/PHY combination does.

    Andrew