[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <1398857241-20137-1-git-send-email-claudiu.manoil@freescale.com>
Date: Wed, 30 Apr 2014 14:27:21 +0300
From: Claudiu Manoil <claudiu.manoil@...escale.com>
To: <netdev@...r.kernel.org>
CC: "David S. Miller" <davem@...emloft.net>
Subject: [PATCH net] gianfar: Avoid unnecessary reg accesses in adjust_link()
For phy devices that don't issue interrupts upon link
state changes, phylib polls the link state resulting in
repeated calls to adjust_link(), even if the link state
didn't change. As a result, some mac registers are
repeatedly read and written with the same values, which
is not ok.
To fix this, adjust_link() has been refactored to check
first whether the link state has changed and to take action
only if needed, updating mac registers and local state
variables. The 'new_state' local flag, set if one of the
link params changed (link, speed or duplex), has been
rendered useless and removed by this refactoring.
Signed-off-by: Claudiu Manoil <claudiu.manoil@...escale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 223 ++++++++++++++++---------------
1 file changed, 113 insertions(+), 110 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 9125d9a..e2d4247 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -121,6 +121,7 @@ static irqreturn_t gfar_error(int irq, void *dev_id);
static irqreturn_t gfar_transmit(int irq, void *dev_id);
static irqreturn_t gfar_interrupt(int irq, void *dev_id);
static void adjust_link(struct net_device *dev);
+static noinline void gfar_update_link_state(struct gfar_private *priv);
static int init_phy(struct net_device *dev);
static int gfar_probe(struct platform_device *ofdev);
static int gfar_remove(struct platform_device *ofdev);
@@ -3076,41 +3077,6 @@ static irqreturn_t gfar_interrupt(int irq, void *grp_id)
return IRQ_HANDLED;
}
-static u32 gfar_get_flowctrl_cfg(struct gfar_private *priv)
-{
- struct phy_device *phydev = priv->phydev;
- u32 val = 0;
-
- if (!phydev->duplex)
- return val;
-
- if (!priv->pause_aneg_en) {
- if (priv->tx_pause_en)
- val |= MACCFG1_TX_FLOW;
- if (priv->rx_pause_en)
- val |= MACCFG1_RX_FLOW;
- } else {
- u16 lcl_adv, rmt_adv;
- u8 flowctrl;
- /* get link partner capabilities */
- rmt_adv = 0;
- if (phydev->pause)
- rmt_adv = LPA_PAUSE_CAP;
- if (phydev->asym_pause)
- rmt_adv |= LPA_PAUSE_ASYM;
-
- lcl_adv = mii_advertise_flowctrl(phydev->advertising);
-
- flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
- if (flowctrl & FLOW_CTRL_TX)
- val |= MACCFG1_TX_FLOW;
- if (flowctrl & FLOW_CTRL_RX)
- val |= MACCFG1_RX_FLOW;
- }
-
- return val;
-}
-
/* Called every time the controller might need to be made
* aware of new link state. The PHY code conveys this
* information through variables in the phydev structure, and this
@@ -3120,83 +3086,12 @@ static u32 gfar_get_flowctrl_cfg(struct gfar_private *priv)
static void adjust_link(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
- struct gfar __iomem *regs = priv->gfargrp[0].regs;
struct phy_device *phydev = priv->phydev;
- int new_state = 0;
- if (test_bit(GFAR_RESETTING, &priv->state))
- return;
-
- if (phydev->link) {
- u32 tempval1 = gfar_read(®s->maccfg1);
- u32 tempval = gfar_read(®s->maccfg2);
- u32 ecntrl = gfar_read(®s->ecntrl);
-
- /* Now we make sure that we can be in full duplex mode.
- * If not, we operate in half-duplex mode.
- */
- if (phydev->duplex != priv->oldduplex) {
- new_state = 1;
- if (!(phydev->duplex))
- tempval &= ~(MACCFG2_FULL_DUPLEX);
- else
- tempval |= MACCFG2_FULL_DUPLEX;
-
- priv->oldduplex = phydev->duplex;
- }
-
- if (phydev->speed != priv->oldspeed) {
- new_state = 1;
- switch (phydev->speed) {
- case 1000:
- tempval =
- ((tempval & ~(MACCFG2_IF)) | MACCFG2_GMII);
-
- ecntrl &= ~(ECNTRL_R100);
- break;
- case 100:
- case 10:
- tempval =
- ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII);
-
- /* Reduced mode distinguishes
- * between 10 and 100
- */
- if (phydev->speed == SPEED_100)
- ecntrl |= ECNTRL_R100;
- else
- ecntrl &= ~(ECNTRL_R100);
- break;
- default:
- netif_warn(priv, link, dev,
- "Ack! Speed (%d) is not 10/100/1000!\n",
- phydev->speed);
- break;
- }
-
- priv->oldspeed = phydev->speed;
- }
-
- tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW);
- tempval1 |= gfar_get_flowctrl_cfg(priv);
-
- gfar_write(®s->maccfg1, tempval1);
- gfar_write(®s->maccfg2, tempval);
- gfar_write(®s->ecntrl, ecntrl);
-
- if (!priv->oldlink) {
- new_state = 1;
- priv->oldlink = 1;
- }
- } else if (priv->oldlink) {
- new_state = 1;
- priv->oldlink = 0;
- priv->oldspeed = 0;
- priv->oldduplex = -1;
- }
-
- if (new_state && netif_msg_link(priv))
- phy_print_status(phydev);
+ if (unlikely(phydev->link != priv->oldlink ||
+ phydev->duplex != priv->oldduplex ||
+ phydev->speed != priv->oldspeed))
+ gfar_update_link_state(priv);
}
/* Update the hash table based on the current list of multicast
@@ -3442,6 +3337,114 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
return IRQ_HANDLED;
}
+static u32 gfar_get_flowctrl_cfg(struct gfar_private *priv)
+{
+ struct phy_device *phydev = priv->phydev;
+ u32 val = 0;
+
+ if (!phydev->duplex)
+ return val;
+
+ if (!priv->pause_aneg_en) {
+ if (priv->tx_pause_en)
+ val |= MACCFG1_TX_FLOW;
+ if (priv->rx_pause_en)
+ val |= MACCFG1_RX_FLOW;
+ } else {
+ u16 lcl_adv, rmt_adv;
+ u8 flowctrl;
+ /* get link partner capabilities */
+ rmt_adv = 0;
+ if (phydev->pause)
+ rmt_adv = LPA_PAUSE_CAP;
+ if (phydev->asym_pause)
+ rmt_adv |= LPA_PAUSE_ASYM;
+
+ lcl_adv = mii_advertise_flowctrl(phydev->advertising);
+
+ flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
+ if (flowctrl & FLOW_CTRL_TX)
+ val |= MACCFG1_TX_FLOW;
+ if (flowctrl & FLOW_CTRL_RX)
+ val |= MACCFG1_RX_FLOW;
+ }
+
+ return val;
+}
+
+static noinline void gfar_update_link_state(struct gfar_private *priv)
+{
+ struct gfar __iomem *regs = priv->gfargrp[0].regs;
+ struct phy_device *phydev = priv->phydev;
+
+ if (unlikely(test_bit(GFAR_RESETTING, &priv->state)))
+ return;
+
+ if (phydev->link) {
+ u32 tempval1 = gfar_read(®s->maccfg1);
+ u32 tempval = gfar_read(®s->maccfg2);
+ u32 ecntrl = gfar_read(®s->ecntrl);
+
+ if (phydev->duplex != priv->oldduplex) {
+ if (!(phydev->duplex))
+ tempval &= ~(MACCFG2_FULL_DUPLEX);
+ else
+ tempval |= MACCFG2_FULL_DUPLEX;
+
+ priv->oldduplex = phydev->duplex;
+ }
+
+ if (phydev->speed != priv->oldspeed) {
+ switch (phydev->speed) {
+ case 1000:
+ tempval =
+ ((tempval & ~(MACCFG2_IF)) | MACCFG2_GMII);
+
+ ecntrl &= ~(ECNTRL_R100);
+ break;
+ case 100:
+ case 10:
+ tempval =
+ ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII);
+
+ /* Reduced mode distinguishes
+ * between 10 and 100
+ */
+ if (phydev->speed == SPEED_100)
+ ecntrl |= ECNTRL_R100;
+ else
+ ecntrl &= ~(ECNTRL_R100);
+ break;
+ default:
+ netif_warn(priv, link, priv->ndev,
+ "Ack! Speed (%d) is not 10/100/1000!\n",
+ phydev->speed);
+ break;
+ }
+
+ priv->oldspeed = phydev->speed;
+ }
+
+ tempval1 &= ~(MACCFG1_TX_FLOW | MACCFG1_RX_FLOW);
+ tempval1 |= gfar_get_flowctrl_cfg(priv);
+
+ gfar_write(®s->maccfg1, tempval1);
+ gfar_write(®s->maccfg2, tempval);
+ gfar_write(®s->ecntrl, ecntrl);
+
+ if (!priv->oldlink)
+ priv->oldlink = 1;
+
+ } else if (priv->oldlink) {
+ priv->oldlink = 0;
+ priv->oldspeed = 0;
+ priv->oldduplex = -1;
+ }
+
+ if (netif_msg_link(priv))
+ phy_print_status(phydev);
+}
+
static struct of_device_id gfar_match[] =
{
{
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists