All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework
@ 2015-07-16 22:51 Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 1/6] net: bcmgenet: Remove excessive PHY reset Florian Fainelli
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

Hi David, Petri, Jaedon,

This patch series reworks how we perform PHY initialization and resets in the
GENET driver. Although this contains mostly fixes, some of the changes are a
bit too intrusive to be backported to 'net' at the moment.

Some of the motivations behind these changes were to reduce the time spent in how
performing MDIO transactions, since it is better to perform then when we have
interrupts enabled. This reduces the bring-up time of GENET from ~600 msecs down
to ~8 msecs, and about the same time for suspend/resume.

Since I do not currently have a system which is not DT-aware, can you (Petri,
Jaedon) give this a try and confirm things keep working as expected?

Thanks!

Florian Fainelli (6):
  net: bcmgenet: Remove excessive PHY reset
  net: bcmgenet: Use correct dev_id for free_irq
  net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up()
  net: bcmgenet: Determine PHY type before scanning MDIO bus
  net: bcmgenet: Delay PHY initialization to bcmgenet_open()
  net: bcmgenet: Remove init parameter from bcmgenet_mii_config

 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 33 +++++-----
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  5 +-
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 84 ++++++++++++--------------
 3 files changed, 59 insertions(+), 63 deletions(-)

-- 
2.1.0

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

* [PATCH net-next 1/6] net: bcmgenet: Remove excessive PHY reset
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
@ 2015-07-16 22:51 ` Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 2/6] net: bcmgenet: Use correct dev_id for free_irq Florian Fainelli
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

We are currently issuing multiple PHY resets during a suspend/resume,
first during bcmgenet_power_up() which does a hardware reset, then a
software reset by calling bcmgenet_mii_reset(). This is both unnecessary
and can take as long as 10ms per MDIO transactions while we re-apply
workarounds because we do not yet have MDIO interrupts enabled.

phy_resume() takes care of re-apply our workarounds in case we need any,
and bcmgenet_power_up() does a PHY hardware reset, all of this is more
than enough to guarantee that the PHY operates correctly.

Fixes: 1c1008c793fa4 ("net: bcmgenet: add main driver file")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  3 ---
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 -
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 11 -----------
 3 files changed, 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 64c1e9db6b0b..674f374dceee 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -907,9 +907,6 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,
 	}
 
 	bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
-
-	if (mode == GENET_POWER_PASSIVE)
-		bcmgenet_mii_reset(priv->dev);
 }
 
 /* ioctl handle special commands that are not present in ethtool. */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 6159deab8c98..9f9ac0089d4d 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -672,7 +672,6 @@ GENET_IO_MACRO(rbuf, GENET_RBUF_OFF);
 int bcmgenet_mii_init(struct net_device *dev);
 int bcmgenet_mii_config(struct net_device *dev, bool init);
 void bcmgenet_mii_exit(struct net_device *dev);
-void bcmgenet_mii_reset(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 adf23d2ac488..c5f9c7b5d9e7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -163,16 +163,6 @@ void bcmgenet_mii_setup(struct net_device *dev)
 	phy_print_status(phydev);
 }
 
-void bcmgenet_mii_reset(struct net_device *dev)
-{
-	struct bcmgenet_priv *priv = netdev_priv(dev);
-
-	if (priv->phydev) {
-		phy_init_hw(priv->phydev);
-		phy_start_aneg(priv->phydev);
-	}
-}
-
 void bcmgenet_phy_power_set(struct net_device *dev, bool enable)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
@@ -215,7 +205,6 @@ static void bcmgenet_internal_phy_setup(struct net_device *dev)
 	reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 	reg |= EXT_PWR_DN_EN_LD;
 	bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
-	bcmgenet_mii_reset(dev);
 }
 
 static void bcmgenet_moca_phy_setup(struct bcmgenet_priv *priv)
-- 
2.1.0

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

* [PATCH net-next 2/6] net: bcmgenet: Use correct dev_id for free_irq
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 1/6] net: bcmgenet: Remove excessive PHY reset Florian Fainelli
@ 2015-07-16 22:51 ` Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 3/6] net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up() Florian Fainelli
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

