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

[Scott Branden <scott.branden@broadcom.com>]

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