netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/4] net: bcmgenet: restore internal EPHY support
@ 2019-10-16 23:06 Doug Berger
  2019-10-16 23:06 ` [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC Doug Berger
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Doug Berger @ 2019-10-16 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit,
	bcm-kernel-feedback-list, netdev, linux-kernel, Doug Berger

I managed to get my hands on an old BCM97435SVMB board to do some
testing with the latest kernel and uncovered a number of things
that managed to get broken over the years (some by me ;).

This commit set attempts to correct the errors I observed in my
testing.

The first commit applies to all internal PHYs to restore proper
reporting of link status when a link comes up.

The second commit restores the soft reset to the initialization of
the older internal EPHYs used by 40nm Set-Top Box devices.

The third corrects a bug I introduced when removing excessive soft
resets by altering the initialization sequence in a way that keeps
the GENETv3 MAC interface happy.

Finally, I observed a number of issues when manually configuring
the network interface of the older EPHYs that appear to be resolved
by the fourth commit.

Doug Berger (4):
  net: bcmgenet: don't set phydev->link from MAC
  net: phy: bcm7xxx: define soft_reset for 40nm EPHY
  net: bcmgenet: soft reset 40nm EPHYs before MAC init
  net: bcmgenet: reset 40nm EPHY on energy detect

 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  41 +++++----
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |   2 +-
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 112 +++++++++++--------------
 drivers/net/phy/bcm7xxx.c                      |   1 +
 4 files changed, 79 insertions(+), 77 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC
  2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
@ 2019-10-16 23:06 ` Doug Berger
  2019-10-17  3:50   ` Florian Fainelli
  2019-10-16 23:06 ` [PATCH net 2/4] net: phy: bcm7xxx: define soft_reset for 40nm EPHY Doug Berger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Doug Berger @ 2019-10-16 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit,
	bcm-kernel-feedback-list, netdev, linux-kernel, Doug Berger