bcmgenet_open()'s error path call free_irq() with a dev_id argument
different from the one we used to call request_irq() with, this will
make us trip over the warning in kernel/irq/manage.c:__free_irq()

Fixes: 1c1008c793fa4 ("net: bcmgenet: add main driver file")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 674f374dceee..c634ddbbd21d 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2695,7 +2695,7 @@ static int bcmgenet_open(struct net_device *dev)
 	return 0;
 
 err_irq0:
-	free_irq(priv->irq0, dev);
+	free_irq(priv->irq0, priv);
 err_fini_dma:
 	bcmgenet_fini_dma(priv);
 err_clk_disable:
-- 
2.1.0

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

* [PATCH net-next 3/6] net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up()
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 1/6] net: bcmgenet: Remove excessive PHY reset Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 2/6] net: bcmgenet: Use correct dev_id for free_irq Florian Fainelli
@ 2015-07-16 22:51 ` Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 4/6] net: bcmgenet: Determine PHY type before scanning MDIO bus Florian Fainelli
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

We are currently disabling the GPHY interface during bcmgenet_close(),
and attempting to power it back on during bcmgenet_open(). This works
fine for the first time, because we called bcmgenet_mii_config() which
took care of enabling the interface, however, bcmgenet_power_up() really
needs to power on the GPHY for correctness.

This will be particularly important as we want to move
bcmgenet_mii_probe() down to bcmgenet_open() to avoid seeing the "PHY
already attached" message.

Fixes: a642c4f7906f36 ("net: bcmgenet: power up and down integrated GPHY when unused")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index c634ddbbd21d..2efe72f94869 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -907,6 +907,8 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,
 	}
 
 	bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
+	if (mode == GENET_POWER_PASSIVE)
+		bcmgenet_phy_power_set(priv->dev, true);
 }
 
 /* ioctl handle special commands that are not present in ethtool. */
-- 
2.1.0

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

* [PATCH net-next 4/6] net: bcmgenet: Determine PHY type before scanning MDIO bus
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
                   ` (2 preceding siblings ...)
  2015-07-16 22:51 ` [PATCH net-next 3/6] net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up() Florian Fainelli
@ 2015-07-16 22:51 ` Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 5/6] net: bcmgenet: Delay PHY initialization to bcmgenet_open() Florian Fainelli
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

Our internal GPHY might be powered off before we attempt scanning the
MDIO bus and bind a driver to it. The way we are currently determining
whether a PHY is internal or not is done *after* we have successfully
matched its driver. If the PHY is powered down, it will not respond to
the MDIO bus, so we will not be able to bind a driver to it.

Our Device Tree for GENET interfaces specifies a "phy-mode" value:
"internal" which tells if this internal uses an internal PHY or not.

If of_get_phy_mode() fails to parse the 'phy-mode' property, do an
additional manual lookup, and if we find "internal" set the
corresponding internal variable accordingly.

Replace all uses of phy_is_internal() with a check against
priv->internal_phy to avoid having to rely on whether or not
priv->phydev is set correctly.

Fixes: 1c1008c793fa4 ("net: bcmgenet: add main driver file")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 ++++++-------
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 29 +++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 2efe72f94869..076565463226 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1724,7 +1724,7 @@ static int init_umac(struct bcmgenet_priv *priv)
 	int0_enable |= UMAC_IRQ_TXDMA_DONE;
 
 	/* Monitor cable plug/unplugged event for internal PHY */
