Re: [PATCH net 1/3] net: bcmgenet: use RGMII loopback for MAC reset

[Scott Branden]

On 2019-11-05 11:27 a.m., Doug Berger wrote:
> On 11/5/19 11:14 AM, Scott Branden wrote:
> > Hi Doug,
> >
> > On 2019-11-05 11:07 a.m., Doug Berger wrote:
> > > As noted in commit 28c2d1a7a0bf ("net: bcmgenet: enable loopback
> > > during UniMAC sw_reset") the UniMAC must be clocked while sw_reset
> > > is asserted for its state machines to reset cleanly.
> > >
> > > The transmit and receive clocks used by the UniMAC are derived from
> > > the signals used on its PHY interface. The bcmgenet MAC can be
> > > configured to work with different PHY interfaces including MII,
> > > GMII, RGMII, and Reverse MII on internal and external interfaces.
> > > Unfortunately for the UniMAC, when configured for MII the Tx clock
> > > is always driven from the PHY which places it outside of the direct
> > > control of the MAC.
-- SNIP
> > > +ÂÂÂÂÂÂÂ /* Switch MAC clocking to RGMII generated clock */
> > > +ÂÂÂÂÂÂÂ bcmgenet_sys_writel(priv, PORT_MODE_EXT_GPHY, SYS_PORT_CTRL);
> > > +ÂÂÂÂÂÂÂ /* Ensure 5 clks with Rx disabled
> > > +ÂÂÂÂÂÂÂÂ * followed by 5 clks with Reset asserted
> > > +ÂÂÂÂÂÂÂÂ */
> > > +ÂÂÂÂÂÂÂ udelay(4);
> > How do these magic delays work, they are different values?
> > In one case you have a udelay(4) to ensure rx disabled for 5 clks.
> > Yet below you have a udelay(2) to ensure 4 more clocks?
> The delays are based on 2.5MHz clock cycles (the clock used for 10Mbps).
> 5 clocks is 2us.
>
> The udelay(4) is for 10 clocks: rx is disabled for 5 and then 5 more
> clocks with reset held. The requirement is poorly specified and this is
> a conservative interpretation.
>
> The udelay(2) allows at least 5 more clocks without reset before rx can
> be enabled.
Thanks, the part I was missing was "2.5MHz clock cycles (the clock used 
for 10Mbps)".
If that was added to the comment it would help those unfamiliar with in 
understanding.
> > > +ÂÂÂÂÂÂÂ reg &= ~(CMD_SW_RESET | CMD_LCL_LOOP_EN);
> > > +ÂÂÂÂÂÂÂ bcmgenet_umac_writel(priv, reg, UMAC_CMD);
> > > +ÂÂÂÂÂÂÂ /* Ensure 5 more clocks before Rx is enabled */
> > > +ÂÂÂÂÂÂÂ udelay(2);
> > > +ÂÂÂ }
> > > +
> > >  ÂÂÂÂÂ priv->ext_phy = !priv->internal_phy &&
> > >  ÂÂÂÂÂÂÂÂÂÂÂÂÂ (priv->phy_interface != PHY_INTERFACE_MODE_MOCA);
> > >  Â @@ -254,6 +284,9 @@ int bcmgenet_mii_config(struct net_device *dev,
> > > bool init)
> > >  ÂÂÂÂÂÂÂÂÂ phy_set_max_speed(phydev, SPEED_100);
> > >  ÂÂÂÂÂÂÂÂÂ bcmgenet_sys_writel(priv,
> > >  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ PORT_MODE_EXT_EPHY, SYS_PORT_CTRL);
> > > +ÂÂÂÂÂÂÂ /* Restore the MII PHY after isolation */
> > > +ÂÂÂÂÂÂÂ if (bmcr >= 0)
> > > +ÂÂÂÂÂÂÂÂÂÂÂ phy_write(phydev, MII_BMCR, bmcr);
> > >  ÂÂÂÂÂÂÂÂÂ break;
> > >  Â ÂÂÂÂÂ case PHY_INTERFACE_MODE_REVMII:
> Regards,
>      Doug