When commit 28b2e0d2cd13 ("net: phy: remove parameter new_link from
phy_mac_interrupt()") removed the new_link parameter it set the
phydev->link state from the MAC before invoking phy_mac_interrupt().

However, once commit 88d6272acaaa ("net: phy: avoid unneeded MDIO
reads in genphy_read_status") was added this initialization prevents
the proper determination of the connection parameters by the function
genphy_read_status().

This commit removes that initialization to restore the proper
functionality.

Fixes: 88d6272acaaa ("net: phy: avoid unneeded MDIO reads in genphy_read_status")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 12cb77ef1081..10d68017ff6c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2612,10 +2612,8 @@ static void bcmgenet_irq_task(struct work_struct *work)
 	spin_unlock_irq(&priv->lock);
 
 	/* Link UP/DOWN event */
-	if (status & UMAC_IRQ_LINK_EVENT) {
-		priv->dev->phydev->link = !!(status & UMAC_IRQ_LINK_UP);
+	if (status & UMAC_IRQ_LINK_EVENT)
 		phy_mac_interrupt(priv->dev->phydev);
-	}
 }
 
 /* bcmgenet_isr1: handle Rx and Tx priority queues */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net 2/4] net: phy: bcm7xxx: define soft_reset for 40nm EPHY
  2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
  2019-10-16 23:06 ` [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC Doug Berger
@ 2019-10-16 23:06 ` Doug Berger
  2019-10-17  3:53   ` Florian Fainelli
  2019-10-16 23:06 ` [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init Doug Berger
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Doug Berger @ 2019-10-16 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit,
	bcm-kernel-feedback-list, netdev, linux-kernel, Doug Berger

The internal 40nm EPHYs use a "Workaround for putting the PHY in
IDDQ mode." These PHYs require a soft reset to restore functionality
after they are powered back up.

This commit defines the soft_reset function to use genphy_soft_reset
during phy_init_hw to accommodate this.

Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/net/phy/bcm7xxx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 8fc33867e524..af8eabe7a6d4 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -572,6 +572,7 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
 	.name           = _name,					\
 	/* PHY_BASIC_FEATURES */					\
 	.flags          = PHY_IS_INTERNAL,				\
+	.soft_reset	= genphy_soft_reset,				\
 	.config_init    = bcm7xxx_config_init,				\
 	.suspend        = bcm7xxx_suspend,				\
 	.resume         = bcm7xxx_config_init,				\
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init
  2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
  2019-10-16 23:06 ` [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC Doug Berger
  2019-10-16 23:06 ` [PATCH net 2/4] net: phy: bcm7xxx: define soft_reset for 40nm EPHY Doug Berger
@ 2019-10-16 23:06 ` Doug Berger
  2019-10-17  3:53   ` Florian Fainelli
  2019-10-16 23:06 ` [PATCH net 4/4] net: bcmgenet: reset 40nm EPHY on energy detect Doug Berger
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Doug Berger @ 2019-10-16 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit,
	bcm-kernel-feedback-list, netdev, linux-kernel, Doug Berger

It turns out that the "Workaround for putting the PHY in IDDQ mode"
used by the internal EPHYs on 40nm Set-Top Box chips when powering
down puts the interface to the GENET MAC in a state that can cause
subsequent MAC resets to be incomplete.

Rather than restore the forced soft reset when powering up internal
PHYs, this commit moves the invocation of phy_init_hw earlier in
the MAC initialization sequence to just before the MAC reset in the
open and resume functions. This allows the interface to be stable
and allows the MAC resets to be successful.

The bcmgenet_mii_probe() function is split in two to accommodate
this. The new function bcmgenet_mii_connect() handles the first
half of the functionality before the MAC initialization, and the
bcmgenet_mii_config() function is extended to provide the remaining
PHY configuration following the MAC initialization.

Fixes: 484bfa1507bf ("Revert "net: bcmgenet: Software reset EPHY after power on"")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  28 ++++---
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |   2 +-
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 112 +++++++++++--------------
 3 files changed, 69 insertions(+), 73 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 10d68017ff6c..f0937c650e3c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2872,6 +2872,12 @@ static int bcmgenet_open(struct net_device *dev)
 	if (priv->internal_phy)
 		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
+	ret = bcmgenet_mii_connect(dev);
+	if (ret) {
+		netdev_err(dev, "failed to connect to PHY\n");
+		goto err_clk_disable;
+	}
+
 	/* take MAC out of reset */
 	bcmgenet_umac_reset(priv);
 
@@ -2881,6 +2887,12 @@ static int bcmgenet_open(struct net_device *dev)
 	reg = bcmgenet_umac_readl(priv, UMAC_CMD);
 	priv->crc_fwd_en = !!(reg & CMD_CRC_FWD);
 
+	ret = bcmgenet_mii_config(dev, true);
+	if (ret) {
+		netdev_err(dev, "unsupported PHY\n");
+		goto err_disconnect_phy;
+	}
+
 	bcmgenet_set_hw_addr(priv, dev->dev_addr);
 
 	if (priv->internal_phy) {
@@ -2896,7 +2908,7 @@ static int bcmgenet_open(struct net_device *dev)
 	ret = bcmgenet_init_dma(priv);
 	if (ret) {
 		netdev_err(dev, "failed to initialize DMA\n");
-		goto err_clk_disable;
+		goto err_disconnect_phy;
 	}
 
 	/* Always enable ring 16 - descriptor ring */
@@ -2919,25 +2931,19 @@ static int bcmgenet_open(struct net_device *dev)
 		goto err_irq0;
 	}
 
-	ret = bcmgenet_mii_probe(dev);
-	if (ret) {
-		netdev_err(dev, "failed to connect to PHY\n");
-		goto err_irq1;
-	}
-
 	bcmgenet_netif_start(dev);
 
 	netif_tx_start_all_queues(dev);
 
 	return 0;
 
-err_irq1:
-	free_irq(priv->irq1, priv);
 err_irq0:
 	free_irq(priv->irq0, priv);
 err_fini_dma:
 	bcmgenet_dma_teardown(priv);
 	bcmgenet_fini_dma(priv);
+err_disconnect_phy:
+	phy_disconnect(dev->phydev);
 err_clk_disable:
 	if (priv->internal_phy)
 		bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
@@ -3618,6 +3624,8 @@ static int bcmgenet_resume(struct device *d)
 	if (priv->internal_phy)
 		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
+	phy_init_hw(dev->phydev);
+
 	bcmgenet_umac_reset(priv);
 
 	init_umac(priv);
@@ -3626,8 +3634,6 @@ static int bcmgenet_resume(struct device *d)
 	if (priv->wolopts)
 		clk_disable_unprepare(priv->clk_wol);
 
-	phy_init_hw(dev->phydev);
-
 	/* Speed settings must be restored */
 	bcmgenet_mii_config(priv->dev, false);
 
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index dbc69d8fa05f..7fbf573d8d52 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -720,8 +720,8 @@ GENET_IO_MACRO(rbuf, GENET_RBUF_OFF);
 
 /* MDIO routines */
 int bcmgenet_mii_init(struct net_device *dev);
+int bcmgenet_mii_connect(struct net_device *dev);
 int bcmgenet_mii_config(struct net_device *dev, bool init);
-int bcmgenet_mii_probe(struct net_device *dev);
 void bcmgenet_mii_exit(struct net_device *dev);
 void bcmgenet_phy_power_set(struct net_device *dev, bool enable);
 void bcmgenet_mii_setup(struct net_device *dev);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index e7c291bf4ed1..17bb8d60a157 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -173,6 +173,46 @@ static void bcmgenet_moca_phy_setup(struct bcmgenet_priv *priv)
 					  bcmgenet_fixed_phy_link_update);
 }
 
+int bcmgenet_mii_connect(struct net_device *dev)
+{
+	struct bcmgenet_priv *priv = netdev_priv(dev);
+	struct device_node *dn = priv->pdev->dev.of_node;
+	struct phy_device *phydev;
+	u32 phy_flags = 0;
+	int ret;
+
+	/* Communicate the integrated PHY revision */
+	if (priv->internal_phy)
+		phy_flags = priv->gphy_rev;
+
+	/* Initialize link state variables that bcmgenet_mii_setup() uses */
+	priv->old_link = -1;
+	priv->old_speed = -1;
+	priv->old_duplex = -1;
+	priv->old_pause = -1;
+
+	if (dn) {
+		phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
+					phy_flags, priv->phy_interface);
+		if (!phydev) {
+			pr_err("could not attach to PHY\n");
+			return -ENODEV;
+		}
+	} else {
+		phydev = dev->phydev;
+		phydev->dev_flags = phy_flags;
+
+		ret = phy_connect_direct(dev, phydev, bcmgenet_mii_setup,
+					 priv->phy_interface);
+		if (ret) {
+			pr_err("could not attach to PHY\n");
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
 int bcmgenet_mii_config(struct net_device *dev, bool init)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
@@ -266,71 +306,21 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 		bcmgenet_ext_writel(priv, reg, EXT_RGMII_OOB_CTRL);
 	}
 
-	if (init)
-		dev_info(kdev, "configuring instance for %s\n", phy_name);
-
-	return 0;
-}
+	if (init) {
+		linkmode_copy(phydev->advertising, phydev->supported);
 
-int bcmgenet_mii_probe(struct net_device *dev)
-{
-	struct bcmgenet_priv *priv = netdev_priv(dev);
-	struct device_node *dn = priv->pdev->dev.of_node;
-	struct phy_device *phydev;
-	u32 phy_flags = 0;
-	int ret;
-
-	/* Communicate the integrated PHY revision */
-	if (priv->internal_phy)
-		phy_flags = priv->gphy_rev;
-
-	/* Initialize link state variables that bcmgenet_mii_setup() uses */
-	priv->old_link = -1;
-	priv->old_speed = -1;
-	priv->old_duplex = -1;
-	priv->old_pause = -1;
-
-	if (dn) {
-		phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
-					phy_flags, priv->phy_interface);
-		if (!phydev) {
-			pr_err("could not attach to PHY\n");
-			return -ENODEV;
-		}
-	} else {
-		phydev = dev->phydev;
-		phydev->dev_flags = phy_flags;
-
-		ret = phy_connect_direct(dev, phydev, bcmgenet_mii_setup,
-					 priv->phy_interface);
-		if (ret) {
-			pr_err("could not attach to PHY\n");
-			return -ENODEV;
-		}
-	}
+		/* The internal PHY has its link interrupts routed to the
+		 * Ethernet MAC ISRs. On GENETv5 there is a hardware issue
+		 * that prevents the signaling of link UP interrupts when
+		 * the link operates at 10Mbps, so fallback to polling for
+		 * those versions of GENET.
+		 */
+		if (priv->internal_phy && !GENET_IS_V5(priv))
+			phydev->irq = PHY_IGNORE_INTERRUPT;
 
-	/* Configure port multiplexer based on what the probed PHY device since
-	 * reading the 'max-speed' property determines the maximum supported
-	 * PHY speed which is needed for bcmgenet_mii_config() to configure
-	 * things appropriately.
-	 */
-	ret = bcmgenet_mii_config(dev, true);
-	if (ret) {
-		phy_disconnect(dev->phydev);
-		return ret;
+		dev_info(kdev, "configuring instance for %s\n", phy_name);
 	}
 
-	linkmode_copy(phydev->advertising, phydev->supported);
-
-	/* The internal PHY has its link interrupts routed to the
-	 * Ethernet MAC ISRs. On GENETv5 there is a hardware issue
-	 * that prevents the signaling of link UP interrupts when
-	 * the link operates at 10Mbps, so fallback to polling for
-	 * those versions of GENET.
-	 */
-	if (priv->internal_phy && !GENET_IS_V5(priv))
-		dev->phydev->irq = PHY_IGNORE_INTERRUPT;
-
 	return 0;
 }
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net 4/4] net: bcmgenet: reset 40nm EPHY on energy detect
  2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
                   ` (2 preceding siblings ...)
  2019-10-16 23:06 ` [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init Doug Berger
@ 2019-10-16 23:06 ` Doug Berger
  2019-10-17  3:56   ` Florian Fainelli
  2019-10-17  4:00 ` [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Florian Fainelli
  2019-10-18 17:01 ` David Miller
  5 siblings, 1 reply; 12+ messages in thread
From: Doug Berger @ 2019-10-16 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit,
	bcm-kernel-feedback-list, netdev, linux-kernel, Doug Berger

The EPHY integrated into the 40nm Set-Top Box devices can falsely
detect energy when connected to a disabled peer interface. When the
peer interface is enabled the EPHY will detect and report the link
as active, but on occasion may get into a state where it is not
able to exchange data with the connected GENET MAC. This issue has
not been observed when the link parameters are auto-negotiated;
however, it has been observed with a manually configured link.

It has been empirically determined that issuing a soft reset to the
EPHY when energy is detected prevents it from getting into this bad
state.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index f0937c650e3c..0f138280315a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2018,6 +2018,8 @@ static void bcmgenet_link_intr_enable(struct bcmgenet_priv *priv)
 	 */
 	if (priv->internal_phy) {
 		int0_enable |= UMAC_IRQ_LINK_EVENT;
+		if (GENET_IS_V1(priv) || GENET_IS_V2(priv) || GENET_IS_V3(priv))
+			int0_enable |= UMAC_IRQ_PHY_DET_R;
 	} else if (priv->ext_phy) {
 		int0_enable |= UMAC_IRQ_LINK_EVENT;
 	} else if (priv->phy_interface == PHY_INTERFACE_MODE_MOCA) {
@@ -2611,9 +2613,14 @@ static void bcmgenet_irq_task(struct work_struct *work)
 	priv->irq0_stat = 0;
 	spin_unlock_irq(&priv->lock);
 
+	if (status & UMAC_IRQ_PHY_DET_R &&
+	    priv->dev->phydev->autoneg != AUTONEG_ENABLE)
+		phy_init_hw(priv->dev->phydev);
+
 	/* Link UP/DOWN event */
 	if (status & UMAC_IRQ_LINK_EVENT)
 		phy_mac_interrupt(priv->dev->phydev);
+
 }
 
 /* bcmgenet_isr1: handle Rx and Tx priority queues */
@@ -2708,7 +2715,7 @@ static irqreturn_t bcmgenet_isr0(int irq, void *dev_id)
 	}
 
 	/* all other interested interrupts handled in bottom half */
-	status &= UMAC_IRQ_LINK_EVENT;
+	status &= (UMAC_IRQ_LINK_EVENT | UMAC_IRQ_PHY_DET_R);
 	if (status) {
 		/* Save irq status for bottom-half processing. */
 		spin_lock_irqsave(&priv->lock, flags);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC
  2019-10-16 23:06 ` [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC Doug Berger
@ 2019-10-17  3:50   ` Florian Fainelli
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Fainelli @ 2019-10-17  3:50 UTC (permalink / raw)
  To: Doug Berger, David S. Miller
  Cc: Andrew Lunn, Heiner Kallweit, bcm-kernel-feedback-list, netdev,
	linux-kernel



On 10/16/2019 4:06 PM, Doug Berger wrote:
> When commit 28b2e0d2cd13 ("net: phy: remove parameter new_link from
> phy_mac_interrupt()") removed the new_link parameter it set the
> phydev->link state from the MAC before invoking phy_mac_interrupt().
> 
> However, once commit 88d6272acaaa ("net: phy: avoid unneeded MDIO
> reads in genphy_read_status") was added this initialization prevents
> the proper determination of the connection parameters by the function
> genphy_read_status().
> 
> This commit removes that initialization to restore the proper
> functionality.
> 
> Fixes: 88d6272acaaa ("net: phy: avoid unneeded MDIO reads in genphy_read_status")
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 2/4] net: phy: bcm7xxx: define soft_reset for 40nm EPHY
  2019-10-16 23:06 ` [PATCH net 2/4] net: phy: bcm7xxx: define soft_reset for 40nm EPHY Doug Berger
@ 2019-10-17  3:53   ` Florian Fainelli
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Fainelli @ 2019-10-17  3:53 UTC (permalink / raw)
  To: Doug Berger, David S. Miller
  Cc: Andrew Lunn, Heiner Kallweit, bcm-kernel-feedback-list, netdev,
	linux-kernel, jaedon.shin



On 10/16/2019 4:06 PM, Doug Berger wrote:
> The internal 40nm EPHYs use a "Workaround for putting the PHY in
> IDDQ mode." These PHYs require a soft reset to restore functionality
> after they are powered back up.
> 
> This commit defines the soft_reset function to use genphy_soft_reset
> during phy_init_hw to accommodate this.
> 
> Fixes: 6e2d85ec0559 ("net: phy: Stop with excessive soft reset")
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init
  2019-10-16 23:06 ` [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init Doug Berger
@ 2019-10-17  3:53   ` Florian Fainelli
  2019-10-31 18:57     ` Doug Berger
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2019-10-17  3:53 UTC (permalink / raw)
  To: Doug Berger, David S. Miller
  Cc: Andrew Lunn, Heiner Kallweit, bcm-kernel-feedback-list, netdev,
	linux-kernel



On 10/16/2019 4:06 PM, Doug Berger wrote:
> It turns out that the "Workaround for putting the PHY in IDDQ mode"
> used by the internal EPHYs on 40nm Set-Top Box chips when powering
> down puts the interface to the GENET MAC in a state that can cause
> subsequent MAC resets to be incomplete.
> 
> Rather than restore the forced soft reset when powering up internal
> PHYs, this commit moves the invocation of phy_init_hw earlier in
> the MAC initialization sequence to just before the MAC reset in the
> open and resume functions. This allows the interface to be stable
> and allows the MAC resets to be successful.
> 
> The bcmgenet_mii_probe() function is split in two to accommodate
> this. The new function bcmgenet_mii_connect() handles the first
> half of the functionality before the MAC initialization, and the
> bcmgenet_mii_config() function is extended to provide the remaining
> PHY configuration following the MAC initialization.
> 
> Fixes: 484bfa1507bf ("Revert "net: bcmgenet: Software reset EPHY after power on"")
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

We will have to see how difficult it might be to back port towards
stable trees of interest, hopefully not too difficult.
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 4/4] net: bcmgenet: reset 40nm EPHY on energy detect
  2019-10-16 23:06 ` [PATCH net 4/4] net: bcmgenet: reset 40nm EPHY on energy detect Doug Berger
@ 2019-10-17  3:56   ` Florian Fainelli
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Fainelli @ 2019-10-17  3:56 UTC (permalink / raw)
  To: Doug Berger, David S. Miller
  Cc: Andrew Lunn, Heiner Kallweit, bcm-kernel-feedback-list, netdev,
	linux-kernel



On 10/16/2019 4:06 PM, Doug Berger wrote:
> The EPHY integrated into the 40nm Set-Top Box devices can falsely
> detect energy when connected to a disabled peer interface. When the
> peer interface is enabled the EPHY will detect and report the link
> as active, but on occasion may get into a state where it is not
> able to exchange data with the connected GENET MAC. This issue has
> not been observed when the link parameters are auto-negotiated;
> however, it has been observed with a manually configured link.
> 
> It has been empirically determined that issuing a soft reset to the
> EPHY when energy is detected prevents it from getting into this bad
> state.
> 
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 0/4] net: bcmgenet: restore internal EPHY support
  2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
                   ` (3 preceding siblings ...)
  2019-10-16 23:06 ` [PATCH net 4/4] net: bcmgenet: reset 40nm EPHY on energy detect Doug Berger
@ 2019-10-17  4:00 ` Florian Fainelli
  2019-10-18 17:01 ` David Miller
  5 siblings, 0 replies; 12+ messages in thread
From: Florian Fainelli @ 2019-10-17  4:00 UTC (permalink / raw)
  To: Doug Berger, David S. Miller
  Cc: Andrew Lunn, Heiner Kallweit, bcm-kernel-feedback-list, netdev,
	linux-kernel, jaedon.shin



On 10/16/2019 4:06 PM, Doug Berger wrote:
> I managed to get my hands on an old BCM97435SVMB board to do some
> testing with the latest kernel and uncovered a number of things
> that managed to get broken over the years (some by me ;).
> 
> This commit set attempts to correct the errors I observed in my
> testing.
> 
> The first commit applies to all internal PHYs to restore proper
> reporting of link status when a link comes up.
> 
> The second commit restores the soft reset to the initialization of
> the older internal EPHYs used by 40nm Set-Top Box devices.
> 
> The third corrects a bug I introduced when removing excessive soft
> resets by altering the initialization sequence in a way that keeps
> the GENETv3 MAC interface happy.
> 
> Finally, I observed a number of issues when manually configuring
> the network interface of the older EPHYs that appear to be resolved
> by the fourth commit.

Thank you very much for addressing all of those problems!

> 
> Doug Berger (4):
>   net: bcmgenet: don't set phydev->link from MAC
>   net: phy: bcm7xxx: define soft_reset for 40nm EPHY
>   net: bcmgenet: soft reset 40nm EPHYs before MAC init
>   net: bcmgenet: reset 40nm EPHY on energy detect
> 
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c |  41 +++++----
>  drivers/net/ethernet/broadcom/genet/bcmgenet.h |   2 +-
>  drivers/net/ethernet/broadcom/genet/bcmmii.c   | 112 +++++++++++--------------
>  drivers/net/phy/bcm7xxx.c                      |   1 +
>  4 files changed, 79 insertions(+), 77 deletions(-)
> 

-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 0/4] net: bcmgenet: restore internal EPHY support
  2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
                   ` (4 preceding siblings ...)
  2019-10-17  4:00 ` [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Florian Fainelli
@ 2019-10-18 17:01 ` David Miller
  5 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2019-10-18 17:01 UTC (permalink / raw)
  To: opendmb
  Cc: f.fainelli, andrew, hkallweit1, bcm-kernel-feedback-list, netdev,
	linux-kernel

From: Doug Berger <opendmb@gmail.com>
Date: Wed, 16 Oct 2019 16:06:28 -0700

> I managed to get my hands on an old BCM97435SVMB board to do some
> testing with the latest kernel and uncovered a number of things
> that managed to get broken over the years (some by me ;).
> 
> This commit set attempts to correct the errors I observed in my
> testing.
> 
> The first commit applies to all internal PHYs to restore proper
> reporting of link status when a link comes up.
> 
> The second commit restores the soft reset to the initialization of
> the older internal EPHYs used by 40nm Set-Top Box devices.
> 
> The third corrects a bug I introduced when removing excessive soft
> resets by altering the initialization sequence in a way that keeps
> the GENETv3 MAC interface happy.
> 
> Finally, I observed a number of issues when manually configuring
> the network interface of the older EPHYs that appear to be resolved
> by the fourth commit.

Series applied and queued up for -stable.

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init
  2019-10-17  3:53   ` Florian Fainelli
@ 2019-10-31 18:57     ` Doug Berger
  0 siblings, 0 replies; 12+ messages in thread
From: Doug Berger @ 2019-10-31 18:57 UTC (permalink / raw)
  To: Florian Fainelli, David S. Miller
  Cc: Andrew Lunn, Heiner Kallweit, bcm-kernel-feedback-list, netdev,
	linux-kernel

On 10/16/19 8:53 PM, Florian Fainelli wrote:
> 
> 
> On 10/16/2019 4:06 PM, Doug Berger wrote:
>> It turns out that the "Workaround for putting the PHY in IDDQ mode"
>> used by the internal EPHYs on 40nm Set-Top Box chips when powering
>> down puts the interface to the GENET MAC in a state that can cause
>> subsequent MAC resets to be incomplete.
>>
>> Rather than restore the forced soft reset when powering up internal
>> PHYs, this commit moves the invocation of phy_init_hw earlier in
>> the MAC initialization sequence to just before the MAC reset in the
>> open and resume functions. This allows the interface to be stable
>> and allows the MAC resets to be successful.
>>
>> The bcmgenet_mii_probe() function is split in two to accommodate
>> this. The new function bcmgenet_mii_connect() handles the first
>> half of the functionality before the MAC initialization, and the
>> bcmgenet_mii_config() function is extended to provide the remaining
>> PHY configuration following the MAC initialization.
>>
>> Fixes: 484bfa1507bf ("Revert "net: bcmgenet: Software reset EPHY after power on"")
>> Signed-off-by: Doug Berger <opendmb@gmail.com>
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> We will have to see how difficult it might be to back port towards
> stable trees of interest, hopefully not too difficult.
> 

There is more going on here with the MAC reset than this commit resolves
and I am actively investigating a different approach that may include
the reversion of this commit.

Therefore, I would recommend not spending effort attempting to backport
this specific commit until I have submitted a more complete solution
(hopefully in the near future :).

Sorry for any confusion this creates.
    Doug

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-10-31 18:57 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-16 23:06 [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Doug Berger
2019-10-16 23:06 ` [PATCH net 1/4] net: bcmgenet: don't set phydev->link from MAC Doug Berger
2019-10-17  3:50   ` Florian Fainelli
2019-10-16 23:06 ` [PATCH net 2/4] net: phy: bcm7xxx: define soft_reset for 40nm EPHY Doug Berger
2019-10-17  3:53   ` Florian Fainelli
2019-10-16 23:06 ` [PATCH net 3/4] net: bcmgenet: soft reset 40nm EPHYs before MAC init Doug Berger
2019-10-17  3:53   ` Florian Fainelli
2019-10-31 18:57     ` Doug Berger
2019-10-16 23:06 ` [PATCH net 4/4] net: bcmgenet: reset 40nm EPHY on energy detect Doug Berger
2019-10-17  3:56   ` Florian Fainelli
2019-10-17  4:00 ` [PATCH net 0/4] net: bcmgenet: restore internal EPHY support Florian Fainelli
2019-10-18 17:01 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).