-	if (phy_is_internal(priv->phydev)) {
+	if (priv->internal_phy) {
 		int0_enable |= UMAC_IRQ_LINK_EVENT;
 	} else if (priv->ext_phy) {
 		int0_enable |= UMAC_IRQ_LINK_EVENT;
@@ -2631,7 +2631,7 @@ static int bcmgenet_open(struct net_device *dev)
 	/* If this is an internal GPHY, power it back on now, before UniMAC is
 	 * brought out of reset as absolutely no UniMAC activity is allowed
 	 */
-	if (phy_is_internal(priv->phydev))
+	if (priv->internal_phy)
 		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
 	/* take MAC out of reset */
@@ -2650,7 +2650,7 @@ static int bcmgenet_open(struct net_device *dev)
 
 	bcmgenet_set_hw_addr(priv, dev->dev_addr);
 
-	if (phy_is_internal(priv->phydev)) {
+	if (priv->internal_phy) {
 		reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 		reg |= EXT_ENERGY_DET_MASK;
 		bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
@@ -2756,7 +2756,7 @@ static int bcmgenet_close(struct net_device *dev)
 	free_irq(priv->irq0, priv);
 	free_irq(priv->irq1, priv);
 
-	if (phy_is_internal(priv->phydev))
+	if (priv->internal_phy)
 		ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
 
 	if (!IS_ERR(priv->clk))
@@ -3318,7 +3318,7 @@ static int bcmgenet_suspend(struct device *d)
 	if (device_may_wakeup(d) && priv->wolopts) {
 		ret = bcmgenet_power_down(priv, GENET_POWER_WOL_MAGIC);
 		clk_prepare_enable(priv->clk_wol);
-	} else if (phy_is_internal(priv->phydev)) {
+	} else if (priv->internal_phy) {
 		ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
 	}
 
@@ -3347,7 +3347,7 @@ static int bcmgenet_resume(struct device *d)
 	/* If this is an internal GPHY, power it back on now, before UniMAC is
 	 * brought out of reset as absolutely no UniMAC activity is allowed
 	 */
-	if (phy_is_internal(priv->phydev))
+	if (priv->internal_phy)
 		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
 	bcmgenet_umac_reset(priv);
@@ -3369,7 +3369,7 @@ static int bcmgenet_resume(struct device *d)
 
 	bcmgenet_set_hw_addr(priv, dev->dev_addr);
 
-	if (phy_is_internal(priv->phydev)) {
+	if (priv->internal_phy) {
 		reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 		reg |= EXT_ENERGY_DET_MASK;
 		bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 9f9ac0089d4d..84274de83670 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -593,6 +593,7 @@ struct bcmgenet_priv {
 	/* MDIO bus variables */
 	wait_queue_head_t wq;
 	struct phy_device *phydev;
+	bool internal_phy;
 	struct device_node *phy_dn;
 	struct device_node *mdio_dn;
 	struct mii_bus *mii_bus;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index c5f9c7b5d9e7..35df947e738c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -227,10 +227,10 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 	u32 port_ctrl;
 	u32 reg;
 
-	priv->ext_phy = !phy_is_internal(priv->phydev) &&
+	priv->ext_phy = !priv->internal_phy &&
 			(priv->phy_interface != PHY_INTERFACE_MODE_MOCA);
 
-	if (phy_is_internal(priv->phydev))
+	if (priv->internal_phy)
 		priv->phy_interface = PHY_INTERFACE_MODE_NA;
 
 	switch (priv->phy_interface) {
@@ -248,7 +248,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 
 		bcmgenet_sys_writel(priv, port_ctrl, SYS_PORT_CTRL);
 
-		if (phy_is_internal(priv->phydev)) {
+		if (priv->internal_phy) {
 			phy_name = "internal PHY";
 			bcmgenet_internal_phy_setup(dev);
 		} else if (priv->phy_interface == PHY_INTERFACE_MODE_MOCA) {
@@ -386,7 +386,7 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 	/* The internal PHY has its link interrupts routed to the
 	 * Ethernet MAC ISRs
 	 */
-	if (phy_is_internal(priv->phydev))
+	if (priv->internal_phy)
 		priv->mii_bus->irq[phydev->addr] = PHY_IGNORE_INTERRUPT;
 	else
 		priv->mii_bus->irq[phydev->addr] = PHY_POLL;
@@ -479,7 +479,9 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 {
 	struct device_node *dn = priv->pdev->dev.of_node;
 	struct device *kdev = &priv->pdev->dev;
+	const char *phy_mode_str = NULL;
 	char *compat;
+	int phy_mode;
 	int ret;
 
 	compat = kasprintf(GFP_KERNEL, "brcm,genet-mdio-v%d", priv->version);
@@ -503,7 +505,24 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 	priv->phy_dn = of_parse_phandle(dn, "phy-handle", 0);
 
 	/* Get the link mode */
-	priv->phy_interface = of_get_phy_mode(dn);
+	phy_mode = of_get_phy_mode(dn);
+	priv->phy_interface = phy_mode;
+
+	/* We need to specifically look up whether this PHY interface is internal
+	 * or not *before* we even try to probe the PHY driver over MDIO as we
+	 * may have shut down the internal PHY for power saving purposes.
+	 */
+	if (phy_mode < 0) {
+		ret = of_property_read_string(dn, "phy-mode", &phy_mode_str);
+		if (ret < 0) {
+			dev_err(kdev, "invalid PHY mode property\n");
+			return ret;
+		}
+
+		priv->phy_interface = PHY_INTERFACE_MODE_NA;
+		if (!strcasecmp(phy_mode_str, "internal"))
+			priv->internal_phy = true;
+	}
 
 	return 0;
 }
-- 
2.1.0

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

* [PATCH net-next 5/6] net: bcmgenet: Delay PHY initialization to bcmgenet_open()
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
                   ` (3 preceding siblings ...)
  2015-07-16 22:51 ` [PATCH net-next 4/6] net: bcmgenet: Determine PHY type before scanning MDIO bus Florian Fainelli
@ 2015-07-16 22:51 ` Florian Fainelli
  2015-07-16 22:51 ` [PATCH net-next 6/6] net: bcmgenet: Remove init parameter from bcmgenet_mii_config Florian Fainelli
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

We are currently doing a full PHY initialization and even starting the
pHY state machine during bcmgenet_mii_init() which is executed in the
driver's probe function. This is convenient to determine whether we can
attach to a proper PHY device but comes at the expense of spending up to
10ms per MDIO transactions (to reach the waitqueue timeout), which slows
things down.

This also creates a sitaution where we end-up attaching twice to the
PHY, which is not quite correct either.

Fix this by moving bcmgenet_mii_probe() into bcmgenet_open() and update
its error path accordingly.

Avoid printing the message "attached PHY at address 1 [...]" every time
we bring up/down the interface and remove this print since it duplicates
what the PHY driver already does for us.

Fixes: 1c1008c793fa4 ("net: bcmgenet: add main driver file")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 12 +++++----
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 37 +++++++++-----------------
 3 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 076565463226..fbab7757adfa 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2686,16 +2686,18 @@ static int bcmgenet_open(struct net_device *dev)
 		goto err_irq0;
 	}
 
-	/* Re-configure the port multiplexer towards the PHY device */
-	bcmgenet_mii_config(priv->dev, false);
-
-	phy_connect_direct(dev, priv->phydev, bcmgenet_mii_setup,
-			   priv->phy_interface);
+	ret = bcmgenet_mii_probe(dev);
+	if (ret) {
+		netdev_err(dev, "failed to connect to PHY\n");
+		goto err_irq1;
+	}
 
 	bcmgenet_netif_start(dev);
 
 	return 0;
 
+err_irq1:
+	free_irq(priv->irq1, priv);
 err_irq0:
 	free_irq(priv->irq0, priv);
 err_fini_dma:
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 84274de83670..e25b5327cc40 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -672,6 +672,7 @@ GENET_IO_MACRO(rbuf, GENET_RBUF_OFF);
 /* MDIO routines */
 int bcmgenet_mii_init(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 35df947e738c..b503897a0da3 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -316,7 +316,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 	return 0;
 }
 
-static int bcmgenet_mii_probe(struct net_device *dev)
+int bcmgenet_mii_probe(struct net_device *dev)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
 	struct device_node *dn = priv->pdev->dev.of_node;
@@ -334,22 +334,6 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 	priv->old_pause = -1;
 
 	if (dn) {
-		if (priv->phydev) {
-			pr_info("PHY already attached\n");
-			return 0;
-		}
-
-		/* In the case of a fixed PHY, the DT node associated
-		 * to the PHY is the Ethernet MAC DT node.
-		 */
-		if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
-			ret = of_phy_register_fixed_link(dn);
-			if (ret)
-				return ret;
-
-			priv->phy_dn = of_node_get(dn);
-		}
-
 		phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
 					phy_flags, priv->phy_interface);
 		if (!phydev) {
@@ -391,9 +375,6 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 	else
 		priv->mii_bus->irq[phydev->addr] = PHY_POLL;
 
-	pr_info("attached PHY at address %d [%s]\n",
-		phydev->addr, phydev->drv->name);
-
 	return 0;
 }
 
@@ -504,6 +485,17 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 	/* Fetch the PHY phandle */
 	priv->phy_dn = of_parse_phandle(dn, "phy-handle", 0);
 
+	/* In the case of a fixed PHY, the DT node associated
+	 * to the PHY is the Ethernet MAC DT node.
+	 */
+	if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
+		ret = of_phy_register_fixed_link(dn);
+		if (ret)
+			return ret;
+
+		priv->phy_dn = of_node_get(dn);
+	}
+
 	/* Get the link mode */
 	phy_mode = of_get_phy_mode(dn);
 	priv->phy_interface = phy_mode;
@@ -623,10 +615,6 @@ int bcmgenet_mii_init(struct net_device *dev)
 
 	ret = bcmgenet_mii_bus_init(priv);
 	if (ret)
-		goto out_free;
-
-	ret = bcmgenet_mii_probe(dev);
-	if (ret)
 		goto out;
 
 	return 0;
@@ -634,7 +622,6 @@ int bcmgenet_mii_init(struct net_device *dev)
 out:
 	of_node_put(priv->phy_dn);
 	mdiobus_unregister(priv->mii_bus);
-out_free:
 	kfree(priv->mii_bus->irq);
 	mdiobus_free(priv->mii_bus);
 	return ret;
-- 
2.1.0

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

* [PATCH net-next 6/6] net: bcmgenet: Remove init parameter from bcmgenet_mii_config
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
                   ` (4 preceding siblings ...)
  2015-07-16 22:51 ` [PATCH net-next 5/6] net: bcmgenet: Delay PHY initialization to bcmgenet_open() Florian Fainelli
@ 2015-07-16 22:51 ` Florian Fainelli
  2015-07-17 13:53 ` [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Jaedon Shin
  2015-07-21  3:48 ` David Miller
  7 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-16 22:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, pgynther, jaedon.shin, Florian Fainelli

Now that we have reworked the way we perform the PHY initialization, we
no longer need to differentiate between init time vs. non-init time
calls, just use a dev_info_once() print to print the PHY type.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
 drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 +-
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 7 +++----
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index fbab7757adfa..5bf7ce0ae221 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3364,7 +3364,7 @@ static int bcmgenet_resume(struct device *d)
 
 	phy_init_hw(priv->phydev);
 	/* Speed settings must be restored */
-	bcmgenet_mii_config(priv->dev, false);
+	bcmgenet_mii_config(priv->dev);
 
 	/* disable ethernet MAC while updating its registers */
 	umac_enable_set(priv, CMD_TX_EN | CMD_RX_EN, false);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index e25b5327cc40..7299d1075422 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -671,7 +671,7 @@ GENET_IO_MACRO(rbuf, GENET_RBUF_OFF);
 
 /* MDIO routines */
 int bcmgenet_mii_init(struct net_device *dev);
-int bcmgenet_mii_config(struct net_device *dev, bool init);
+int bcmgenet_mii_config(struct net_device *dev);
 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);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index b503897a0da3..0802cd9d2424 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -217,7 +217,7 @@ static void bcmgenet_moca_phy_setup(struct bcmgenet_priv *priv)
 	bcmgenet_sys_writel(priv, reg, SYS_PORT_CTRL);
 }
 
-int bcmgenet_mii_config(struct net_device *dev, bool init)
+int bcmgenet_mii_config(struct net_device *dev)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
 	struct phy_device *phydev = priv->phydev;
@@ -310,8 +310,7 @@ 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);
+	dev_info_once(kdev, "configuring instance for %s\n", phy_name);
 
 	return 0;
 }
@@ -359,7 +358,7 @@ int bcmgenet_mii_probe(struct net_device *dev)
 	 * PHY speed which is needed for bcmgenet_mii_config() to configure
 	 * things appropriately.
 	 */
-	ret = bcmgenet_mii_config(dev, true);
+	ret = bcmgenet_mii_config(dev);
 	if (ret) {
 		phy_disconnect(priv->phydev);
 		return ret;
-- 
2.1.0

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

* Re: [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
                   ` (5 preceding siblings ...)
  2015-07-16 22:51 ` [PATCH net-next 6/6] net: bcmgenet: Remove init parameter from bcmgenet_mii_config Florian Fainelli
@ 2015-07-17 13:53 ` Jaedon Shin
  2015-07-20 20:00   ` Florian Fainelli
  2015-07-21  3:48 ` David Miller
  7 siblings, 1 reply; 10+ messages in thread
From: Jaedon Shin @ 2015-07-17 13:53 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, David Miller, Petri Gynther

> On Jul 17, 2015, at 7:51 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> Hi David, Petri, Jaedon,
> 
> This patch series reworks how we perform PHY initialization and resets in the
> GENET driver. Although this contains mostly fixes, some of the changes are a
> bit too intrusive to be backported to 'net' at the moment.
> 
> Some of the motivations behind these changes were to reduce the time spent in how
> performing MDIO transactions, since it is better to perform then when we have
> interrupts enabled. This reduces the bring-up time of GENET from ~600 msecs down
> to ~8 msecs, and about the same time for suspend/resume.
> 
> Since I do not currently have a system which is not DT-aware, can you (Petri,
> Jaedon) give this a try and confirm things keep working as expected?
> 
> Thanks!
> 

I tested your patch series on Broadcom 40nm set-top box platform that used
internal phy. I did not have the exact measurements. but I expect it to improve
on the interface-up or link-up time. and I compared the changes roughly from
kernel print time. please see below.

- before patching
[    1.865126] bcmgenet 10430000.ethernet eth0: Link is Down
[    3.941132] bcmgenet 10430000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx

- after patching
[    3.145127] bcmgenet 10430000.ethernet eth0: Link is Down
[    4.189140] bcmgenet 10430000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx

> Florian Fainelli (6):
>  net: bcmgenet: Remove excessive PHY reset
>  net: bcmgenet: Use correct dev_id for free_irq
>  net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up()
>  net: bcmgenet: Determine PHY type before scanning MDIO bus
>  net: bcmgenet: Delay PHY initialization to bcmgenet_open()
>  net: bcmgenet: Remove init parameter from bcmgenet_mii_config
> 
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 33 +++++-----
> drivers/net/ethernet/broadcom/genet/bcmgenet.h |  5 +-
> drivers/net/ethernet/broadcom/genet/bcmmii.c   | 84 ++++++++++++--------------
> 3 files changed, 59 insertions(+), 63 deletions(-)
> 
> -- 
> 2.1.0
> 

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

* Re: [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework
  2015-07-17 13:53 ` [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Jaedon Shin
@ 2015-07-20 20:00   ` Florian Fainelli
  0 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2015-07-20 20:00 UTC (permalink / raw)
  To: Jaedon Shin; +Cc: netdev, David Miller, Petri Gynther

On 17/07/15 06:53, Jaedon Shin wrote:
>> On Jul 17, 2015, at 7:51 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> Hi David, Petri, Jaedon,
>>
>> This patch series reworks how we perform PHY initialization and resets in the
>> GENET driver. Although this contains mostly fixes, some of the changes are a
>> bit too intrusive to be backported to 'net' at the moment.
>>
>> Some of the motivations behind these changes were to reduce the time spent in how
>> performing MDIO transactions, since it is better to perform then when we have
>> interrupts enabled. This reduces the bring-up time of GENET from ~600 msecs down
>> to ~8 msecs, and about the same time for suspend/resume.
>>
>> Since I do not currently have a system which is not DT-aware, can you (Petri,
>> Jaedon) give this a try and confirm things keep working as expected?
>>
>> Thanks!
>>
> 
> I tested your patch series on Broadcom 40nm set-top box platform that used
> internal phy. I did not have the exact measurements. but I expect it to improve
> on the interface-up or link-up time. and I compared the changes roughly from
> kernel print time. please see below.

Thanks for testing! The patches are not really meant to improve the link
up/down time, the fact that you are seeing an improvement here is
coincidental and/or not related. The patches are meant to provide an
improvement during system resume (out of S2 and S3) and during the
initial probe of the driver.

Sounds like you tested this with a DT-enabled MIPS platform, do you have
code handy to do the same experiement without DT?

> 
> - before patching
> [    1.865126] bcmgenet 10430000.ethernet eth0: Link is Down
> [    3.941132] bcmgenet 10430000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> 
> - after patching
> [    3.145127] bcmgenet 10430000.ethernet eth0: Link is Down
> [    4.189140] bcmgenet 10430000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> 
>> Florian Fainelli (6):
>>  net: bcmgenet: Remove excessive PHY reset
>>  net: bcmgenet: Use correct dev_id for free_irq
>>  net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up()
>>  net: bcmgenet: Determine PHY type before scanning MDIO bus
>>  net: bcmgenet: Delay PHY initialization to bcmgenet_open()
>>  net: bcmgenet: Remove init parameter from bcmgenet_mii_config
>>
>> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 33 +++++-----
>> drivers/net/ethernet/broadcom/genet/bcmgenet.h |  5 +-
>> drivers/net/ethernet/broadcom/genet/bcmmii.c   | 84 ++++++++++++--------------
>> 3 files changed, 59 insertions(+), 63 deletions(-)
>>
>> -- 
>> 2.1.0
>>
> 


-- 
Florian

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

* Re: [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework
  2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
                   ` (6 preceding siblings ...)
  2015-07-17 13:53 ` [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Jaedon Shin
@ 2015-07-21  3:48 ` David Miller
  7 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2015-07-21  3:48 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, pgynther, jaedon.shin

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 16 Jul 2015 15:51:13 -0700

> This patch series reworks how we perform PHY initialization and resets in the
> GENET driver. Although this contains mostly fixes, some of the changes are a
> bit too intrusive to be backported to 'net' at the moment.
> 
> Some of the motivations behind these changes were to reduce the time spent in how
> performing MDIO transactions, since it is better to perform then when we have
> interrupts enabled. This reduces the bring-up time of GENET from ~600 msecs down
> to ~8 msecs, and about the same time for suspend/resume.
> 
> Since I do not currently have a system which is not DT-aware, can you (Petri,
> Jaedon) give this a try and confirm things keep working as expected?

Series applied, thanks Florian.

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

end of thread, other threads:[~2015-07-21  3:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-16 22:51 [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Florian Fainelli
2015-07-16 22:51 ` [PATCH net-next 1/6] net: bcmgenet: Remove excessive PHY reset Florian Fainelli
2015-07-16 22:51 ` [PATCH net-next 2/6] net: bcmgenet: Use correct dev_id for free_irq Florian Fainelli
2015-07-16 22:51 ` [PATCH net-next 3/6] net: bcmgenet: Power on integrated GPHY in bcmgenet_power_up() Florian Fainelli
2015-07-16 22:51 ` [PATCH net-next 4/6] net: bcmgenet: Determine PHY type before scanning MDIO bus Florian Fainelli
2015-07-16 22:51 ` [PATCH net-next 5/6] net: bcmgenet: Delay PHY initialization to bcmgenet_open() Florian Fainelli
2015-07-16 22:51 ` [PATCH net-next 6/6] net: bcmgenet: Remove init parameter from bcmgenet_mii_config Florian Fainelli
2015-07-17 13:53 ` [PATCH net-next 0/6] net: bcmgenet: PHY initialization rework Jaedon Shin
2015-07-20 20:00   ` Florian Fainelli
2015-07-21  3:48 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.