* [PATCH 1/2] net-next: stmmac: add adjust_link function
@ 2017-05-15 11:41 Corentin Labbe
2017-05-15 11:41 ` [PATCH 2/2] net-next: stmmac: remove struct mac_link Corentin Labbe
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Corentin Labbe @ 2017-05-15 11:41 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
My dwmac-sun8i serie will add some if (has_sun8i) to
stmmac_adjust_link()
Since the current stmmac_adjust_link() alreaady have lots of if (has_gmac/gmac4),
It is now better to create an adjust_link() function for each dwmac.
So this patch add an adjust_link() function pointer, and move code out
of stmmac_adjust_link to it.
Removing in the process stmmac_mac_flow_ctrl/stmmac_hw_fix_mac_speed
since there not used anymore.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 3 +
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 54 ++++++++++++++
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 46 ++++++++++++
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 54 ++++++++++++++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 83 +---------------------
5 files changed, 158 insertions(+), 82 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index b7ce3fbb5375..451c231006fe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -469,11 +469,14 @@ struct stmmac_dma_ops {
};
struct mac_device_info;
+struct stmmac_priv;
/* Helpers to program the MAC core */
struct stmmac_ops {
/* MAC core initialization */
void (*core_init)(struct mac_device_info *hw, int mtu);
+ /* adjust link */
+ int (*adjust_link)(struct stmmac_priv *priv);
/* Enable the MAC RX/TX */
void (*set_mac)(void __iomem *ioaddr, bool enable);
/* Enable and verify that the IPC module is supported */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index f3d9305e5f70..5f3aace46c41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -26,6 +26,7 @@
#include <linux/slab.h>
#include <linux/ethtool.h>
#include <asm/io.h>
+#include "stmmac.h"
#include "stmmac_pcs.h"
#include "dwmac1000.h"
@@ -75,6 +76,58 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
#endif
}
+static int dwmac1000_adjust_link(struct stmmac_priv *priv)
+{
+ struct net_device *ndev = priv->dev;
+ struct phy_device *phydev = ndev->phydev;
+ int new_state = 0;
+ u32 tx_cnt = priv->plat->tx_queues_to_use;
+ u32 ctrl;
+
+ ctrl = readl(priv->ioaddr + GMAC_CONTROL);
+
+ if (phydev->duplex != priv->oldduplex) {
+ new_state = 1;
+ if (!(phydev->duplex))
+ ctrl &= ~GMAC_CONTROL_DM;
+ else
+ ctrl |= GMAC_CONTROL_DM;
+ priv->oldduplex = phydev->duplex;
+ }
+
+ if (phydev->pause)
+ priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
+ priv->pause, tx_cnt);
+
+ if (phydev->speed != priv->speed) {
+ new_state = 1;
+ switch (phydev->speed) {
+ case 1000:
+ ctrl &= ~GMAC_CONTROL_PS;
+ break;
+ case 100:
+ ctrl |= GMAC_CONTROL_PS;
+ ctrl |= GMAC_CONTROL_FES;
+ break;
+ case 10:
+ ctrl |= GMAC_CONTROL_PS;
+ ctrl |= ~GMAC_CONTROL_FES;
+ break;
+ default:
+ netif_warn(priv, link, priv->dev,
+ "broken speed: %d\n", phydev->speed);
+ phydev->speed = SPEED_UNKNOWN;
+ break;
+ }
+ if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
+ priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
+ priv->speed = phydev->speed;
+ }
+
+ writel(ctrl, priv->ioaddr + GMAC_CONTROL);
+ return new_state;
+}
+
static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw)
{
void __iomem *ioaddr = hw->pcsr;
@@ -490,6 +543,7 @@ static void dwmac1000_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
static const struct stmmac_ops dwmac1000_ops = {
.core_init = dwmac1000_core_init,
+ .adjust_link = dwmac1000_adjust_link,
.set_mac = stmmac_set_mac,
.rx_ipc = dwmac1000_rx_ipc_enable,
.dump_regs = dwmac1000_dump_regs,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index 1b3609105484..ba3d46e65e1a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -27,6 +27,7 @@
#include <linux/crc32.h>
#include <asm/io.h>
#include "dwmac100.h"
+#include "stmmac.h"
static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
{
@@ -40,6 +41,50 @@ static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
#endif
}
+static int dwmac100_adjust_link(struct stmmac_priv *priv)
+{
+ struct net_device *ndev = priv->dev;
+ struct phy_device *phydev = ndev->phydev;
+ int new_state = 0;
+ u32 tx_cnt = priv->plat->tx_queues_to_use;
+ u32 ctrl;
+
+ ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
+ if (phydev->duplex != priv->oldduplex) {
+ new_state = 1;
+ if (!(phydev->duplex))
+ ctrl &= ~MAC_CONTROL_F;
+ else
+ ctrl |= MAC_CONTROL_F;
+ priv->oldduplex = phydev->duplex;
+ }
+
+ if (phydev->pause)
+ priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
+ priv->pause, tx_cnt);
+
+ if (phydev->speed != priv->speed) {
+ new_state = 1;
+ switch (phydev->speed) {
+ case 100:
+ case 10:
+ ctrl &= ~MAC_CONTROL_PS;
+ break;
+ default:
+ netif_warn(priv, link, priv->dev,
+ "broken speed: %d\n", phydev->speed);
+ phydev->speed = SPEED_UNKNOWN;
+ break;
+ }
+ if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
+ priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
+ priv->speed = phydev->speed;
+ }
+
+ writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
+ return new_state;
+}
+
static void dwmac100_dump_mac_regs(struct mac_device_info *hw, u32 *reg_space)
{
void __iomem *ioaddr = hw->pcsr;
@@ -150,6 +195,7 @@ static void dwmac100_pmt(struct mac_device_info *hw, unsigned long mode)
static const struct stmmac_ops dwmac100_ops = {
.core_init = dwmac100_core_init,
+ .adjust_link = dwmac100_adjust_link,
.set_mac = stmmac_set_mac,
.rx_ipc = dwmac100_rx_ipc_enable,
.dump_regs = dwmac100_dump_mac_regs,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 48793f2e9307..133b6bcd7b61 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -19,6 +19,7 @@
#include <linux/io.h>
#include "stmmac_pcs.h"
#include "dwmac4.h"
+#include "stmmac.h"
static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
{
@@ -59,6 +60,58 @@ static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
writel(value, ioaddr + GMAC_INT_EN);
}
+static int dwmac4_adjust_link(struct stmmac_priv *priv)
+{
+ struct net_device *ndev = priv->dev;
+ struct phy_device *phydev = ndev->phydev;
+ int new_state = 0;
+ u32 tx_cnt = priv->plat->tx_queues_to_use;
+ u32 ctrl;
+
+ ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
+
+ if (phydev->duplex != priv->oldduplex) {
+ new_state = 1;
+ if (!(phydev->duplex))
+ ctrl &= ~GMAC_CONFIG_DM;
+ else
+ ctrl |= GMAC_CONFIG_DM;
+ priv->oldduplex = phydev->duplex;
+ }
+
+ if (phydev->pause)
+ priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
+ priv->pause, tx_cnt);
+
+ if (phydev->speed != priv->speed) {
+ new_state = 1;
+ switch (phydev->speed) {
+ case 1000:
+ ctrl &= ~GMAC_CONFIG_PS;
+ break;
+ case 100:
+ ctrl |= GMAC_CONFIG_PS;
+ ctrl |= GMAC_CONFIG_FES;
+ break;
+ case 10:
+ ctrl |= GMAC_CONFIG_PS;
+ ctrl |= ~GMAC_CONFIG_FES;
+ break;
+ default:
+ netif_warn(priv, link, priv->dev,
+ "broken speed: %d\n", phydev->speed);
+ phydev->speed = SPEED_UNKNOWN;
+ break;
+ }
+ if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
+ priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
+ priv->speed = phydev->speed;
+ }
+
+ writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
+ return new_state;
+}
+
static void dwmac4_rx_queue_enable(struct mac_device_info *hw,
u8 mode, u32 queue)
{
@@ -669,6 +722,7 @@ static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
static const struct stmmac_ops dwmac4_ops = {
.core_init = dwmac4_core_init,
+ .adjust_link = dwmac4_adjust_link,
.set_mac = stmmac_set_mac,
.rx_ipc = dwmac4_rx_ipc_enable,
.rx_queue_enable = dwmac4_rx_queue_enable,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b05a042cf2c6..fb3e2ddaa7c9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -286,21 +286,6 @@ static inline u32 stmmac_rx_dirty(struct stmmac_priv *priv, u32 queue)
}
/**
- * stmmac_hw_fix_mac_speed - callback for speed selection
- * @priv: driver private structure
- * Description: on some platforms (e.g. ST), some HW system configuration
- * registers have to be set according to the link speed negotiated.
- */
-static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
-{
- struct net_device *ndev = priv->dev;
- struct phy_device *phydev = ndev->phydev;
-
- if (likely(priv->plat->fix_mac_speed))
- priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
-}
-
-/**
* stmmac_enable_eee_mode - check and enter in LPI mode
* @priv: driver private structure
* Description: this function is to verify and enter in LPI mode in case of
@@ -759,19 +744,6 @@ static void stmmac_release_ptp(struct stmmac_priv *priv)
}
/**
- * stmmac_mac_flow_ctrl - Configure flow control in all queues
- * @priv: driver private structure
- * Description: It is used for configuring the flow control in all queues
- */
-static void stmmac_mac_flow_ctrl(struct stmmac_priv *priv, u32 duplex)
-{
- u32 tx_cnt = priv->plat->tx_queues_to_use;
-
- priv->hw->mac->flow_ctrl(priv->hw, duplex, priv->flow_ctrl,
- priv->pause, tx_cnt);
-}
-
-/**
* stmmac_adjust_link - adjusts the link parameters
* @dev: net device structure
* Description: this is the helper called by the physical abstraction layer
@@ -793,60 +765,7 @@ static void stmmac_adjust_link(struct net_device *dev)
spin_lock_irqsave(&priv->lock, flags);
if (phydev->link) {
- u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
-
- /* 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))
- ctrl &= ~priv->hw->link.duplex;
- else
- ctrl |= priv->hw->link.duplex;
- priv->oldduplex = phydev->duplex;
- }
- /* Flow Control operation */
- if (phydev->pause)
- stmmac_mac_flow_ctrl(priv, phydev->duplex);
-
- if (phydev->speed != priv->speed) {
- new_state = 1;
- switch (phydev->speed) {
- case 1000:
- if (priv->plat->has_gmac ||
- priv->plat->has_gmac4)
- ctrl &= ~priv->hw->link.port;
- break;
- case 100:
- if (priv->plat->has_gmac ||
- priv->plat->has_gmac4) {
- ctrl |= priv->hw->link.port;
- ctrl |= priv->hw->link.speed;
- } else {
- ctrl &= ~priv->hw->link.port;
- }
- break;
- case 10:
- if (priv->plat->has_gmac ||
- priv->plat->has_gmac4) {
- ctrl |= priv->hw->link.port;
- ctrl &= ~(priv->hw->link.speed);
- } else {
- ctrl &= ~priv->hw->link.port;
- }
- break;
- default:
- netif_warn(priv, link, priv->dev,
- "broken speed: %d\n", phydev->speed);
- phydev->speed = SPEED_UNKNOWN;
- break;
- }
- if (phydev->speed != SPEED_UNKNOWN)
- stmmac_hw_fix_mac_speed(priv);
- priv->speed = phydev->speed;
- }
-
- writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
+ new_state = priv->hw->mac->adjust_link(priv);
if (!priv->oldlink) {
new_state = 1;
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] net-next: stmmac: remove struct mac_link
2017-05-15 11:41 [PATCH 1/2] net-next: stmmac: add adjust_link function Corentin Labbe
@ 2017-05-15 11:41 ` Corentin Labbe
2017-05-16 16:38 ` [PATCH 1/2] net-next: stmmac: add adjust_link function David Miller
2017-05-16 16:43 ` Florian Fainelli
2 siblings, 0 replies; 6+ messages in thread
From: Corentin Labbe @ 2017-05-15 11:41 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
With the usage of adjust_link(), the struct mac_link is now useless.
This patch remove it.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 7 -------
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 3 ---
drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 3 ---
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 3 ---
4 files changed, 16 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 451c231006fe..63350b70e10f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -551,12 +551,6 @@ struct stmmac_hwtimestamp {
extern const struct stmmac_hwtimestamp stmmac_ptp;
extern const struct stmmac_mode_ops dwmac4_ring_mode_ops;
-struct mac_link {
- int port;
- int duplex;
- int speed;
-};
-
struct mii_regs {
unsigned int addr; /* MII Address */
unsigned int data; /* MII Data */
@@ -587,7 +581,6 @@ struct mac_device_info {
const struct stmmac_mode_ops *mode;
const struct stmmac_hwtimestamp *ptp;
struct mii_regs mii; /* MII register Addresses */
- struct mac_link link;
void __iomem *pcsr; /* vpointer to device CSRs */
int multicast_filter_bins;
int unicast_filter_entries;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 5f3aace46c41..52092ec5f4af 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -585,9 +585,6 @@ struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr, int mcbins,
mac->mac = &dwmac1000_ops;
mac->dma = &dwmac1000_dma_ops;
- mac->link.port = GMAC_CONTROL_PS;
- mac->link.duplex = GMAC_CONTROL_DM;
- mac->link.speed = GMAC_CONTROL_FES;
mac->mii.addr = GMAC_MII_ADDR;
mac->mii.data = GMAC_MII_DATA;
mac->mii.addr_shift = 11;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index ba3d46e65e1a..faddbf3c2916 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -221,9 +221,6 @@ struct mac_device_info *dwmac100_setup(void __iomem *ioaddr, int *synopsys_id)
mac->mac = &dwmac100_ops;
mac->dma = &dwmac100_dma_ops;
- mac->link.port = MAC_CONTROL_PS;
- mac->link.duplex = MAC_CONTROL_F;
- mac->link.speed = 0;
mac->mii.addr = MAC_MII_ADDR;
mac->mii.data = MAC_MII_DATA;
mac->mii.addr_shift = 11;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 133b6bcd7b61..baf32c91122d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -801,9 +801,6 @@ struct mac_device_info *dwmac4_setup(void __iomem *ioaddr, int mcbins,
if (mac->multicast_filter_bins)
mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
- mac->link.port = GMAC_CONFIG_PS;
- mac->link.duplex = GMAC_CONFIG_DM;
- mac->link.speed = GMAC_CONFIG_FES;
mac->mii.addr = GMAC_MDIO_ADDR;
mac->mii.data = GMAC_MDIO_DATA;
mac->mii.addr_shift = 21;
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net-next: stmmac: add adjust_link function
2017-05-15 11:41 [PATCH 1/2] net-next: stmmac: add adjust_link function Corentin Labbe
2017-05-15 11:41 ` [PATCH 2/2] net-next: stmmac: remove struct mac_link Corentin Labbe
@ 2017-05-16 16:38 ` David Miller
2017-05-16 16:43 ` Florian Fainelli
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-05-16 16:38 UTC (permalink / raw)
To: clabbe.montjoie; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Mon, 15 May 2017 13:41:39 +0200
> My dwmac-sun8i serie will add some if (has_sun8i) to
> stmmac_adjust_link()
> Since the current stmmac_adjust_link() alreaady have lots of if (has_gmac/gmac4),
> It is now better to create an adjust_link() function for each dwmac.
>
> So this patch add an adjust_link() function pointer, and move code out
> of stmmac_adjust_link to it.
>
> Removing in the process stmmac_mac_flow_ctrl/stmmac_hw_fix_mac_speed
> since there not used anymore.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
This will introduce crashes because you only added the adjust_link
method to one of the two stmmac_ops instances in
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
Furthermore, a patch series should always have a header posting
with subject of the format "[PATCH 0/N]: xxx" which describes at
a high level what the patch series does as a whole, how it is doing
it, and why it is doing it that way.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net-next: stmmac: add adjust_link function
2017-05-15 11:41 [PATCH 1/2] net-next: stmmac: add adjust_link function Corentin Labbe
2017-05-15 11:41 ` [PATCH 2/2] net-next: stmmac: remove struct mac_link Corentin Labbe
2017-05-16 16:38 ` [PATCH 1/2] net-next: stmmac: add adjust_link function David Miller
@ 2017-05-16 16:43 ` Florian Fainelli
2017-05-16 19:04 ` Corentin Labbe
2017-05-17 8:21 ` Corentin Labbe
2 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-05-16 16:43 UTC (permalink / raw)
To: Corentin Labbe, peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel
On 05/15/2017 04:41 AM, Corentin Labbe wrote:
> My dwmac-sun8i serie will add some if (has_sun8i) to
> stmmac_adjust_link()
> Since the current stmmac_adjust_link() alreaady have lots of if (has_gmac/gmac4),
> It is now better to create an adjust_link() function for each dwmac.
Is it really, because the diffstat really seems to indicate otherwise
and by looking at the code, I am definitively not convinced this is an
improvement other the current code.
>
> So this patch add an adjust_link() function pointer, and move code out
> of stmmac_adjust_link to it.
Can't we keep the existing adjust_link() function and just have a
different one for dwmac-sun8i that either re-uses portions of the
existing, or duplicate just what it needs?
>
> Removing in the process stmmac_mac_flow_ctrl/stmmac_hw_fix_mac_speed
> since there not used anymore.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/common.h | 3 +
> .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 54 ++++++++++++++
> .../net/ethernet/stmicro/stmmac/dwmac100_core.c | 46 ++++++++++++
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 54 ++++++++++++++
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 83 +---------------------
> 5 files changed, 158 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index b7ce3fbb5375..451c231006fe 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -469,11 +469,14 @@ struct stmmac_dma_ops {
> };
>
> struct mac_device_info;
> +struct stmmac_priv;
>
> /* Helpers to program the MAC core */
> struct stmmac_ops {
> /* MAC core initialization */
> void (*core_init)(struct mac_device_info *hw, int mtu);
> + /* adjust link */
> + int (*adjust_link)(struct stmmac_priv *priv);
> /* Enable the MAC RX/TX */
> void (*set_mac)(void __iomem *ioaddr, bool enable);
> /* Enable and verify that the IPC module is supported */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index f3d9305e5f70..5f3aace46c41 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -26,6 +26,7 @@
> #include <linux/slab.h>
> #include <linux/ethtool.h>
> #include <asm/io.h>
> +#include "stmmac.h"
> #include "stmmac_pcs.h"
> #include "dwmac1000.h"
>
> @@ -75,6 +76,58 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
> #endif
> }
>
> +static int dwmac1000_adjust_link(struct stmmac_priv *priv)
> +{
> + struct net_device *ndev = priv->dev;
> + struct phy_device *phydev = ndev->phydev;
> + int new_state = 0;
> + u32 tx_cnt = priv->plat->tx_queues_to_use;
> + u32 ctrl;
Reverse christmas tree declaration please.
> +
> + ctrl = readl(priv->ioaddr + GMAC_CONTROL);
> +
> + if (phydev->duplex != priv->oldduplex) {
> + new_state = 1;
bool new_state
> + if (!(phydev->duplex))
Parenthesis not needed (this is an integer, not a bitmask)
> + ctrl &= ~GMAC_CONTROL_DM;
> + else
> + ctrl |= GMAC_CONTROL_DM;
> + priv->oldduplex = phydev->duplex;
> + }
> +
> + if (phydev->pause)
> + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
> + priv->pause, tx_cnt);
> +
> + if (phydev->speed != priv->speed) {
> + new_state = 1;
> + switch (phydev->speed) {
> + case 1000:
case SPEED_1000 and so on?
> + ctrl &= ~GMAC_CONTROL_PS;
> + break;
> + case 100:
> + ctrl |= GMAC_CONTROL_PS;
> + ctrl |= GMAC_CONTROL_FES;
> + break;
> + case 10:
> + ctrl |= GMAC_CONTROL_PS;
> + ctrl |= ~GMAC_CONTROL_FES;
> + break;
> + default:
> + netif_warn(priv, link, priv->dev,
> + "broken speed: %d\n", phydev->speed);
> + phydev->speed = SPEED_UNKNOWN;
> + break;
> + }
> + if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
> + priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> + priv->speed = phydev->speed;
> + }
> +
> + writel(ctrl, priv->ioaddr + GMAC_CONTROL);
> + return new_state;
> +}
> +
> static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw)
> {
> void __iomem *ioaddr = hw->pcsr;
> @@ -490,6 +543,7 @@ static void dwmac1000_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
>
> static const struct stmmac_ops dwmac1000_ops = {
> .core_init = dwmac1000_core_init,
> + .adjust_link = dwmac1000_adjust_link,
> .set_mac = stmmac_set_mac,
> .rx_ipc = dwmac1000_rx_ipc_enable,
> .dump_regs = dwmac1000_dump_regs,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> index 1b3609105484..ba3d46e65e1a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> @@ -27,6 +27,7 @@
> #include <linux/crc32.h>
> #include <asm/io.h>
> #include "dwmac100.h"
> +#include "stmmac.h"
>
> static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
> {
> @@ -40,6 +41,50 @@ static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
> #endif
> }
>
> +static int dwmac100_adjust_link(struct stmmac_priv *priv)
> +{
> + struct net_device *ndev = priv->dev;
> + struct phy_device *phydev = ndev->phydev;
> + int new_state = 0;
> + u32 tx_cnt = priv->plat->tx_queues_to_use;
> + u32 ctrl;
> +
> + ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
> + if (phydev->duplex != priv->oldduplex) {
> + new_state = 1;
> + if (!(phydev->duplex))
> + ctrl &= ~MAC_CONTROL_F;
> + else
> + ctrl |= MAC_CONTROL_F;
> + priv->oldduplex = phydev->duplex;
> + }
> +
> + if (phydev->pause)
> + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
> + priv->pause, tx_cnt);
> +
> + if (phydev->speed != priv->speed) {
> + new_state = 1;
> + switch (phydev->speed) {
> + case 100:
> + case 10:
> + ctrl &= ~MAC_CONTROL_PS;
> + break;
> + default:
> + netif_warn(priv, link, priv->dev,
> + "broken speed: %d\n", phydev->speed);
> + phydev->speed = SPEED_UNKNOWN;
> + break;
> + }
> + if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
> + priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> + priv->speed = phydev->speed;
> + }
> +
> + writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> + return new_state;
> +}
> +
> static void dwmac100_dump_mac_regs(struct mac_device_info *hw, u32 *reg_space)
> {
> void __iomem *ioaddr = hw->pcsr;
> @@ -150,6 +195,7 @@ static void dwmac100_pmt(struct mac_device_info *hw, unsigned long mode)
>
> static const struct stmmac_ops dwmac100_ops = {
> .core_init = dwmac100_core_init,
> + .adjust_link = dwmac100_adjust_link,
> .set_mac = stmmac_set_mac,
> .rx_ipc = dwmac100_rx_ipc_enable,
> .dump_regs = dwmac100_dump_mac_regs,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> index 48793f2e9307..133b6bcd7b61 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> @@ -19,6 +19,7 @@
> #include <linux/io.h>
> #include "stmmac_pcs.h"
> #include "dwmac4.h"
> +#include "stmmac.h"
>
> static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
> {
> @@ -59,6 +60,58 @@ static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
> writel(value, ioaddr + GMAC_INT_EN);
> }
>
> +static int dwmac4_adjust_link(struct stmmac_priv *priv)
> +{
> + struct net_device *ndev = priv->dev;
> + struct phy_device *phydev = ndev->phydev;
> + int new_state = 0;
> + u32 tx_cnt = priv->plat->tx_queues_to_use;
> + u32 ctrl;
> +
> + ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
> +
> + if (phydev->duplex != priv->oldduplex) {
> + new_state = 1;
> + if (!(phydev->duplex))
> + ctrl &= ~GMAC_CONFIG_DM;
> + else
> + ctrl |= GMAC_CONFIG_DM;
> + priv->oldduplex = phydev->duplex;
> + }
> +
> + if (phydev->pause)
> + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
> + priv->pause, tx_cnt);
> +
> + if (phydev->speed != priv->speed) {
> + new_state = 1;
> + switch (phydev->speed) {
> + case 1000:
> + ctrl &= ~GMAC_CONFIG_PS;
> + break;
> + case 100:
> + ctrl |= GMAC_CONFIG_PS;
> + ctrl |= GMAC_CONFIG_FES;
> + break;
> + case 10:
> + ctrl |= GMAC_CONFIG_PS;
> + ctrl |= ~GMAC_CONFIG_FES;
> + break;
> + default:
> + netif_warn(priv, link, priv->dev,
> + "broken speed: %d\n", phydev->speed);
> + phydev->speed = SPEED_UNKNOWN;
> + break;
> + }
> + if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
> + priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> + priv->speed = phydev->speed;
> + }
> +
> + writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> + return new_state;
> +}
> +
> static void dwmac4_rx_queue_enable(struct mac_device_info *hw,
> u8 mode, u32 queue)
> {
> @@ -669,6 +722,7 @@ static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
>
> static const struct stmmac_ops dwmac4_ops = {
> .core_init = dwmac4_core_init,
> + .adjust_link = dwmac4_adjust_link,
> .set_mac = stmmac_set_mac,
> .rx_ipc = dwmac4_rx_ipc_enable,
> .rx_queue_enable = dwmac4_rx_queue_enable,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b05a042cf2c6..fb3e2ddaa7c9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -286,21 +286,6 @@ static inline u32 stmmac_rx_dirty(struct stmmac_priv *priv, u32 queue)
> }
>
> /**
> - * stmmac_hw_fix_mac_speed - callback for speed selection
> - * @priv: driver private structure
> - * Description: on some platforms (e.g. ST), some HW system configuration
> - * registers have to be set according to the link speed negotiated.
> - */
> -static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
> -{
> - struct net_device *ndev = priv->dev;
> - struct phy_device *phydev = ndev->phydev;
> -
> - if (likely(priv->plat->fix_mac_speed))
> - priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> -}
> -
> -/**
> * stmmac_enable_eee_mode - check and enter in LPI mode
> * @priv: driver private structure
> * Description: this function is to verify and enter in LPI mode in case of
> @@ -759,19 +744,6 @@ static void stmmac_release_ptp(struct stmmac_priv *priv)
> }
>
> /**
> - * stmmac_mac_flow_ctrl - Configure flow control in all queues
> - * @priv: driver private structure
> - * Description: It is used for configuring the flow control in all queues
> - */
> -static void stmmac_mac_flow_ctrl(struct stmmac_priv *priv, u32 duplex)
> -{
> - u32 tx_cnt = priv->plat->tx_queues_to_use;
> -
> - priv->hw->mac->flow_ctrl(priv->hw, duplex, priv->flow_ctrl,
> - priv->pause, tx_cnt);
> -}
> -
> -/**
> * stmmac_adjust_link - adjusts the link parameters
> * @dev: net device structure
> * Description: this is the helper called by the physical abstraction layer
> @@ -793,60 +765,7 @@ static void stmmac_adjust_link(struct net_device *dev)
> spin_lock_irqsave(&priv->lock, flags);
>
> if (phydev->link) {
> - u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
> -
> - /* 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))
> - ctrl &= ~priv->hw->link.duplex;
> - else
> - ctrl |= priv->hw->link.duplex;
> - priv->oldduplex = phydev->duplex;
> - }
> - /* Flow Control operation */
> - if (phydev->pause)
> - stmmac_mac_flow_ctrl(priv, phydev->duplex);
> -
> - if (phydev->speed != priv->speed) {
> - new_state = 1;
> - switch (phydev->speed) {
> - case 1000:
> - if (priv->plat->has_gmac ||
> - priv->plat->has_gmac4)
> - ctrl &= ~priv->hw->link.port;
> - break;
> - case 100:
> - if (priv->plat->has_gmac ||
> - priv->plat->has_gmac4) {
> - ctrl |= priv->hw->link.port;
> - ctrl |= priv->hw->link.speed;
> - } else {
> - ctrl &= ~priv->hw->link.port;
> - }
> - break;
> - case 10:
> - if (priv->plat->has_gmac ||
> - priv->plat->has_gmac4) {
> - ctrl |= priv->hw->link.port;
> - ctrl &= ~(priv->hw->link.speed);
> - } else {
> - ctrl &= ~priv->hw->link.port;
> - }
> - break;
> - default:
> - netif_warn(priv, link, priv->dev,
> - "broken speed: %d\n", phydev->speed);
> - phydev->speed = SPEED_UNKNOWN;
> - break;
> - }
> - if (phydev->speed != SPEED_UNKNOWN)
> - stmmac_hw_fix_mac_speed(priv);
> - priv->speed = phydev->speed;
> - }
> -
> - writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> + new_state = priv->hw->mac->adjust_link(priv);
>
> if (!priv->oldlink) {
> new_state = 1;
>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net-next: stmmac: add adjust_link function
2017-05-16 16:43 ` Florian Fainelli
@ 2017-05-16 19:04 ` Corentin Labbe
2017-05-17 8:21 ` Corentin Labbe
1 sibling, 0 replies; 6+ messages in thread
From: Corentin Labbe @ 2017-05-16 19:04 UTC (permalink / raw)
To: Florian Fainelli; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
On Tue, May 16, 2017 at 09:43:04AM -0700, Florian Fainelli wrote:
> On 05/15/2017 04:41 AM, Corentin Labbe wrote:
> > My dwmac-sun8i serie will add some if (has_sun8i) to
> > stmmac_adjust_link()
> > Since the current stmmac_adjust_link() alreaady have lots of if (has_gmac/gmac4),
> > It is now better to create an adjust_link() function for each dwmac.
>
> Is it really, because the diffstat really seems to indicate otherwise
> and by looking at the code, I am definitively not convinced this is an
> improvement other the current code.
>
> >
> > So this patch add an adjust_link() function pointer, and move code out
> > of stmmac_adjust_link to it.
>
> Can't we keep the existing adjust_link() function and just have a
> different one for dwmac-sun8i that either re-uses portions of the
> existing, or duplicate just what it needs?
>
I have done this work because the maintainer thinked it was a good idea (and so am I)
See https://www.mail-archive.com/netdev@vger.kernel.org/msg167484.html
You can see also the necessary modification of adjust_link for dwmac-sun8i here: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-May/504034.html
I agree that without dwmac-sun8i, this patch is near uncessary, but with it I add 4 "if (sun8i)".
Anyway I will fix all problem you reported below.
Thanks
Regards
> >
> > Removing in the process stmmac_mac_flow_ctrl/stmmac_hw_fix_mac_speed
> > since there not used anymore.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> > drivers/net/ethernet/stmicro/stmmac/common.h | 3 +
> > .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 54 ++++++++++++++
> > .../net/ethernet/stmicro/stmmac/dwmac100_core.c | 46 ++++++++++++
> > drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 54 ++++++++++++++
> > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 83 +---------------------
> > 5 files changed, 158 insertions(+), 82 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> > index b7ce3fbb5375..451c231006fe 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> > @@ -469,11 +469,14 @@ struct stmmac_dma_ops {
> > };
> >
> > struct mac_device_info;
> > +struct stmmac_priv;
> >
> > /* Helpers to program the MAC core */
> > struct stmmac_ops {
> > /* MAC core initialization */
> > void (*core_init)(struct mac_device_info *hw, int mtu);
> > + /* adjust link */
> > + int (*adjust_link)(struct stmmac_priv *priv);
> > /* Enable the MAC RX/TX */
> > void (*set_mac)(void __iomem *ioaddr, bool enable);
> > /* Enable and verify that the IPC module is supported */
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > index f3d9305e5f70..5f3aace46c41 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > @@ -26,6 +26,7 @@
> > #include <linux/slab.h>
> > #include <linux/ethtool.h>
> > #include <asm/io.h>
> > +#include "stmmac.h"
> > #include "stmmac_pcs.h"
> > #include "dwmac1000.h"
> >
> > @@ -75,6 +76,58 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
> > #endif
> > }
> >
> > +static int dwmac1000_adjust_link(struct stmmac_priv *priv)
> > +{
> > + struct net_device *ndev = priv->dev;
> > + struct phy_device *phydev = ndev->phydev;
> > + int new_state = 0;
> > + u32 tx_cnt = priv->plat->tx_queues_to_use;
> > + u32 ctrl;
>
> Reverse christmas tree declaration please.
>
> > +
> > + ctrl = readl(priv->ioaddr + GMAC_CONTROL);
> > +
> > + if (phydev->duplex != priv->oldduplex) {
> > + new_state = 1;
>
> bool new_state
>
> > + if (!(phydev->duplex))
>
> Parenthesis not needed (this is an integer, not a bitmask)
>
> > + ctrl &= ~GMAC_CONTROL_DM;
> > + else
> > + ctrl |= GMAC_CONTROL_DM;
> > + priv->oldduplex = phydev->duplex;
> > + }
> > +
> > + if (phydev->pause)
> > + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
> > + priv->pause, tx_cnt);
> > +
> > + if (phydev->speed != priv->speed) {
> > + new_state = 1;
> > + switch (phydev->speed) {
> > + case 1000:
>
> case SPEED_1000 and so on?
>
> > + ctrl &= ~GMAC_CONTROL_PS;
> > + break;
> > + case 100:
> > + ctrl |= GMAC_CONTROL_PS;
> > + ctrl |= GMAC_CONTROL_FES;
> > + break;
> > + case 10:
> > + ctrl |= GMAC_CONTROL_PS;
> > + ctrl |= ~GMAC_CONTROL_FES;
> > + break;
> > + default:
> > + netif_warn(priv, link, priv->dev,
> > + "broken speed: %d\n", phydev->speed);
> > + phydev->speed = SPEED_UNKNOWN;
> > + break;
> > + }
> > + if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
> > + priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> > + priv->speed = phydev->speed;
> > + }
> > +
> > + writel(ctrl, priv->ioaddr + GMAC_CONTROL);
> > + return new_state;
> > +}
> > +
> > static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw)
> > {
> > void __iomem *ioaddr = hw->pcsr;
> > @@ -490,6 +543,7 @@ static void dwmac1000_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
> >
> > static const struct stmmac_ops dwmac1000_ops = {
> > .core_init = dwmac1000_core_init,
> > + .adjust_link = dwmac1000_adjust_link,
> > .set_mac = stmmac_set_mac,
> > .rx_ipc = dwmac1000_rx_ipc_enable,
> > .dump_regs = dwmac1000_dump_regs,
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> > index 1b3609105484..ba3d46e65e1a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> > @@ -27,6 +27,7 @@
> > #include <linux/crc32.h>
> > #include <asm/io.h>
> > #include "dwmac100.h"
> > +#include "stmmac.h"
> >
> > static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
> > {
> > @@ -40,6 +41,50 @@ static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
> > #endif
> > }
> >
> > +static int dwmac100_adjust_link(struct stmmac_priv *priv)
> > +{
> > + struct net_device *ndev = priv->dev;
> > + struct phy_device *phydev = ndev->phydev;
> > + int new_state = 0;
> > + u32 tx_cnt = priv->plat->tx_queues_to_use;
> > + u32 ctrl;
> > +
> > + ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
> > + if (phydev->duplex != priv->oldduplex) {
> > + new_state = 1;
> > + if (!(phydev->duplex))
> > + ctrl &= ~MAC_CONTROL_F;
> > + else
> > + ctrl |= MAC_CONTROL_F;
> > + priv->oldduplex = phydev->duplex;
> > + }
> > +
> > + if (phydev->pause)
> > + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
> > + priv->pause, tx_cnt);
> > +
> > + if (phydev->speed != priv->speed) {
> > + new_state = 1;
> > + switch (phydev->speed) {
> > + case 100:
> > + case 10:
> > + ctrl &= ~MAC_CONTROL_PS;
> > + break;
> > + default:
> > + netif_warn(priv, link, priv->dev,
> > + "broken speed: %d\n", phydev->speed);
> > + phydev->speed = SPEED_UNKNOWN;
> > + break;
> > + }
> > + if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
> > + priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> > + priv->speed = phydev->speed;
> > + }
> > +
> > + writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> > + return new_state;
> > +}
> > +
> > static void dwmac100_dump_mac_regs(struct mac_device_info *hw, u32 *reg_space)
> > {
> > void __iomem *ioaddr = hw->pcsr;
> > @@ -150,6 +195,7 @@ static void dwmac100_pmt(struct mac_device_info *hw, unsigned long mode)
> >
> > static const struct stmmac_ops dwmac100_ops = {
> > .core_init = dwmac100_core_init,
> > + .adjust_link = dwmac100_adjust_link,
> > .set_mac = stmmac_set_mac,
> > .rx_ipc = dwmac100_rx_ipc_enable,
> > .dump_regs = dwmac100_dump_mac_regs,
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> > index 48793f2e9307..133b6bcd7b61 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> > @@ -19,6 +19,7 @@
> > #include <linux/io.h>
> > #include "stmmac_pcs.h"
> > #include "dwmac4.h"
> > +#include "stmmac.h"
> >
> > static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
> > {
> > @@ -59,6 +60,58 @@ static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
> > writel(value, ioaddr + GMAC_INT_EN);
> > }
> >
> > +static int dwmac4_adjust_link(struct stmmac_priv *priv)
> > +{
> > + struct net_device *ndev = priv->dev;
> > + struct phy_device *phydev = ndev->phydev;
> > + int new_state = 0;
> > + u32 tx_cnt = priv->plat->tx_queues_to_use;
> > + u32 ctrl;
> > +
> > + ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
> > +
> > + if (phydev->duplex != priv->oldduplex) {
> > + new_state = 1;
> > + if (!(phydev->duplex))
> > + ctrl &= ~GMAC_CONFIG_DM;
> > + else
> > + ctrl |= GMAC_CONFIG_DM;
> > + priv->oldduplex = phydev->duplex;
> > + }
> > +
> > + if (phydev->pause)
> > + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex, priv->flow_ctrl,
> > + priv->pause, tx_cnt);
> > +
> > + if (phydev->speed != priv->speed) {
> > + new_state = 1;
> > + switch (phydev->speed) {
> > + case 1000:
> > + ctrl &= ~GMAC_CONFIG_PS;
> > + break;
> > + case 100:
> > + ctrl |= GMAC_CONFIG_PS;
> > + ctrl |= GMAC_CONFIG_FES;
> > + break;
> > + case 10:
> > + ctrl |= GMAC_CONFIG_PS;
> > + ctrl |= ~GMAC_CONFIG_FES;
> > + break;
> > + default:
> > + netif_warn(priv, link, priv->dev,
> > + "broken speed: %d\n", phydev->speed);
> > + phydev->speed = SPEED_UNKNOWN;
> > + break;
> > + }
> > + if (phydev->speed != SPEED_UNKNOWN && likely(priv->plat->fix_mac_speed))
> > + priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> > + priv->speed = phydev->speed;
> > + }
> > +
> > + writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> > + return new_state;
> > +}
> > +
> > static void dwmac4_rx_queue_enable(struct mac_device_info *hw,
> > u8 mode, u32 queue)
> > {
> > @@ -669,6 +722,7 @@ static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
> >
> > static const struct stmmac_ops dwmac4_ops = {
> > .core_init = dwmac4_core_init,
> > + .adjust_link = dwmac4_adjust_link,
> > .set_mac = stmmac_set_mac,
> > .rx_ipc = dwmac4_rx_ipc_enable,
> > .rx_queue_enable = dwmac4_rx_queue_enable,
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index b05a042cf2c6..fb3e2ddaa7c9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -286,21 +286,6 @@ static inline u32 stmmac_rx_dirty(struct stmmac_priv *priv, u32 queue)
> > }
> >
> > /**
> > - * stmmac_hw_fix_mac_speed - callback for speed selection
> > - * @priv: driver private structure
> > - * Description: on some platforms (e.g. ST), some HW system configuration
> > - * registers have to be set according to the link speed negotiated.
> > - */
> > -static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
> > -{
> > - struct net_device *ndev = priv->dev;
> > - struct phy_device *phydev = ndev->phydev;
> > -
> > - if (likely(priv->plat->fix_mac_speed))
> > - priv->plat->fix_mac_speed(priv->plat->bsp_priv, phydev->speed);
> > -}
> > -
> > -/**
> > * stmmac_enable_eee_mode - check and enter in LPI mode
> > * @priv: driver private structure
> > * Description: this function is to verify and enter in LPI mode in case of
> > @@ -759,19 +744,6 @@ static void stmmac_release_ptp(struct stmmac_priv *priv)
> > }
> >
> > /**
> > - * stmmac_mac_flow_ctrl - Configure flow control in all queues
> > - * @priv: driver private structure
> > - * Description: It is used for configuring the flow control in all queues
> > - */
> > -static void stmmac_mac_flow_ctrl(struct stmmac_priv *priv, u32 duplex)
> > -{
> > - u32 tx_cnt = priv->plat->tx_queues_to_use;
> > -
> > - priv->hw->mac->flow_ctrl(priv->hw, duplex, priv->flow_ctrl,
> > - priv->pause, tx_cnt);
> > -}
> > -
> > -/**
> > * stmmac_adjust_link - adjusts the link parameters
> > * @dev: net device structure
> > * Description: this is the helper called by the physical abstraction layer
> > @@ -793,60 +765,7 @@ static void stmmac_adjust_link(struct net_device *dev)
> > spin_lock_irqsave(&priv->lock, flags);
> >
> > if (phydev->link) {
> > - u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
> > -
> > - /* 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))
> > - ctrl &= ~priv->hw->link.duplex;
> > - else
> > - ctrl |= priv->hw->link.duplex;
> > - priv->oldduplex = phydev->duplex;
> > - }
> > - /* Flow Control operation */
> > - if (phydev->pause)
> > - stmmac_mac_flow_ctrl(priv, phydev->duplex);
> > -
> > - if (phydev->speed != priv->speed) {
> > - new_state = 1;
> > - switch (phydev->speed) {
> > - case 1000:
> > - if (priv->plat->has_gmac ||
> > - priv->plat->has_gmac4)
> > - ctrl &= ~priv->hw->link.port;
> > - break;
> > - case 100:
> > - if (priv->plat->has_gmac ||
> > - priv->plat->has_gmac4) {
> > - ctrl |= priv->hw->link.port;
> > - ctrl |= priv->hw->link.speed;
> > - } else {
> > - ctrl &= ~priv->hw->link.port;
> > - }
> > - break;
> > - case 10:
> > - if (priv->plat->has_gmac ||
> > - priv->plat->has_gmac4) {
> > - ctrl |= priv->hw->link.port;
> > - ctrl &= ~(priv->hw->link.speed);
> > - } else {
> > - ctrl &= ~priv->hw->link.port;
> > - }
> > - break;
> > - default:
> > - netif_warn(priv, link, priv->dev,
> > - "broken speed: %d\n", phydev->speed);
> > - phydev->speed = SPEED_UNKNOWN;
> > - break;
> > - }
> > - if (phydev->speed != SPEED_UNKNOWN)
> > - stmmac_hw_fix_mac_speed(priv);
> > - priv->speed = phydev->speed;
> > - }
> > -
> > - writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> > + new_state = priv->hw->mac->adjust_link(priv);
> >
> > if (!priv->oldlink) {
> > new_state = 1;
> >
>
>
> --
> Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net-next: stmmac: add adjust_link function
2017-05-16 16:43 ` Florian Fainelli
2017-05-16 19:04 ` Corentin Labbe
@ 2017-05-17 8:21 ` Corentin Labbe
1 sibling, 0 replies; 6+ messages in thread
From: Corentin Labbe @ 2017-05-17 8:21 UTC (permalink / raw)
To: Florian Fainelli; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
On Tue, May 16, 2017 at 09:43:04AM -0700, Florian Fainelli wrote:
> On 05/15/2017 04:41 AM, Corentin Labbe wrote:
> > My dwmac-sun8i serie will add some if (has_sun8i) to
> > stmmac_adjust_link()
> > Since the current stmmac_adjust_link() alreaady have lots of if (has_gmac/gmac4),
> > It is now better to create an adjust_link() function for each dwmac.
>
> Is it really, because the diffstat really seems to indicate otherwise
> and by looking at the code, I am definitively not convinced this is an
> improvement other the current code.
>
> >
> > So this patch add an adjust_link() function pointer, and move code out
> > of stmmac_adjust_link to it.
>
> Can't we keep the existing adjust_link() function and just have a
> different one for dwmac-sun8i that either re-uses portions of the
> existing, or duplicate just what it needs?
>
I found another way, removing port/speed variable and use new struct links member speed10/speed100/speed1000/speedmask
I will make the current adjust_link function simplier. (and no if has_xxx for any dwmac type)
I will send that try soon.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-17 8:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 11:41 [PATCH 1/2] net-next: stmmac: add adjust_link function Corentin Labbe
2017-05-15 11:41 ` [PATCH 2/2] net-next: stmmac: remove struct mac_link Corentin Labbe
2017-05-16 16:38 ` [PATCH 1/2] net-next: stmmac: add adjust_link function David Miller
2017-05-16 16:43 ` Florian Fainelli
2017-05-16 19:04 ` Corentin Labbe
2017-05-17 8:21 ` Corentin Labbe
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).