* [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality
@ 2020-01-22 9:09 Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Ong Boon Leong @ 2020-01-22 9:09 UTC (permalink / raw)
To: netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
Jose Abreu, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
Thanks to all feedbacks from community.
We updated the patch-series to below:-
1/5: It ensures that the real_num_rx|tx_queues are set in both driver
probe() and resume(). So, move the netif_set_real_num_rx|tx_queues()
into stmmac_hw_setup(). Added lock_acquired to allow to decide when
to rtnl_lock() & rtnl_unlock() pair:-
stmmac_open(): rtnl_lock() & rtnl_unlock() is not needed.
stmmac_resume(): rtnl_lock() & rtnl_unlock() is needed.
2/5: It ensures that the previous value of GMAC_VLAN_TAG register is
read first before for updating the register.
3/5: It ensures the GMAC IP v4.xx and above behaves correctly to:-
ip link set <devname> multicast off|on
4/5: Added similar IFF_MULTICAST flag for xgmac2.
5/5: It ensures PCI platform data is using plat->phy_interface.
Rgds,
Boon Leong
Changes from v2:-
patch 1/5 - added control for rtnl_lock() & rtnl_unlock() to ensure
they are used forstmmac_resume()
patch 4/5 - added IFF_MULTICAST flag check for xgmac to ensure
multicast works correctly.
v1:-
- Drop v1 patches (1/7, 3/7 & 4/7) that are not valid.
Aashish Verma (1):
net: stmmac: Fix incorrect location to set real_num_rx|tx_queues
Tan, Tee Min (2):
net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+
net: stmmac: xgmac: fix missing IFF_MULTICAST checki in
dwxgmac2_set_filter
Verma, Aashish (1):
net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter
Voon Weifeng (1):
net: stmmac: update pci platform data to use phy_interface
.../net/ethernet/stmicro/stmmac/dwmac4_core.c | 9 +++++----
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++++++-------
.../net/ethernet/stmicro/stmmac/stmmac_pci.c | 14 ++++++++------
4 files changed, 26 insertions(+), 18 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues
2020-01-22 9:09 [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
@ 2020-01-22 9:09 ` Ong Boon Leong
2020-01-22 9:55 ` Jose Abreu
2020-01-22 9:09 ` [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+ Ong Boon Leong
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ong Boon Leong @ 2020-01-22 9:09 UTC (permalink / raw)
To: netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
Jose Abreu, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
From: Aashish Verma <aashishx.verma@intel.com>
netif_set_real_num_tx_queues() & netif_set_real_num_rx_queues() should be
used to inform network stack about the real Tx & Rx queue (active) number
in both stmmac_open() and stmmac_resume(), therefore, we move the code
from stmmac_dvr_probe() to stmmac_hw_setup().
For driver open(), rtnl_lock is acquired by network stack but not in the
resume(). Therefore, we introduce lock_acquired boolean to control when
to use rtnl_lock|unlock() within stmmac_hw_setup().
Thanks Jose Abreu for input.
Fixes: c02b7a914551 ("net: stmmac: use netif_set_real_num_{rx,tx}_queues")
Signed-off-by: Aashish Verma <aashishx.verma@intel.com>
Tested-by: Tan, Tee Min <tee.min.tan@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 80d59b775907..417397158d4a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2525,7 +2525,8 @@ static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)
* 0 on success and an appropriate (-)ve integer as defined in errno.h
* file on failure.
*/
-static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
+static int stmmac_hw_setup(struct net_device *dev, bool init_ptp,
+ bool lock_acquired)
{
struct stmmac_priv *priv = netdev_priv(dev);
u32 rx_cnt = priv->plat->rx_queues_to_use;
@@ -2624,6 +2625,14 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
if (priv->dma_cap.vlins)
stmmac_enable_vlan(priv, priv->hw, STMMAC_VLAN_INSERT);
+ /* Configure real RX and TX queues */
+ if (!lock_acquired)
+ rtnl_lock();
+ netif_set_real_num_rx_queues(dev, priv->plat->rx_queues_to_use);
+ netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use);
+ if (!lock_acquired)
+ rtnl_unlock();
+
/* Start the ball rolling... */
stmmac_start_all_dma(priv);
@@ -2695,7 +2704,7 @@ static int stmmac_open(struct net_device *dev)
goto init_error;
}
- ret = stmmac_hw_setup(dev, true);
+ ret = stmmac_hw_setup(dev, true, true);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
goto init_error;
@@ -4622,10 +4631,6 @@ int stmmac_dvr_probe(struct device *device,
stmmac_check_ether_addr(priv);
- /* Configure real RX and TX queues */
- netif_set_real_num_rx_queues(ndev, priv->plat->rx_queues_to_use);
- netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
-
ndev->netdev_ops = &stmmac_netdev_ops;
ndev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
@@ -4973,7 +4978,7 @@ int stmmac_resume(struct device *dev)
stmmac_clear_descriptors(priv);
- stmmac_hw_setup(ndev, false);
+ stmmac_hw_setup(ndev, false, false);
stmmac_init_coalesce(priv);
stmmac_set_rx_mode(ndev);
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+
2020-01-22 9:09 [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
@ 2020-01-22 9:09 ` Ong Boon Leong
2020-01-22 10:31 ` Jose Abreu
2020-01-22 9:09 ` [PATCH net v3 3/5] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter Ong Boon Leong
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ong Boon Leong @ 2020-01-22 9:09 UTC (permalink / raw)
To: netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
Jose Abreu, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
From: "Tan, Tee Min" <tee.min.tan@intel.com>
It should always do a read of current value of GMAC_VLAN_TAG instead of
directly overwriting the register value.
Fixes: c1be0022df0d ("net: stmmac: Add VLAN HASH filtering support in GMAC4+")
Signed-off-by: Tan, Tee Min <tee.min.tan@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 40ca00e596dd..6e3d0ab0ecd6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -736,11 +736,14 @@ static void dwmac4_update_vlan_hash(struct mac_device_info *hw, u32 hash,
__le16 perfect_match, bool is_double)
{
void __iomem *ioaddr = hw->pcsr;
+ u32 value;
writel(hash, ioaddr + GMAC_VLAN_HASH_TABLE);
+ value = readl(ioaddr + GMAC_VLAN_TAG);
+
if (hash) {
- u32 value = GMAC_VLAN_VTHM | GMAC_VLAN_ETV;
+ value |= GMAC_VLAN_VTHM | GMAC_VLAN_ETV;
if (is_double) {
value |= GMAC_VLAN_EDVLP;
value |= GMAC_VLAN_ESVL;
@@ -759,8 +762,6 @@ static void dwmac4_update_vlan_hash(struct mac_device_info *hw, u32 hash,
writel(value | perfect_match, ioaddr + GMAC_VLAN_TAG);
} else {
- u32 value = readl(ioaddr + GMAC_VLAN_TAG);
-
value &= ~(GMAC_VLAN_VTHM | GMAC_VLAN_ETV);
value &= ~(GMAC_VLAN_EDVLP | GMAC_VLAN_ESVL);
value &= ~GMAC_VLAN_DOVLTC;
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net v3 3/5] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter
2020-01-22 9:09 [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+ Ong Boon Leong
@ 2020-01-22 9:09 ` Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 4/5] net: stmmac: xgmac: fix missing IFF_MULTICAST checki in dwxgmac2_set_filter Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 5/5] net: stmmac: update pci platform data to use phy_interface Ong Boon Leong
4 siblings, 0 replies; 11+ messages in thread
From: Ong Boon Leong @ 2020-01-22 9:09 UTC (permalink / raw)
To: netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
Jose Abreu, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
From: "Verma, Aashish" <aashishx.verma@intel.com>
Without checking for IFF_MULTICAST flag, it is wrong to assume multicast
filtering is always enabled. By checking against IFF_MULTICAST, now
the driver behaves correctly when the multicast support is toggled by below
command:-
ip link set <devname> multicast off|on
Fixes: 477286b53f55 ("stmmac: add GMAC4 core support")
Signed-off-by: Verma, Aashish <aashishx.verma@intel.com>
Tested-by: Tan, Tee Min <tee.min.tan@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 6e3d0ab0ecd6..53be936137d0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -420,7 +420,7 @@ static void dwmac4_set_filter(struct mac_device_info *hw,
value |= GMAC_PACKET_FILTER_PM;
/* Set all the bits of the HASH tab */
memset(mc_filter, 0xff, sizeof(mc_filter));
- } else if (!netdev_mc_empty(dev)) {
+ } else if (!netdev_mc_empty(dev) && (dev->flags & IFF_MULTICAST)) {
struct netdev_hw_addr *ha;
/* Hash filter for multicast */
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net v3 4/5] net: stmmac: xgmac: fix missing IFF_MULTICAST checki in dwxgmac2_set_filter
2020-01-22 9:09 [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
` (2 preceding siblings ...)
2020-01-22 9:09 ` [PATCH net v3 3/5] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter Ong Boon Leong
@ 2020-01-22 9:09 ` Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 5/5] net: stmmac: update pci platform data to use phy_interface Ong Boon Leong
4 siblings, 0 replies; 11+ messages in thread
From: Ong Boon Leong @ 2020-01-22 9:09 UTC (permalink / raw)
To: netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
Jose Abreu, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
From: "Tan, Tee Min" <tee.min.tan@intel.com>
Without checking for IFF_MULTICAST flag, it is wrong to assume multicast
filtering is always enabled. By checking against IFF_MULTICAST, now
the driver behaves correctly when the multicast support is toggled by below
command:-
ip link set <devname> multicast off|on
Fixes: 0efedbf11f07a ("net: stmmac: xgmac: Fix XGMAC selftests")
Signed-off-by: Tan, Tee Min <tee.min.tan@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 082f5ee9e525..13a153386c18 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -458,7 +458,7 @@ static void dwxgmac2_set_filter(struct mac_device_info *hw,
for (i = 0; i < XGMAC_MAX_HASH_TABLE; i++)
writel(~0x0, ioaddr + XGMAC_HASH_TABLE(i));
- } else if (!netdev_mc_empty(dev)) {
+ } else if (!netdev_mc_empty(dev) && (dev->flags & IFF_MULTICAST)) {
struct netdev_hw_addr *ha;
value |= XGMAC_FILTER_HMC;
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net v3 5/5] net: stmmac: update pci platform data to use phy_interface
2020-01-22 9:09 [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
` (3 preceding siblings ...)
2020-01-22 9:09 ` [PATCH net v3 4/5] net: stmmac: xgmac: fix missing IFF_MULTICAST checki in dwxgmac2_set_filter Ong Boon Leong
@ 2020-01-22 9:09 ` Ong Boon Leong
4 siblings, 0 replies; 11+ messages in thread
From: Ong Boon Leong @ 2020-01-22 9:09 UTC (permalink / raw)
To: netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
Jose Abreu, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
From: Voon Weifeng <weifeng.voon@intel.com>
The recent patch to support passive mode converter did not take care the
phy interface configuration in PCI platform data. Hence, converting all
the PCI platform data from plat->interface to plat->phy_interface as the
default mode is meant for PHY.
Fixes: 0060c8783330 ("net: stmmac: implement support for passive mode converters via dt")
Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
Tested-by: Tan, Tee Min <tee.min.tan@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 8237dbc3e991..d2bc04dedd7c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -96,7 +96,7 @@ static int stmmac_default_data(struct pci_dev *pdev,
plat->bus_id = 1;
plat->phy_addr = 0;
- plat->interface = PHY_INTERFACE_MODE_GMII;
+ plat->phy_interface = PHY_INTERFACE_MODE_GMII;
plat->dma_cfg->pbl = 32;
plat->dma_cfg->pblx8 = true;
@@ -220,7 +220,8 @@ static int ehl_sgmii_data(struct pci_dev *pdev,
{
plat->bus_id = 1;
plat->phy_addr = 0;
- plat->interface = PHY_INTERFACE_MODE_SGMII;
+ plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+
return ehl_common_data(pdev, plat);
}
@@ -233,7 +234,8 @@ static int ehl_rgmii_data(struct pci_dev *pdev,
{
plat->bus_id = 1;
plat->phy_addr = 0;
- plat->interface = PHY_INTERFACE_MODE_RGMII;
+ plat->phy_interface = PHY_INTERFACE_MODE_RGMII;
+
return ehl_common_data(pdev, plat);
}
@@ -261,7 +263,7 @@ static int tgl_sgmii_data(struct pci_dev *pdev,
{
plat->bus_id = 1;
plat->phy_addr = 0;
- plat->interface = PHY_INTERFACE_MODE_SGMII;
+ plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
return tgl_common_data(pdev, plat);
}
@@ -361,7 +363,7 @@ static int quark_default_data(struct pci_dev *pdev,
plat->bus_id = pci_dev_id(pdev);
plat->phy_addr = ret;
- plat->interface = PHY_INTERFACE_MODE_RMII;
+ plat->phy_interface = PHY_INTERFACE_MODE_RMII;
plat->dma_cfg->pbl = 16;
plat->dma_cfg->pblx8 = true;
@@ -418,7 +420,7 @@ static int snps_gmac5_default_data(struct pci_dev *pdev,
plat->bus_id = 1;
plat->phy_addr = -1;
- plat->interface = PHY_INTERFACE_MODE_GMII;
+ plat->phy_interface = PHY_INTERFACE_MODE_GMII;
plat->dma_cfg->pbl = 32;
plat->dma_cfg->pblx8 = true;
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues
2020-01-22 9:09 ` [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
@ 2020-01-22 9:55 ` Jose Abreu
2020-01-24 8:56 ` Ong, Boon Leong
0 siblings, 1 reply; 11+ messages in thread
From: Jose Abreu @ 2020-01-22 9:55 UTC (permalink / raw)
To: Ong Boon Leong, netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
David S . Miller, Maxime Coquelin, Joao Pinto, Arnd Bergmann,
Alexandru Ardelean, linux-stm32, linux-arm-kernel, linux-kernel
From: Ong Boon Leong <boon.leong.ong@intel.com>
Date: Jan/22/2020, 09:09:32 (UTC+00:00)
> For driver open(), rtnl_lock is acquired by network stack but not in the
> resume(). Therefore, we introduce lock_acquired boolean to control when
> to use rtnl_lock|unlock() within stmmac_hw_setup().
Why not use rtnl_is_locked() instead of the boolean ?
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+
2020-01-22 9:09 ` [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+ Ong Boon Leong
@ 2020-01-22 10:31 ` Jose Abreu
2020-01-24 8:56 ` Ong, Boon Leong
0 siblings, 1 reply; 11+ messages in thread
From: Jose Abreu @ 2020-01-22 10:31 UTC (permalink / raw)
To: Ong Boon Leong, netdev
Cc: Tan Tee Min, Voon Weifeng, Giuseppe Cavallaro, Alexandre TORGUE,
David S . Miller, Maxime Coquelin, Joao Pinto, Arnd Bergmann,
Alexandru Ardelean, linux-stm32, linux-arm-kernel, linux-kernel
From: Ong Boon Leong <boon.leong.ong@intel.com>
Date: Jan/22/2020, 09:09:33 (UTC+00:00)
> It should always do a read of current value of GMAC_VLAN_TAG instead of
> directly overwriting the register value.
Thanks for adding patch 4/5 but I meant in previous reply that this patch
should also go for XGMAC cores ...
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues
2020-01-22 9:55 ` Jose Abreu
@ 2020-01-24 8:56 ` Ong, Boon Leong
2020-01-24 9:06 ` Jose Abreu
0 siblings, 1 reply; 11+ messages in thread
From: Ong, Boon Leong @ 2020-01-24 8:56 UTC (permalink / raw)
To: Jose Abreu, netdev
Cc: Tan, Tee Min, Voon, Weifeng, Giuseppe Cavallaro,
Alexandre TORGUE, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
>-----Original Message-----
>From: Jose Abreu <Jose.Abreu@synopsys.com>
>Sent: Wednesday, January 22, 2020 5:56 PM
>To: Ong, Boon Leong <boon.leong.ong@intel.com>; netdev@vger.kernel.org
>Cc: Tan, Tee Min <tee.min.tan@intel.com>; Voon, Weifeng
><weifeng.voon@intel.com>; Giuseppe Cavallaro <peppe.cavallaro@st.com>;
>Alexandre TORGUE <alexandre.torgue@st.com>; David S . Miller
><davem@davemloft.net>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
>Joao Pinto <Joao.Pinto@synopsys.com>; Arnd Bergmann <arnd@arndb.de>;
>Alexandru Ardelean <alexandru.ardelean@analog.com>; linux-stm32@st-md-
>mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org
>Subject: RE: [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set
>real_num_rx|tx_queues
>
>From: Ong Boon Leong <boon.leong.ong@intel.com>
>Date: Jan/22/2020, 09:09:32 (UTC+00:00)
>
>> For driver open(), rtnl_lock is acquired by network stack but not in the
>> resume(). Therefore, we introduce lock_acquired boolean to control when
>> to use rtnl_lock|unlock() within stmmac_hw_setup().
>
>Why not use rtnl_is_locked() instead of the boolean ?
We know that stmmac_open() is called with rtnl_mutex locked by caller.
And, stmmac_resume() is called without rtnl_mutex is locked by caller.
If we replace the boolean with rtnl_is_locked(), then we will have the
following logics in stmmac_hw_setup():-
if (!rtnl_is_locked) ---- (A)
rtnl_lock();
netif_set_real_num_rx_queues();
netif_set_real_num_tx_queues();
if (!rtnl_is_locked) ---- (B)
rtnl_unlock();
For stmmac_open(), (A) is false but (B) is true.
So, the stmmac_open() exits with rtnl_mutex is released.
Here, the above logic does not perserve the original rtnl_mutex
is locked when stmmac_open() is called.
For stmmac_resume(), (A) is true, and (B) is also true.
So, the stmmac_resume() exits with rtnl_mutex is released.
Here, the above logic works well as the original rtnl_mutex is released
when stmmac_resume() is called.
So, as far as I can see, the proposed boolean approach works fine for both
stmmac_open() and stmmac_resume().
Do you agree?
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+
2020-01-22 10:31 ` Jose Abreu
@ 2020-01-24 8:56 ` Ong, Boon Leong
0 siblings, 0 replies; 11+ messages in thread
From: Ong, Boon Leong @ 2020-01-24 8:56 UTC (permalink / raw)
To: Jose Abreu, netdev
Cc: Tan, Tee Min, Voon, Weifeng, Giuseppe Cavallaro,
Alexandre TORGUE, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
>Thanks for adding patch 4/5 but I meant in previous reply that this patch
>should also go for XGMAC cores ...
>
Noted. We will add tis in v4.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues
2020-01-24 8:56 ` Ong, Boon Leong
@ 2020-01-24 9:06 ` Jose Abreu
0 siblings, 0 replies; 11+ messages in thread
From: Jose Abreu @ 2020-01-24 9:06 UTC (permalink / raw)
To: Ong, Boon Leong, netdev
Cc: Tan, Tee Min, Voon, Weifeng, Giuseppe Cavallaro,
Alexandre TORGUE, David S . Miller, Maxime Coquelin, Joao Pinto,
Arnd Bergmann, Alexandru Ardelean, linux-stm32, linux-arm-kernel,
linux-kernel
From: Ong, Boon Leong <boon.leong.ong@intel.com>
Date: Jan/24/2020, 08:56:28 (UTC+00:00)
> >Why not use rtnl_is_locked() instead of the boolean ?
>
> We know that stmmac_open() is called with rtnl_mutex locked by caller.
> And, stmmac_resume() is called without rtnl_mutex is locked by caller.
> If we replace the boolean with rtnl_is_locked(), then we will have the
> following logics in stmmac_hw_setup():-
>
> if (!rtnl_is_locked) ---- (A)
> rtnl_lock();
> netif_set_real_num_rx_queues();
> netif_set_real_num_tx_queues();
> if (!rtnl_is_locked) ---- (B)
> rtnl_unlock();
>
> For stmmac_open(), (A) is false but (B) is true.
> So, the stmmac_open() exits with rtnl_mutex is released.
> Here, the above logic does not perserve the original rtnl_mutex
> is locked when stmmac_open() is called.
>
> For stmmac_resume(), (A) is true, and (B) is also true.
> So, the stmmac_resume() exits with rtnl_mutex is released.
> Here, the above logic works well as the original rtnl_mutex is released
> when stmmac_resume() is called.
>
> So, as far as I can see, the proposed boolean approach works fine for both
> stmmac_open() and stmmac_resume().
>
> Do you agree?
Can't you just wrap all the HW related logic in stmmac_resume() and
stmmac_suspend() with the rtnl lock ? Seems like the right thing to do and
you won't need the boolean.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-01-24 9:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-22 9:09 [PATCH net v3 0/5] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 1/5] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
2020-01-22 9:55 ` Jose Abreu
2020-01-24 8:56 ` Ong, Boon Leong
2020-01-24 9:06 ` Jose Abreu
2020-01-22 9:09 ` [PATCH net v3 2/5] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting in GMAC4+ Ong Boon Leong
2020-01-22 10:31 ` Jose Abreu
2020-01-24 8:56 ` Ong, Boon Leong
2020-01-22 9:09 ` [PATCH net v3 3/5] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 4/5] net: stmmac: xgmac: fix missing IFF_MULTICAST checki in dwxgmac2_set_filter Ong Boon Leong
2020-01-22 9:09 ` [PATCH net v3 5/5] net: stmmac: update pci platform data to use phy_interface Ong Boon Leong
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).