| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8c5c8028-a897-bf70-95ba-a1ffc8b68264@broadcom.com>
Date: Tue, 5 Nov 2019 11:14:16 -0800
From: Scott Branden <scott.branden@...adcom.com>
To: Doug Berger <opendmb@...il.com>,
"David S. Miller" <davem@...emloft.net>
Cc: Florian Fainelli <f.fainelli@...il.com>,
bcm-kernel-feedback-list@...adcom.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 1/3] net: bcmgenet: use RGMII loopback for MAC reset
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.
>
> The earlier commit enabled a local loopback mode within the UniMAC
> so that the receive clock would be derived from the transmit clock
> which addressed the observed issue with an external GPHY disabling
> it's Rx clock. However, when a Tx clock is not available this
> loopback is insufficient.
>
> This commit implements a workaround that leverages the fact that
> the MAC can reliably generate all of its necessary clocking by
> enterring the external GPHY RGMII interface mode with the UniMAC in
> local loopback during the sw_reset interval. Unfortunately, this
> has the undesirable side efect of the RGMII GTXCLK signal being
> driven during the same window.
>
> In most configurations this is a benign side effect as the signal
> is either not routed to a pin or is already expected to drive the
> pin. The one exception is when an external MII PHY is expected to
> drive the same pin with its TX_CLK output creating output driver
> contention.
>
> This commit exploits the IEEE 802.3 clause 22 standard defined
> isolate mode to force an external MII PHY to present a high
> impedance on its TX_CLK output during the window to prevent any
> contention at the pin.
>
> The MII interface is used internally with the 40nm internal EPHY
> which agressively disables its clocks for power savings leading to
> incomplete resets of the UniMAC and many instabilities observed
> over the years. The workaround of this commit is expected to put
> an end to those problems.
>
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
> Signed-off-by: Doug Berger <opendmb@...il.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 --
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 33 ++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 0f138280315a..a1776ed8d7a1 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1996,8 +1996,6 @@ static void reset_umac(struct bcmgenet_priv *priv)
>
> /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */
> bcmgenet_umac_writel(priv, CMD_SW_RESET | CMD_LCL_LOOP_EN, UMAC_CMD);
> - udelay(2);
> - bcmgenet_umac_writel(priv, 0, UMAC_CMD);
> }
>
> static void bcmgenet_intr_disable(struct bcmgenet_priv *priv)
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> index 17bb8d60a157..fcd181ae3a7d 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
> @@ -221,8 +221,38 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
> const char *phy_name = NULL;
> u32 id_mode_dis = 0;
> u32 port_ctrl;
> + int bmcr = -1;
> + int ret;
> u32 reg;
>
> + /* MAC clocking workaround during reset of umac state machines */
> + reg = bcmgenet_umac_readl(priv, UMAC_CMD);
> + if (reg & CMD_SW_RESET) {
> + /* An MII PHY must be isolated to prevent TXC contention */
> + if (priv->phy_interface == PHY_INTERFACE_MODE_MII) {
> + ret = phy_read(phydev, MII_BMCR);
> + if (ret >= 0) {
> + bmcr = ret;
> + ret = phy_write(phydev, MII_BMCR,
> + bmcr | BMCR_ISOLATE);
> + }
> + if (ret) {
> + netdev_err(dev, "failed to isolate PHY\n");
> + return ret;
> + }
> + }
> + /* 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?
> + 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:
Powered by blists - more mailing lists