* [PATCH v2 0/4] net-next: stmmac: rework the speed selection
@ 2017-05-22 12:33 Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool Corentin Labbe
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Corentin Labbe @ 2017-05-22 12:33 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
Hello
The current stmmac_adjust_link() part which handle speed have
some if (has_platform) code and my dwmac-sun8i will add more of them.
So we need to handle better speed selection.
Moreover the struct link member speed and port are hard to guess their
purpose. And their unique usage are to be combined for writing speed.
My first try was to create an adjust_link() in stmmac_ops but it duplicate some code
The current solution is to have direct value for 10/100/1000 and a mask for them.
The first 3 patchs fix some minor problem found in stmmac_adjust_link() and reported by Florian Fainelli in my previous serie.
The last patch is the real work.
This serie is tested on cubieboard2 (dwmac1000) and opipc (dwmac-sun8i).
Regards
Changes since v2:
- use true/false for new_state in patch #1
Corentin Labbe (4):
net: stmmac: Convert new_state to bool
net: stmmac: Remove unnecessary parenthesis
net: stmmac: use SPEED_xxx instead of raw value
net: stmmac: rework the speed selection
drivers/net/ethernet/stmicro/stmmac/common.h | 8 +++---
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 26 ++++++++++--------
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 6 +++--
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 26 ++++++++++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 +++++++---------------
5 files changed, 48 insertions(+), 49 deletions(-)
--
2.13.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool
2017-05-22 12:33 [PATCH v2 0/4] net-next: stmmac: rework the speed selection Corentin Labbe
@ 2017-05-22 12:33 ` Corentin Labbe
2017-05-22 13:07 ` Joe Perches
2017-05-22 12:33 ` [PATCH v2 2/4] net-next: stmmac: Remove unnecessary parenthesis Corentin Labbe
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Corentin Labbe @ 2017-05-22 12:33 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
This patch convert new_state from int to bool since it store only 1 or 0
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1da17cd519f6..94b37323844b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -785,7 +785,7 @@ static void stmmac_adjust_link(struct net_device *dev)
struct stmmac_priv *priv = netdev_priv(dev);
struct phy_device *phydev = dev->phydev;
unsigned long flags;
- int new_state = 0;
+ bool new_state = false;
if (!phydev)
return;
@@ -798,7 +798,7 @@ static void stmmac_adjust_link(struct net_device *dev)
/* 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;
+ new_state = true;
if (!(phydev->duplex))
ctrl &= ~priv->hw->link.duplex;
else
@@ -810,7 +810,7 @@ static void stmmac_adjust_link(struct net_device *dev)
stmmac_mac_flow_ctrl(priv, phydev->duplex);
if (phydev->speed != priv->speed) {
- new_state = 1;
+ new_state = true;
switch (phydev->speed) {
case 1000:
if (priv->plat->has_gmac ||
@@ -849,11 +849,11 @@ static void stmmac_adjust_link(struct net_device *dev)
writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
if (!priv->oldlink) {
- new_state = 1;
+ new_state = true;
priv->oldlink = 1;
}
} else if (priv->oldlink) {
- new_state = 1;
+ new_state = true;
priv->oldlink = 0;
priv->speed = SPEED_UNKNOWN;
priv->oldduplex = DUPLEX_UNKNOWN;
--
2.13.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] net-next: stmmac: Remove unnecessary parenthesis
2017-05-22 12:33 [PATCH v2 0/4] net-next: stmmac: rework the speed selection Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool Corentin Labbe
@ 2017-05-22 12:33 ` Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 3/4] net-next: stmmac: use SPEED_xxx instead of raw value Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 4/4] net-next: stmmac: rework the speed selection Corentin Labbe
3 siblings, 0 replies; 8+ messages in thread
From: Corentin Labbe @ 2017-05-22 12:33 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 94b37323844b..190686e39835 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -799,7 +799,7 @@ static void stmmac_adjust_link(struct net_device *dev)
* If not, we operate in half-duplex mode. */
if (phydev->duplex != priv->oldduplex) {
new_state = true;
- if (!(phydev->duplex))
+ if (!phydev->duplex)
ctrl &= ~priv->hw->link.duplex;
else
ctrl |= priv->hw->link.duplex;
--
2.13.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] net-next: stmmac: use SPEED_xxx instead of raw value
2017-05-22 12:33 [PATCH v2 0/4] net-next: stmmac: rework the speed selection Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 2/4] net-next: stmmac: Remove unnecessary parenthesis Corentin Labbe
@ 2017-05-22 12:33 ` Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 4/4] net-next: stmmac: rework the speed selection Corentin Labbe
3 siblings, 0 replies; 8+ messages in thread
From: Corentin Labbe @ 2017-05-22 12:33 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 190686e39835..9bf09100c199 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -812,12 +812,12 @@ static void stmmac_adjust_link(struct net_device *dev)
if (phydev->speed != priv->speed) {
new_state = true;
switch (phydev->speed) {
- case 1000:
+ case SPEED_1000:
if (priv->plat->has_gmac ||
priv->plat->has_gmac4)
ctrl &= ~priv->hw->link.port;
break;
- case 100:
+ case SPEED_100:
if (priv->plat->has_gmac ||
priv->plat->has_gmac4) {
ctrl |= priv->hw->link.port;
@@ -826,7 +826,7 @@ static void stmmac_adjust_link(struct net_device *dev)
ctrl &= ~priv->hw->link.port;
}
break;
- case 10:
+ case SPEED_10:
if (priv->plat->has_gmac ||
priv->plat->has_gmac4) {
ctrl |= priv->hw->link.port;
--
2.13.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] net-next: stmmac: rework the speed selection
2017-05-22 12:33 [PATCH v2 0/4] net-next: stmmac: rework the speed selection Corentin Labbe
` (2 preceding siblings ...)
2017-05-22 12:33 ` [PATCH v2 3/4] net-next: stmmac: use SPEED_xxx instead of raw value Corentin Labbe
@ 2017-05-22 12:33 ` Corentin Labbe
2017-05-22 18:49 ` David Miller
3 siblings, 1 reply; 8+ messages in thread
From: Corentin Labbe @ 2017-05-22 12:33 UTC (permalink / raw)
To: peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel, Corentin Labbe
The current stmmac_adjust_link() part which handle speed have
some if (has_platform) code and my dwmac-sun8i will add more of them.
So we need to handle better speed selection.
Moreover the struct link member speed and port are hard to guess their
purpose. And their unique usage are to be combined for writing speed.
So this patch replace speed/port by simpler
speed10/speed100/speed1000/speed_mask variables.
In dwmac4_core_init and dwmac1000_core_init, port/speed value was used
directly without using the struct link. This patch convert also their
usage to speedxxx.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 8 ++++---
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 26 +++++++++++++---------
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 6 +++--
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 26 +++++++++++++---------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 21 ++++-------------
5 files changed, 43 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index b7ce3fbb5375..e82b4b70b7be 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -549,9 +549,11 @@ 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;
+ u32 speed_mask;
+ u32 speed10;
+ u32 speed100;
+ u32 speed1000;
+ u32 duplex;
};
struct mii_regs {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index f3d9305e5f70..b8848a9d70c5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -45,15 +45,17 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
if (hw->ps) {
value |= GMAC_CONTROL_TE;
- if (hw->ps == SPEED_1000) {
- value &= ~GMAC_CONTROL_PS;
- } else {
- value |= GMAC_CONTROL_PS;
-
- if (hw->ps == SPEED_10)
- value &= ~GMAC_CONTROL_FES;
- else
- value |= GMAC_CONTROL_FES;
+ value &= ~hw->link.speed_mask;
+ switch (hw->ps) {
+ case SPEED_1000:
+ value |= hw->link.speed1000;
+ break;
+ case SPEED_100:
+ value |= hw->link.speed100;
+ break;
+ case SPEED_10:
+ value |= hw->link.speed10;
+ break;
}
}
@@ -531,9 +533,11 @@ 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->link.speed10 = GMAC_CONTROL_PS;
+ mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES;
+ mac->link.speed1000 = 0;
+ mac->link.speed_mask = GENMASK(15, 14);
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 1b3609105484..8ef517356313 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -175,9 +175,11 @@ 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->link.speed10 = 0;
+ mac->link.speed100 = 0;
+ mac->link.speed1000 = 0;
+ mac->link.speed_mask = MAC_CONTROL_PS;
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 48793f2e9307..d371e18b122c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -35,15 +35,17 @@ static void dwmac4_core_init(struct mac_device_info *hw, int mtu)
if (hw->ps) {
value |= GMAC_CONFIG_TE;
- if (hw->ps == SPEED_1000) {
- value &= ~GMAC_CONFIG_PS;
- } else {
- value |= GMAC_CONFIG_PS;
-
- if (hw->ps == SPEED_10)
- value &= ~GMAC_CONFIG_FES;
- else
- value |= GMAC_CONFIG_FES;
+ value &= hw->link.speed_mask;
+ switch (hw->ps) {
+ case SPEED_1000:
+ value |= hw->link.speed1000;
+ break;
+ case SPEED_100:
+ value |= hw->link.speed100;
+ break;
+ case SPEED_10:
+ value |= hw->link.speed10;
+ break;
}
}
@@ -747,9 +749,11 @@ 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->link.speed10 = GMAC_CONFIG_PS;
+ mac->link.speed100 = GMAC_CONFIG_FES | GMAC_CONFIG_PS;
+ mac->link.speed1000 = 0;
+ mac->link.speed_mask = GENMASK(15, 14);
mac->mii.addr = GMAC_MDIO_ADDR;
mac->mii.data = GMAC_MDIO_DATA;
mac->mii.addr_shift = 21;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9bf09100c199..ce99b8aa6172 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -811,29 +811,16 @@ static void stmmac_adjust_link(struct net_device *dev)
if (phydev->speed != priv->speed) {
new_state = true;
+ ctrl &= ~priv->hw->link.speed_mask;
switch (phydev->speed) {
case SPEED_1000:
- if (priv->plat->has_gmac ||
- priv->plat->has_gmac4)
- ctrl &= ~priv->hw->link.port;
+ ctrl |= priv->hw->link.speed1000;
break;
case SPEED_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;
- }
+ ctrl |= priv->hw->link.speed100;
break;
case SPEED_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;
- }
+ ctrl |= priv->hw->link.speed10;
break;
default:
netif_warn(priv, link, priv->dev,
--
2.13.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool
2017-05-22 12:33 ` [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool Corentin Labbe
@ 2017-05-22 13:07 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2017-05-22 13:07 UTC (permalink / raw)
To: Corentin Labbe, peppe.cavallaro, alexandre.torgue; +Cc: netdev, linux-kernel
On Mon, 2017-05-22 at 14:33 +0200, Corentin Labbe wrote:
> This patch convert new_state from int to bool since it store only 1 or 0
[]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[]
> @@ -849,11 +849,11 @@ static void stmmac_adjust_link(struct net_device *dev)
> writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
>
> if (!priv->oldlink) {
> - new_state = 1;
> + new_state = true;
> priv->oldlink = 1;
> }
> } else if (priv->oldlink) {
> - new_state = 1;
> + new_state = true;
> priv->oldlink = 0;
> priv->speed = SPEED_UNKNOWN;
> priv->oldduplex = DUPLEX_UNKNOWN;
It seems oldlink could be bool as well.
drivers/net/ethernet/stmicro/stmmac/stmmac.h: int oldlink;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: if (!priv->oldlink) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: priv->oldlink = 1;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: } else if (priv->oldlink) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: priv->oldlink = 0;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: priv->oldlink = 0;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: priv->oldlink = 0;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/4] net-next: stmmac: rework the speed selection
2017-05-22 12:33 ` [PATCH v2 4/4] net-next: stmmac: rework the speed selection Corentin Labbe
@ 2017-05-22 18:49 ` David Miller
2017-05-23 7:10 ` Corentin Labbe
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2017-05-22 18:49 UTC (permalink / raw)
To: clabbe.montjoie; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Mon, 22 May 2017 14:33:47 +0200
> The current stmmac_adjust_link() part which handle speed have
> some if (has_platform) code and my dwmac-sun8i will add more of them.
>
> So we need to handle better speed selection.
> Moreover the struct link member speed and port are hard to guess their
> purpose. And their unique usage are to be combined for writing speed.
>
> So this patch replace speed/port by simpler
> speed10/speed100/speed1000/speed_mask variables.
>
> In dwmac4_core_init and dwmac1000_core_init, port/speed value was used
> directly without using the struct link. This patch convert also their
> usage to speedxxx.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/common.h | 8 ++++---
> .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 26 +++++++++++++---------
> .../net/ethernet/stmicro/stmmac/dwmac100_core.c | 6 +++--
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 26 +++++++++++++---------
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 21 ++++-------------
> 5 files changed, 43 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index b7ce3fbb5375..e82b4b70b7be 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -549,9 +549,11 @@ 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;
> + u32 speed_mask;
> + u32 speed10;
> + u32 speed100;
> + u32 speed1000;
> + u32 duplex;
> };
>
> struct mii_regs {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index f3d9305e5f70..b8848a9d70c5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -45,15 +45,17 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
> if (hw->ps) {
> value |= GMAC_CONTROL_TE;
>
> - if (hw->ps == SPEED_1000) {
> - value &= ~GMAC_CONTROL_PS;
> - } else {
> - value |= GMAC_CONTROL_PS;
> -
> - if (hw->ps == SPEED_10)
> - value &= ~GMAC_CONTROL_FES;
> - else
> - value |= GMAC_CONTROL_FES;
> + value &= ~hw->link.speed_mask;
> + switch (hw->ps) {
> + case SPEED_1000:
> + value |= hw->link.speed1000;
> + break;
> + case SPEED_100:
> + value |= hw->link.speed100;
> + break;
> + case SPEED_10:
> + value |= hw->link.speed10;
> + break;
> }
> }
>
> @@ -531,9 +533,11 @@ 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->link.speed10 = GMAC_CONTROL_PS;
> + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES;
> + mac->link.speed1000 = 0;
> + mac->link.speed_mask = GENMASK(15, 14);
Neither GMAC_CONTROL_PS nor GMAC_CONTROL_FES are defined with
the GENMASK() macro. So it is very confusing to see constant
bit specifications here in C code.
There are two ways to do this properly:
1) Use "(GMAC_CONTROL_PS | GMAC_CONTROL_FES)"
2) Define a new GMAC_CONTROL_SPDMASK to "GMAC_CONTROL_PS | GMAC_CONTROL_FES"
and use that here.
> @@ -747,9 +749,11 @@ 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->link.speed10 = GMAC_CONFIG_PS;
> + mac->link.speed100 = GMAC_CONFIG_FES | GMAC_CONFIG_PS;
> + mac->link.speed1000 = 0;
> + mac->link.speed_mask = GENMASK(15, 14);
> mac->mii.addr = GMAC_MDIO_ADDR;
> mac->mii.data = GMAC_MDIO_DATA;
> mac->mii.addr_shift = 21;
Likewise.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/4] net-next: stmmac: rework the speed selection
2017-05-22 18:49 ` David Miller
@ 2017-05-23 7:10 ` Corentin Labbe
0 siblings, 0 replies; 8+ messages in thread
From: Corentin Labbe @ 2017-05-23 7:10 UTC (permalink / raw)
To: David Miller; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
On Mon, May 22, 2017 at 02:49:44PM -0400, David Miller wrote:
> From: Corentin Labbe <clabbe.montjoie@gmail.com>
> Date: Mon, 22 May 2017 14:33:47 +0200
>
> > - mac->link.port = GMAC_CONTROL_PS;
> > mac->link.duplex = GMAC_CONTROL_DM;
> > - mac->link.speed = GMAC_CONTROL_FES;
> > + mac->link.speed10 = GMAC_CONTROL_PS;
> > + mac->link.speed100 = GMAC_CONTROL_PS | GMAC_CONTROL_FES;
> > + mac->link.speed1000 = 0;
> > + mac->link.speed_mask = GENMASK(15, 14);
>
> Neither GMAC_CONTROL_PS nor GMAC_CONTROL_FES are defined with
> the GENMASK() macro. So it is very confusing to see constant
> bit specifications here in C code.
>
> There are two ways to do this properly:
>
> 1) Use "(GMAC_CONTROL_PS | GMAC_CONTROL_FES)"
>
> 2) Define a new GMAC_CONTROL_SPDMASK to "GMAC_CONTROL_PS | GMAC_CONTROL_FES"
> and use that here.
>
Since dwmac100 use the #1, I will do the same on dwmac4/dwmac1000
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-05-23 7:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-22 12:33 [PATCH v2 0/4] net-next: stmmac: rework the speed selection Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 1/4] net-next: stmmac: Convert new_state to bool Corentin Labbe
2017-05-22 13:07 ` Joe Perches
2017-05-22 12:33 ` [PATCH v2 2/4] net-next: stmmac: Remove unnecessary parenthesis Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 3/4] net-next: stmmac: use SPEED_xxx instead of raw value Corentin Labbe
2017-05-22 12:33 ` [PATCH v2 4/4] net-next: stmmac: rework the speed selection Corentin Labbe
2017-05-22 18:49 ` David Miller
2017-05-23 7:10 ` 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).