Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH net 1/3] net: bcmgenet: use RGMII loopback for MAC reset
To:     Doug Berger <opendmb@gmail.com>,
        "David S. Miller" <davem@davemloft.net>
Cc:     Florian Fainelli <f.fainelli@gmail.com>,
        bcm-kernel-feedback-list@broadcom.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1572980846-37707-1-git-send-email-opendmb@gmail.com>
 <1572980846-37707-2-git-send-email-opendmb@gmail.com>
 <8c5c8028-a897-bf70-95ba-a1ffc8b68264@broadcom.com>
 <5f68345e-c58d-3d99-189b-b4be39c4b899@gmail.com>
From:   Scott Branden <scott.branden@broadcom.com>
Message-ID: <6bc48252-952c-008d-f43d-b8093c021ba8@broadcom.com>
Date:   Tue, 5 Nov 2019 11:48:56 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <5f68345e-c58d-3d99-189b-b4be39c4b899@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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