* [PATCH v2 01/14] sh_eth: fix invalid context bug while calling auto-negotiation by ethtool
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
@ 2018-07-04 8:12 ` Vladimir Zapolskiy
2018-07-06 15:39 ` Sergei Shtylyov
2018-07-04 8:12 ` [PATCH v2 02/14] sh_eth: fix invalid context bug while changing link options " Vladimir Zapolskiy
` (15 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:12 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
potentially sleeping") phy_start_aneg() function utilizes a mutex
to serialize changes to phy state, however the helper function is
called in atomic context.
The bug can be reproduced by running "ethtool -r" command, the bug
is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
Fixes: dc19e4e5e02f ("sh: sh_eth: Add support ethtool")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index e9007b613f17..e8aca46bb925 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2265,18 +2265,10 @@ static void sh_eth_get_regs(struct net_device *ndev, struct ethtool_regs *regs,
static int sh_eth_nway_reset(struct net_device *ndev)
{
- struct sh_eth_private *mdp = netdev_priv(ndev);
- unsigned long flags;
- int ret;
-
if (!ndev->phydev)
return -ENODEV;
- spin_lock_irqsave(&mdp->lock, flags);
- ret = phy_start_aneg(ndev->phydev);
- spin_unlock_irqrestore(&mdp->lock, flags);
-
- return ret;
+ return phy_start_aneg(ndev->phydev);
}
static u32 sh_eth_get_msglevel(struct net_device *ndev)
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 01/14] sh_eth: fix invalid context bug while calling auto-negotiation by ethtool
2018-07-04 8:12 ` [PATCH v2 01/14] sh_eth: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
@ 2018-07-06 15:39 ` Sergei Shtylyov
0 siblings, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-06 15:39 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
Hello!
On 7/4/2018 11:12 AM, Vladimir Zapolskiy wrote:
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however the helper function is
> called in atomic context.
>
> The bug can be reproduced by running "ethtool -r" command, the bug
> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
>
> Fixes: dc19e4e5e02f ("sh: sh_eth: Add support ethtool")
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
The patch obviously does what it promises, however I'm still not sure
whether it should be placed after the next fix, not before...
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 02/14] sh_eth: fix invalid context bug while changing link options by ethtool
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
2018-07-04 8:12 ` [PATCH v2 01/14] sh_eth: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
@ 2018-07-04 8:12 ` Vladimir Zapolskiy
2018-07-06 18:43 ` Sergei Shtylyov
2018-07-04 8:12 ` [PATCH v2 03/14] sh_eth: simplify link auto-negotiation " Vladimir Zapolskiy
` (14 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:12 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The change fixes sleep in atomic context bug, which is encountered
every time when link settings are changed by ethtool.
Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
potentially sleeping") phy_start_aneg() function utilizes a mutex
to serialize changes to phy state, however that helper function is
called in atomic context under a grabbed spinlock, because
phy_start_aneg() is called by phy_ethtool_ksettings_set() and by
replaced phy_ethtool_sset() helpers from phylib.
Now duplex mode setting is enforced in sh_eth_adjust_link() only,
also now RX/TX is disabled when link is put down or modifications
to E-MAC registers ECMR and GECMR are expected for both cases of
checked and ignored link status pin state from E-MAC interrupt handler.
For reference the change is a partial rework of commit 1e1b812bbe10
("sh_eth: fix handling of no LINK signal").
Fixes: dc19e4e5e02f ("sh: sh_eth: Add support ethtool")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 49 ++++++++-------------------
1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index e8aca46bb925..8e429e865552 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1927,8 +1927,15 @@ static void sh_eth_adjust_link(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
struct phy_device *phydev = ndev->phydev;
+ unsigned long flags;
int new_state = 0;
+ spin_lock_irqsave(&mdp->lock, flags);
+
+ /* Disable TX and RX right over here, if E-MAC change is ignored */
+ if (mdp->cd->no_psr || mdp->no_ether_link)
+ sh_eth_rcv_snd_disable(ndev);
+
if (phydev->link) {
if (phydev->duplex != mdp->duplex) {
new_state = 1;
@@ -1947,18 +1954,21 @@ static void sh_eth_adjust_link(struct net_device *ndev)
sh_eth_modify(ndev, ECMR, ECMR_TXF, 0);
new_state = 1;
mdp->link = phydev->link;
- if (mdp->cd->no_psr || mdp->no_ether_link)
- sh_eth_rcv_snd_enable(ndev);
}
} else if (mdp->link) {
new_state = 1;
mdp->link = 0;
mdp->speed = 0;
mdp->duplex = -1;
- if (mdp->cd->no_psr || mdp->no_ether_link)
- sh_eth_rcv_snd_disable(ndev);
}
+ /* Enable TX and RX right over here, if E-MAC change is ignored */
+ if ((mdp->cd->no_psr || mdp->no_ether_link) && phydev->link)
+ sh_eth_rcv_snd_enable(ndev);
+
+ mmiowb();
+ spin_unlock_irqrestore(&mdp->lock, flags);
+
if (new_state && netif_msg_link(mdp))
phy_print_status(phydev);
}
@@ -2049,39 +2059,10 @@ static int sh_eth_get_link_ksettings(struct net_device *ndev,
static int sh_eth_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
- struct sh_eth_private *mdp = netdev_priv(ndev);
- unsigned long flags;
- int ret;
-
if (!ndev->phydev)
return -ENODEV;
- spin_lock_irqsave(&mdp->lock, flags);
-
- /* disable tx and rx */
- sh_eth_rcv_snd_disable(ndev);
-
- ret = phy_ethtool_ksettings_set(ndev->phydev, cmd);
- if (ret)
- goto error_exit;
-
- if (cmd->base.duplex == DUPLEX_FULL)
- mdp->duplex = 1;
- else
- mdp->duplex = 0;
-
- if (mdp->cd->set_duplex)
- mdp->cd->set_duplex(ndev);
-
-error_exit:
- mdelay(1);
-
- /* enable tx and rx */
- sh_eth_rcv_snd_enable(ndev);
-
- spin_unlock_irqrestore(&mdp->lock, flags);
-
- return ret;
+ return phy_ethtool_ksettings_set(ndev->phydev, cmd);
}
/* If it is ever necessary to increase SH_ETH_REG_DUMP_MAX_REGS, the
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 02/14] sh_eth: fix invalid context bug while changing link options by ethtool
2018-07-04 8:12 ` [PATCH v2 02/14] sh_eth: fix invalid context bug while changing link options " Vladimir Zapolskiy
@ 2018-07-06 18:43 ` Sergei Shtylyov
0 siblings, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-06 18:43 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:12 AM, Vladimir Zapolskiy wrote:
> The change fixes sleep in atomic context bug, which is encountered
> every time when link settings are changed by ethtool.
>
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however that helper function is
> called in atomic context under a grabbed spinlock, because
> phy_start_aneg() is called by phy_ethtool_ksettings_set() and by
> replaced phy_ethtool_sset() helpers from phylib.
So if all this boils down to phy_start_aneg(), shouldn't this patch
precede patch #1. Or even fixing both call chains with 1 patch? :-)
> Now duplex mode setting is enforced in sh_eth_adjust_link() only,
> also now RX/TX is disabled when link is put down or modifications
> to E-MAC registers ECMR and GECMR are expected for both cases of
> checked and ignored link status pin state from E-MAC interrupt handler.
>
> For reference the change is a partial rework of commit 1e1b812bbe10
> ("sh_eth: fix handling of no LINK signal").
>
> Fixes: dc19e4e5e02f ("sh: sh_eth: Add support ethtool")
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 03/14] sh_eth: simplify link auto-negotiation by ethtool
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
2018-07-04 8:12 ` [PATCH v2 01/14] sh_eth: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
2018-07-04 8:12 ` [PATCH v2 02/14] sh_eth: fix invalid context bug while changing link options " Vladimir Zapolskiy
@ 2018-07-04 8:12 ` Vladimir Zapolskiy
2018-07-06 18:48 ` Sergei Shtylyov
2018-07-04 8:12 ` [PATCH v2 04/14] sh_eth: remove custom .nway_reset from ethtool ops Vladimir Zapolskiy
` (13 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:12 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
There is no need to call a heavyweight phy_start_aneg() for phy
auto-negotiation by ethtool, the phy is already initialized and
link auto-negotiation is started by calling phy_start() from
sh_eth_phy_start() when a network device is opened.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 8e429e865552..1bed2ee4d709 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2249,7 +2249,7 @@ static int sh_eth_nway_reset(struct net_device *ndev)
if (!ndev->phydev)
return -ENODEV;
- return phy_start_aneg(ndev->phydev);
+ return phy_restart_aneg(ndev->phydev);
}
static u32 sh_eth_get_msglevel(struct net_device *ndev)
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 03/14] sh_eth: simplify link auto-negotiation by ethtool
2018-07-04 8:12 ` [PATCH v2 03/14] sh_eth: simplify link auto-negotiation " Vladimir Zapolskiy
@ 2018-07-06 18:48 ` Sergei Shtylyov
0 siblings, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-06 18:48 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:12 AM, Vladimir Zapolskiy wrote:
> There is no need to call a heavyweight phy_start_aneg() for phy
> auto-negotiation by ethtool, the phy is already initialized and
> link auto-negotiation is started by calling phy_start() from
> sh_eth_phy_start() when a network device is opened.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 04/14] sh_eth: remove custom .nway_reset from ethtool ops
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (2 preceding siblings ...)
2018-07-04 8:12 ` [PATCH v2 03/14] sh_eth: simplify link auto-negotiation " Vladimir Zapolskiy
@ 2018-07-04 8:12 ` Vladimir Zapolskiy
2018-07-06 19:04 ` Sergei Shtylyov
2018-07-04 8:14 ` [PATCH v2 05/14] sh_eth: remove useless serialization in sh_eth_get_link_ksettings() Vladimir Zapolskiy
` (12 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:12 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The generic phy_ethtool_nway_reset() function from phylib can be used
instead of in-house sh_eth_nway_reset().
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 1bed2ee4d709..50ff18870be2 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2244,14 +2244,6 @@ static void sh_eth_get_regs(struct net_device *ndev, struct ethtool_regs *regs,
pm_runtime_put_sync(&mdp->pdev->dev);
}
-static int sh_eth_nway_reset(struct net_device *ndev)
-{
- if (!ndev->phydev)
- return -ENODEV;
-
- return phy_restart_aneg(ndev->phydev);
-}
-
static u32 sh_eth_get_msglevel(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -2402,7 +2394,7 @@ static int sh_eth_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
static const struct ethtool_ops sh_eth_ethtool_ops = {
.get_regs_len = sh_eth_get_regs_len,
.get_regs = sh_eth_get_regs,
- .nway_reset = sh_eth_nway_reset,
+ .nway_reset = phy_ethtool_nway_reset,
.get_msglevel = sh_eth_get_msglevel,
.set_msglevel = sh_eth_set_msglevel,
.get_link = ethtool_op_get_link,
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 05/14] sh_eth: remove useless serialization in sh_eth_get_link_ksettings()
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (3 preceding siblings ...)
2018-07-04 8:12 ` [PATCH v2 04/14] sh_eth: remove custom .nway_reset from ethtool ops Vladimir Zapolskiy
@ 2018-07-04 8:14 ` Vladimir Zapolskiy
2018-07-06 19:06 ` Sergei Shtylyov
2018-07-04 8:14 ` [PATCH v2 06/14] sh_eth: remove custom .get_link_ksettings from ethtool ops Vladimir Zapolskiy
` (11 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:14 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
phy_ethtool_ksettings_get() call does not modify device state or device
driver state, hence there is no need to utilize a driver specific
spinlock.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 50ff18870be2..152edd1e9a23 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2043,15 +2043,10 @@ static int sh_eth_phy_start(struct net_device *ndev)
static int sh_eth_get_link_ksettings(struct net_device *ndev,
struct ethtool_link_ksettings *cmd)
{
- struct sh_eth_private *mdp = netdev_priv(ndev);
- unsigned long flags;
-
if (!ndev->phydev)
return -ENODEV;
- spin_lock_irqsave(&mdp->lock, flags);
phy_ethtool_ksettings_get(ndev->phydev, cmd);
- spin_unlock_irqrestore(&mdp->lock, flags);
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 06/14] sh_eth: remove custom .get_link_ksettings from ethtool ops
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (4 preceding siblings ...)
2018-07-04 8:14 ` [PATCH v2 05/14] sh_eth: remove useless serialization in sh_eth_get_link_ksettings() Vladimir Zapolskiy
@ 2018-07-04 8:14 ` Vladimir Zapolskiy
2018-07-06 19:08 ` Sergei Shtylyov
2018-07-04 8:14 ` [PATCH v2 07/14] sh_eth: remove custom .set_link_ksettings " Vladimir Zapolskiy
` (10 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:14 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The generic phy_ethtool_get_link_ksettings() function from phylib can be
used instead of in-house sh_eth_get_link_ksettings().
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 152edd1e9a23..bd4a0b9c3362 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2040,17 +2040,6 @@ static int sh_eth_phy_start(struct net_device *ndev)
return 0;
}
-static int sh_eth_get_link_ksettings(struct net_device *ndev,
- struct ethtool_link_ksettings *cmd)
-{
- if (!ndev->phydev)
- return -ENODEV;
-
- phy_ethtool_ksettings_get(ndev->phydev, cmd);
-
- return 0;
-}
-
static int sh_eth_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
@@ -2398,7 +2387,7 @@ static const struct ethtool_ops sh_eth_ethtool_ops = {
.get_sset_count = sh_eth_get_sset_count,
.get_ringparam = sh_eth_get_ringparam,
.set_ringparam = sh_eth_set_ringparam,
- .get_link_ksettings = sh_eth_get_link_ksettings,
+ .get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = sh_eth_set_link_ksettings,
.get_wol = sh_eth_get_wol,
.set_wol = sh_eth_set_wol,
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 07/14] sh_eth: remove custom .set_link_ksettings from ethtool ops
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (5 preceding siblings ...)
2018-07-04 8:14 ` [PATCH v2 06/14] sh_eth: remove custom .get_link_ksettings from ethtool ops Vladimir Zapolskiy
@ 2018-07-04 8:14 ` Vladimir Zapolskiy
2018-07-06 19:15 ` Sergei Shtylyov
2018-07-04 8:14 ` [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
` (9 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:14 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The generic phy_ethtool_set_link_ksettings() function from phylib can
be used instead of in-house sh_eth_set_link_ksettings().
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index bd4a0b9c3362..5614fd231bbe 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2040,15 +2040,6 @@ static int sh_eth_phy_start(struct net_device *ndev)
return 0;
}
-static int sh_eth_set_link_ksettings(struct net_device *ndev,
- const struct ethtool_link_ksettings *cmd)
-{
- if (!ndev->phydev)
- return -ENODEV;
-
- return phy_ethtool_ksettings_set(ndev->phydev, cmd);
-}
-
/* If it is ever necessary to increase SH_ETH_REG_DUMP_MAX_REGS, the
* version must be bumped as well. Just adding registers up to that
* limit is fine, as long as the existing register indices don't
@@ -2388,7 +2379,7 @@ static const struct ethtool_ops sh_eth_ethtool_ops = {
.get_ringparam = sh_eth_get_ringparam,
.set_ringparam = sh_eth_set_ringparam,
.get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = sh_eth_set_link_ksettings,
+ .set_link_ksettings = phy_ethtool_set_link_ksettings,
.get_wol = sh_eth_get_wol,
.set_wol = sh_eth_set_wol,
};
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (6 preceding siblings ...)
2018-07-04 8:14 ` [PATCH v2 07/14] sh_eth: remove custom .set_link_ksettings " Vladimir Zapolskiy
@ 2018-07-04 8:14 ` Vladimir Zapolskiy
2018-07-05 6:09 ` Vladimir Zapolskiy
2018-07-06 19:47 ` Sergei Shtylyov
2018-07-04 8:14 ` [PATCH v2 09/14] ravb: fix invalid context bug while changing link options " Vladimir Zapolskiy
` (8 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:14 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
potentially sleeping") phy_start_aneg() function utilizes a mutex
to serialize changes to phy state, however the helper function is
called in atomic context.
The bug can be reproduced by running "ethtool -r" command, the bug
is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 68f122140966..e7d6d1b6e7d6 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1152,15 +1152,10 @@ static int ravb_set_link_ksettings(struct net_device *ndev,
static int ravb_nway_reset(struct net_device *ndev)
{
- struct ravb_private *priv = netdev_priv(ndev);
int error = -ENODEV;
- unsigned long flags;
- if (ndev->phydev) {
- spin_lock_irqsave(&priv->lock, flags);
+ if (ndev->phydev)
error = phy_start_aneg(ndev->phydev);
- spin_unlock_irqrestore(&priv->lock, flags);
- }
return error;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool
2018-07-04 8:14 ` [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
@ 2018-07-05 6:09 ` Vladimir Zapolskiy
2018-07-06 19:47 ` Sergei Shtylyov
1 sibling, 0 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-05 6:09 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however the helper function is
> called in atomic context.
>
> The bug can be reproduced by running "ethtool -r" command, the bug
> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Here is an invalid commit specified, the proper tag is
Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool
@ 2018-07-05 6:09 ` Vladimir Zapolskiy
0 siblings, 0 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-05 6:09 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however the helper function is
> called in atomic context.
>
> The bug can be reproduced by running "ethtool -r" command, the bug
> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Here is an invalid commit specified, the proper tag is
Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool
2018-07-05 6:09 ` Vladimir Zapolskiy
(?)
@ 2018-07-05 8:27 ` Sergei Shtylyov
-1 siblings, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-05 8:27 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
Hello!
On 7/5/2018 9:09 AM, Vladimir Zapolskiy wrote:
>> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
>> potentially sleeping") phy_start_aneg() function utilizes a mutex
>> to serialize changes to phy state, however the helper function is
>> called in atomic context.
>>
>> The bug can be reproduced by running "ethtool -r" command, the bug
>> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
>>
>> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
>
> Here is an invalid commit specified, the proper tag is
>
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
I was just going to tell you that. :-)
> --
> Best wishes,
> Vladimir
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool
2018-07-04 8:14 ` [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
2018-07-05 6:09 ` Vladimir Zapolskiy
@ 2018-07-06 19:47 ` Sergei Shtylyov
1 sibling, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-06 19:47 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however the helper function is
> called in atomic context.
>
> The bug can be reproduced by running "ethtool -r" command, the bug
> is reported if CONFIG_DEBUG_ATOMIC_SLEEP build option is enabled.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Again, the patch order question...
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 09/14] ravb: fix invalid context bug while changing link options by ethtool
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (7 preceding siblings ...)
2018-07-04 8:14 ` [PATCH v2 08/14] ravb: fix invalid context bug while calling auto-negotiation by ethtool Vladimir Zapolskiy
@ 2018-07-04 8:14 ` Vladimir Zapolskiy
2018-07-05 6:09 ` Vladimir Zapolskiy
2018-07-06 19:58 ` Sergei Shtylyov
2018-07-04 8:16 ` [PATCH v2 10/14] ravb: simplify link auto-negotiation " Vladimir Zapolskiy
` (7 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:14 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The change fixes sleep in atomic context bug, which is encountered
every time when link settings are changed by ethtool.
Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
potentially sleeping") phy_start_aneg() function utilizes a mutex
to serialize changes to phy state, however that helper function is
called in atomic context under a grabbed spinlock, because
phy_start_aneg() is called by phy_ethtool_ksettings_set() and by
replaced phy_ethtool_sset() helpers from phylib.
Now duplex mode setting is enforced in ravb_adjust_link() only, also
now RX/TX is disabled when link is put down or modifications to E-MAC
registers ECMR and GECMR are expected for both cases of checked and
ignored link status pin state from E-MAC interrupt handler.
Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 49 ++++++++----------------
1 file changed, 15 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index e7d6d1b6e7d6..40266fe01186 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -980,6 +980,13 @@ static void ravb_adjust_link(struct net_device *ndev)
struct ravb_private *priv = netdev_priv(ndev);
struct phy_device *phydev = ndev->phydev;
bool new_state = false;
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ /* Disable TX and RX right over here, if E-MAC change is ignored */
+ if (priv->no_avb_link)
+ ravb_rcv_snd_disable(ndev);
if (phydev->link) {
if (phydev->duplex != priv->duplex) {
@@ -997,18 +1004,21 @@ static void ravb_adjust_link(struct net_device *ndev)
ravb_modify(ndev, ECMR, ECMR_TXF, 0);
new_state = true;
priv->link = phydev->link;
- if (priv->no_avb_link)
- ravb_rcv_snd_enable(ndev);
}
} else if (priv->link) {
new_state = true;
priv->link = 0;
priv->speed = 0;
priv->duplex = -1;
- if (priv->no_avb_link)
- ravb_rcv_snd_disable(ndev);
}
+ /* Enable TX and RX right over here, if E-MAC change is ignored */
+ if (priv->no_avb_link && phydev->link)
+ ravb_rcv_snd_enable(ndev);
+
+ mmiowb();
+ spin_unlock_irqrestore(&priv->lock, flags);
+
if (new_state && netif_msg_link(priv))
phy_print_status(phydev);
}
@@ -1115,39 +1125,10 @@ static int ravb_get_link_ksettings(struct net_device *ndev,
static int ravb_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
- struct ravb_private *priv = netdev_priv(ndev);
- unsigned long flags;
- int error;
-
if (!ndev->phydev)
return -ENODEV;
- spin_lock_irqsave(&priv->lock, flags);
-
- /* Disable TX and RX */
- ravb_rcv_snd_disable(ndev);
-
- error = phy_ethtool_ksettings_set(ndev->phydev, cmd);
- if (error)
- goto error_exit;
-
- if (cmd->base.duplex == DUPLEX_FULL)
- priv->duplex = 1;
- else
- priv->duplex = 0;
-
- ravb_set_duplex(ndev);
-
-error_exit:
- mdelay(1);
-
- /* Enable TX and RX */
- ravb_rcv_snd_enable(ndev);
-
- mmiowb();
- spin_unlock_irqrestore(&priv->lock, flags);
-
- return error;
+ return phy_ethtool_ksettings_set(ndev->phydev, cmd);
}
static int ravb_nway_reset(struct net_device *ndev)
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 09/14] ravb: fix invalid context bug while changing link options by ethtool
2018-07-04 8:14 ` [PATCH v2 09/14] ravb: fix invalid context bug while changing link options " Vladimir Zapolskiy
@ 2018-07-05 6:09 ` Vladimir Zapolskiy
2018-07-06 19:58 ` Sergei Shtylyov
1 sibling, 0 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-05 6:09 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> The change fixes sleep in atomic context bug, which is encountered
> every time when link settings are changed by ethtool.
>
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however that helper function is
> called in atomic context under a grabbed spinlock, because
> phy_start_aneg() is called by phy_ethtool_ksettings_set() and by
> replaced phy_ethtool_sset() helpers from phylib.
>
> Now duplex mode setting is enforced in ravb_adjust_link() only, also
> now RX/TX is disabled when link is put down or modifications to E-MAC
> registers ECMR and GECMR are expected for both cases of checked and
> ignored link status pin state from E-MAC interrupt handler.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Here is an invalid commit specified, the proper tag is
Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 09/14] ravb: fix invalid context bug while changing link options by ethtool
@ 2018-07-05 6:09 ` Vladimir Zapolskiy
0 siblings, 0 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-05 6:09 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> The change fixes sleep in atomic context bug, which is encountered
> every time when link settings are changed by ethtool.
>
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however that helper function is
> called in atomic context under a grabbed spinlock, because
> phy_start_aneg() is called by phy_ethtool_ksettings_set() and by
> replaced phy_ethtool_sset() helpers from phylib.
>
> Now duplex mode setting is enforced in ravb_adjust_link() only, also
> now RX/TX is disabled when link is put down or modifications to E-MAC
> registers ECMR and GECMR are expected for both cases of checked and
> ignored link status pin state from E-MAC interrupt handler.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Here is an invalid commit specified, the proper tag is
Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 09/14] ravb: fix invalid context bug while changing link options by ethtool
2018-07-04 8:14 ` [PATCH v2 09/14] ravb: fix invalid context bug while changing link options " Vladimir Zapolskiy
2018-07-05 6:09 ` Vladimir Zapolskiy
@ 2018-07-06 19:58 ` Sergei Shtylyov
1 sibling, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-06 19:58 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:14 AM, Vladimir Zapolskiy wrote:
> The change fixes sleep in atomic context bug, which is encountered
> every time when link settings are changed by ethtool.
>
> Since commit 35b5f6b1a82b ("PHYLIB: Locking fixes for PHY I/O
> potentially sleeping") phy_start_aneg() function utilizes a mutex
> to serialize changes to phy state, however that helper function is
> called in atomic context under a grabbed spinlock, because
> phy_start_aneg() is called by phy_ethtool_ksettings_set() and by
> replaced phy_ethtool_sset() helpers from phylib.
>
> Now duplex mode setting is enforced in ravb_adjust_link() only, also
> now RX/TX is disabled when link is put down or modifications to E-MAC
> registers ECMR and GECMR are expected for both cases of checked and
> ignored link status pin state from E-MAC interrupt handler.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
Wrong commit, as already noted...
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 10/14] ravb: simplify link auto-negotiation by ethtool
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (8 preceding siblings ...)
2018-07-04 8:14 ` [PATCH v2 09/14] ravb: fix invalid context bug while changing link options " Vladimir Zapolskiy
@ 2018-07-04 8:16 ` Vladimir Zapolskiy
2018-07-06 20:30 ` Sergei Shtylyov
2018-07-04 8:16 ` [PATCH v2 11/14] ravb: remove custom .nway_reset from ethtool ops Vladimir Zapolskiy
` (6 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:16 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
There is no need to call a heavyweight phy_start_aneg() for phy
auto-negotiation by ethtool, the phy is already initialized and
link auto-negotiation is started by calling phy_start() from
ravb_phy_start() when a network device is opened.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 40266fe01186..31913a469001 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1136,7 +1136,7 @@ static int ravb_nway_reset(struct net_device *ndev)
int error = -ENODEV;
if (ndev->phydev)
- error = phy_start_aneg(ndev->phydev);
+ error = phy_restart_aneg(ndev->phydev);
return error;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 10/14] ravb: simplify link auto-negotiation by ethtool
2018-07-04 8:16 ` [PATCH v2 10/14] ravb: simplify link auto-negotiation " Vladimir Zapolskiy
@ 2018-07-06 20:30 ` Sergei Shtylyov
0 siblings, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-06 20:30 UTC (permalink / raw)
To: Vladimir Zapolskiy, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
On 07/04/2018 11:16 AM, Vladimir Zapolskiy wrote:
> There is no need to call a heavyweight phy_start_aneg() for phy
> auto-negotiation by ethtool, the phy is already initialized and
> link auto-negotiation is started by calling phy_start() from
> ravb_phy_start() when a network device is opened.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
[...]
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v2 11/14] ravb: remove custom .nway_reset from ethtool ops
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (9 preceding siblings ...)
2018-07-04 8:16 ` [PATCH v2 10/14] ravb: simplify link auto-negotiation " Vladimir Zapolskiy
@ 2018-07-04 8:16 ` Vladimir Zapolskiy
2018-07-06 20:30 ` Sergei Shtylyov
2018-07-04 8:16 ` [PATCH v2 12/14] ravb: remove useless serialization in ravb_get_link_ksettings() Vladimir Zapolskiy
` (5 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:16 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The generic phy_ethtool_nway_reset() function from phylib can be used
instead of in-house ravb_nway_reset().
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 31913a469001..6002132093cd 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1131,16 +1131,6 @@ static int ravb_set_link_ksettings(struct net_device *ndev,
return phy_ethtool_ksettings_set(ndev->phydev, cmd);
}
-static int ravb_nway_reset(struct net_device *ndev)
-{
- int error = -ENODEV;
-
- if (ndev->phydev)
- error = phy_restart_aneg(ndev->phydev);
-
- return error;
-}
-
static u32 ravb_get_msglevel(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -1353,7 +1343,7 @@ static int ravb_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
}
static const struct ethtool_ops ravb_ethtool_ops = {
- .nway_reset = ravb_nway_reset,
+ .nway_reset = phy_ethtool_nway_reset,
.get_msglevel = ravb_get_msglevel,
.set_msglevel = ravb_set_msglevel,
.get_link = ethtool_op_get_link,
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 12/14] ravb: remove useless serialization in ravb_get_link_ksettings()
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (10 preceding siblings ...)
2018-07-04 8:16 ` [PATCH v2 11/14] ravb: remove custom .nway_reset from ethtool ops Vladimir Zapolskiy
@ 2018-07-04 8:16 ` Vladimir Zapolskiy
2018-07-06 20:31 ` Sergei Shtylyov
2018-07-04 8:16 ` [PATCH v2 13/14] ravb: remove custom .get_link_ksettings from ethtool ops Vladimir Zapolskiy
` (4 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:16 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
phy_ethtool_ksettings_get() call does not modify device state or device
driver state, hence there is no need to utilize a driver specific
spinlock.
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 6002132093cd..772687a5faee 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1109,15 +1109,10 @@ static int ravb_phy_start(struct net_device *ndev)
static int ravb_get_link_ksettings(struct net_device *ndev,
struct ethtool_link_ksettings *cmd)
{
- struct ravb_private *priv = netdev_priv(ndev);
- unsigned long flags;
-
if (!ndev->phydev)
return -ENODEV;
- spin_lock_irqsave(&priv->lock, flags);
phy_ethtool_ksettings_get(ndev->phydev, cmd);
- spin_unlock_irqrestore(&priv->lock, flags);
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 13/14] ravb: remove custom .get_link_ksettings from ethtool ops
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (11 preceding siblings ...)
2018-07-04 8:16 ` [PATCH v2 12/14] ravb: remove useless serialization in ravb_get_link_ksettings() Vladimir Zapolskiy
@ 2018-07-04 8:16 ` Vladimir Zapolskiy
2018-07-06 20:39 ` Sergei Shtylyov
2018-07-04 8:16 ` [PATCH v2 14/14] ravb: remove custom .set_link_ksettings " Vladimir Zapolskiy
` (3 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:16 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The generic phy_ethtool_get_link_ksettings() function from phylib can be
used instead of in-house ravb_get_link_ksettings().
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 772687a5faee..9fe01259be6f 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1106,17 +1106,6 @@ static int ravb_phy_start(struct net_device *ndev)
return 0;
}
-static int ravb_get_link_ksettings(struct net_device *ndev,
- struct ethtool_link_ksettings *cmd)
-{
- if (!ndev->phydev)
- return -ENODEV;
-
- phy_ethtool_ksettings_get(ndev->phydev, cmd);
-
- return 0;
-}
-
static int ravb_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
@@ -1348,7 +1337,7 @@ static const struct ethtool_ops ravb_ethtool_ops = {
.get_ringparam = ravb_get_ringparam,
.set_ringparam = ravb_set_ringparam,
.get_ts_info = ravb_get_ts_info,
- .get_link_ksettings = ravb_get_link_ksettings,
+ .get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = ravb_set_link_ksettings,
.get_wol = ravb_get_wol,
.set_wol = ravb_set_wol,
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v2 14/14] ravb: remove custom .set_link_ksettings from ethtool ops
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (12 preceding siblings ...)
2018-07-04 8:16 ` [PATCH v2 13/14] ravb: remove custom .get_link_ksettings from ethtool ops Vladimir Zapolskiy
@ 2018-07-04 8:16 ` Vladimir Zapolskiy
2018-07-06 20:41 ` Sergei Shtylyov
2018-07-05 0:56 ` [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers David Miller
` (2 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-04 8:16 UTC (permalink / raw)
To: Sergei Shtylyov, David S . Miller
Cc: Andrew Lunn, Geert Uytterhoeven, netdev, linux-renesas-soc
The generic phy_ethtool_set_link_ksettings() function from phylib can
be used instead of in-house ravb_set_link_ksettings().
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---
drivers/net/ethernet/renesas/ravb_main.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9fe01259be6f..0d811c02ff34 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1106,15 +1106,6 @@ static int ravb_phy_start(struct net_device *ndev)
return 0;
}
-static int ravb_set_link_ksettings(struct net_device *ndev,
- const struct ethtool_link_ksettings *cmd)
-{
- if (!ndev->phydev)
- return -ENODEV;
-
- return phy_ethtool_ksettings_set(ndev->phydev, cmd);
-}
-
static u32 ravb_get_msglevel(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -1338,7 +1329,7 @@ static const struct ethtool_ops ravb_ethtool_ops = {
.set_ringparam = ravb_set_ringparam,
.get_ts_info = ravb_get_ts_info,
.get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = ravb_set_link_ksettings,
+ .set_link_ksettings = phy_ethtool_set_link_ksettings,
.get_wol = ravb_get_wol,
.set_wol = ravb_set_wol,
};
--
2.17.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (13 preceding siblings ...)
2018-07-04 8:16 ` [PATCH v2 14/14] ravb: remove custom .set_link_ksettings " Vladimir Zapolskiy
@ 2018-07-05 0:56 ` David Miller
2018-07-05 5:59 ` Vladimir Zapolskiy
2018-07-05 7:21 ` Geert Uytterhoeven
2018-07-07 1:47 ` David Miller
16 siblings, 1 reply; 45+ messages in thread
From: David Miller @ 2018-07-05 0:56 UTC (permalink / raw)
To: vladimir_zapolskiy
Cc: sergei.shtylyov, andrew, geert, netdev, linux-renesas-soc
From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date: Wed, 4 Jul 2018 11:12:38 +0300
> For ages trivial changes to RAVB and SuperH ethernet links by means of
> standard 'ethtool' trigger a 'sleeping function called from invalid
> context' bug, to visualize it on r8a7795 ULCB:
...
> The root cause is that an attempt to modify ECMR and GECMR registers
> only when RX/TX function is disabled was too overcomplicated in its
> original implementation, also processing of an optional Link Change
> interrupt added even more complexity, as a result the implementation
> was error prone.
>
> The new locking scheme is confirmed to be correct by dumping driver
> specific and generic PHY framework function calls with aid of ftrace
> while running more or less advanced tests.
>
> Please note that sh_eth patches from the series were built-tested only.
>
> On purpose I do not add Fixes tags, the reused PHY handlers were added
> way later than the fixed problems were firstly found in the drivers.
>
> Changes from v1 to v2:
> * the original patches are split to bugfixes and enhancements only,
> both v1 and v2 series are absolutely equal in total, thus I omit
> description of changes in individual patches,
> * the latter implies that there should be no strict need for retesting,
> but because formally two series are different, I have to drop the tags
> given by Geert and Andrew, please send your tags again.
These changes look fine to me but I want to see some reviews and/or
testing before I apply them.
Thanks.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-05 0:56 ` [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers David Miller
@ 2018-07-05 5:59 ` Vladimir Zapolskiy
0 siblings, 0 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-05 5:59 UTC (permalink / raw)
To: David Miller; +Cc: sergei.shtylyov, andrew, geert, netdev, linux-renesas-soc
Hi David,
On 07/05/2018 03:56 AM, David Miller wrote:
> From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Date: Wed, 4 Jul 2018 11:12:38 +0300
>
>> For ages trivial changes to RAVB and SuperH ethernet links by means of
>> standard 'ethtool' trigger a 'sleeping function called from invalid
>> context' bug, to visualize it on r8a7795 ULCB:
> ...
>> The root cause is that an attempt to modify ECMR and GECMR registers
>> only when RX/TX function is disabled was too overcomplicated in its
>> original implementation, also processing of an optional Link Change
>> interrupt added even more complexity, as a result the implementation
>> was error prone.
>>
>> The new locking scheme is confirmed to be correct by dumping driver
>> specific and generic PHY framework function calls with aid of ftrace
>> while running more or less advanced tests.
>>
>> Please note that sh_eth patches from the series were built-tested only.
>>
>> On purpose I do not add Fixes tags, the reused PHY handlers were added
>> way later than the fixed problems were firstly found in the drivers.
>>
>> Changes from v1 to v2:
>> * the original patches are split to bugfixes and enhancements only,
>> both v1 and v2 series are absolutely equal in total, thus I omit
>> description of changes in individual patches,
>> * the latter implies that there should be no strict need for retesting,
>> but because formally two series are different, I have to drop the tags
>> given by Geert and Andrew, please send your tags again.
>
> These changes look fine to me but I want to see some reviews and/or
> testing before I apply them.
>
Thanks to Geert for finding time to test v1 series, the sums of changes
are word for word, https://www.spinics.net/lists/linux-renesas-soc/msg28666.html
Andrew also gave a number of Reviewed-by tags, but they are not directly
applicable to new patches, unfortunately.
In any case let's wait for scrupulous review completed by Sergei, I believe
he'd like to contribute to the review process, and Sergei may highlight more
shortcomings.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
@ 2018-07-05 5:59 ` Vladimir Zapolskiy
0 siblings, 0 replies; 45+ messages in thread
From: Vladimir Zapolskiy @ 2018-07-05 5:59 UTC (permalink / raw)
To: David Miller; +Cc: sergei.shtylyov, andrew, geert, netdev, linux-renesas-soc
Hi David,
On 07/05/2018 03:56 AM, David Miller wrote:
> From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Date: Wed, 4 Jul 2018 11:12:38 +0300
>
>> For ages trivial changes to RAVB and SuperH ethernet links by means of
>> standard 'ethtool' trigger a 'sleeping function called from invalid
>> context' bug, to visualize it on r8a7795 ULCB:
> ...
>> The root cause is that an attempt to modify ECMR and GECMR registers
>> only when RX/TX function is disabled was too overcomplicated in its
>> original implementation, also processing of an optional Link Change
>> interrupt added even more complexity, as a result the implementation
>> was error prone.
>>
>> The new locking scheme is confirmed to be correct by dumping driver
>> specific and generic PHY framework function calls with aid of ftrace
>> while running more or less advanced tests.
>>
>> Please note that sh_eth patches from the series were built-tested only.
>>
>> On purpose I do not add Fixes tags, the reused PHY handlers were added
>> way later than the fixed problems were firstly found in the drivers.
>>
>> Changes from v1 to v2:
>> * the original patches are split to bugfixes and enhancements only,
>> both v1 and v2 series are absolutely equal in total, thus I omit
>> description of changes in individual patches,
>> * the latter implies that there should be no strict need for retesting,
>> but because formally two series are different, I have to drop the tags
>> given by Geert and Andrew, please send your tags again.
>
> These changes look fine to me but I want to see some reviews and/or
> testing before I apply them.
>
Thanks to Geert for finding time to test v1 series, the sums of changes
are word for word, https://www.spinics.net/lists/linux-renesas-soc/msg28666.html
Andrew also gave a number of Reviewed-by tags, but they are not directly
applicable to new patches, unfortunately.
In any case let's wait for scrupulous review completed by Sergei, I believe
he'd like to contribute to the review process, and Sergei may highlight more
shortcomings.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-05 5:59 ` Vladimir Zapolskiy
(?)
@ 2018-07-05 19:13 ` Sergei Shtylyov
-1 siblings, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-05 19:13 UTC (permalink / raw)
To: Vladimir Zapolskiy, David Miller; +Cc: andrew, geert, netdev, linux-renesas-soc
On 07/05/2018 08:59 AM, Vladimir Zapolskiy wrote:
>>> For ages trivial changes to RAVB and SuperH ethernet links by means of
>>> standard 'ethtool' trigger a 'sleeping function called from invalid
>>> context' bug, to visualize it on r8a7795 ULCB:
>> ...
>>> The root cause is that an attempt to modify ECMR and GECMR registers
>>> only when RX/TX function is disabled was too overcomplicated in its
>>> original implementation, also processing of an optional Link Change
>>> interrupt added even more complexity, as a result the implementation
>>> was error prone.
>>>
>>> The new locking scheme is confirmed to be correct by dumping driver
>>> specific and generic PHY framework function calls with aid of ftrace
>>> while running more or less advanced tests.
>>>
>>> Please note that sh_eth patches from the series were built-tested only.
>>>
>>> On purpose I do not add Fixes tags, the reused PHY handlers were added
>>> way later than the fixed problems were firstly found in the drivers.
>>>
>>> Changes from v1 to v2:
>>> * the original patches are split to bugfixes and enhancements only,
>>> both v1 and v2 series are absolutely equal in total, thus I omit
>>> description of changes in individual patches,
>>> * the latter implies that there should be no strict need for retesting,
>>> but because formally two series are different, I have to drop the tags
>>> given by Geert and Andrew, please send your tags again.
>>
>> These changes look fine to me but I want to see some reviews and/or
>> testing before I apply them.
>>
>
> Thanks to Geert for finding time to test v1 series, the sums of changes
> are word for word, https://www.spinics.net/lists/linux-renesas-soc/msg28666.html
I might find a time to test the sh_eth patches on the R-Car V3H Starter Kit board;
I'm still on vacations, so don't have access to the R-Car gen2 boards at home...
> Andrew also gave a number of Reviewed-by tags, but they are not directly
> applicable to new patches, unfortunately.
>
> In any case let's wait for scrupulous review completed by Sergei, I believe
> he'd like to contribute to the review process, and Sergei may highlight more
> shortcomings.
Heh. I'm surprised DaveM haven't lectured you about not mixing fixes and cleanups
in the single series -- they go to his different 2 trees, net.git and net-next.git...
If you need your cleanups based on your fixes, you have to indicate that in the cleanups
patchset...
> --
> Best wishes,
> Vladimir
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (14 preceding siblings ...)
2018-07-05 0:56 ` [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers David Miller
@ 2018-07-05 7:21 ` Geert Uytterhoeven
2018-07-07 1:47 ` David Miller
16 siblings, 0 replies; 45+ messages in thread
From: Geert Uytterhoeven @ 2018-07-05 7:21 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Sergei Shtylyov, David S. Miller, Andrew Lunn, netdev, Linux-Renesas
Hi Vladimir,
On Wed, Jul 4, 2018 at 10:13 AM Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
> For ages trivial changes to RAVB and SuperH ethernet links by means of
> standard 'ethtool' trigger a 'sleeping function called from invalid
> context' bug, to visualize it on r8a7795 ULCB:
>
> % ethtool -r eth0
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:747
> in_atomic(): 1, irqs_disabled(): 128, pid: 554, name: ethtool
> INFO: lockdep is turned off.
> irq event stamp: 0
> hardirqs last enabled at (0): [<0000000000000000>] (null)
> hardirqs last disabled at (0): [<ffff0000080e1d3c>] copy_process.isra.7.part.8+0x2cc/0x1918
> softirqs last enabled at (0): [<ffff0000080e1d3c>] copy_process.isra.7.part.8+0x2cc/0x1918
> softirqs last disabled at (0): [<0000000000000000>] (null)
> CPU: 5 PID: 554 Comm: ethtool Not tainted 4.17.0-rc4-arm64-renesas+ #33
> Hardware name: Renesas H3ULCB board based on r8a7795 ES2.0+ (DT)
> Call trace:
> dump_backtrace+0x0/0x198
> show_stack+0x24/0x30
> dump_stack+0xb8/0xf4
> ___might_sleep+0x1c8/0x1f8
> __might_sleep+0x58/0x90
> __mutex_lock+0x50/0x890
> mutex_lock_nested+0x3c/0x50
> phy_start_aneg_priv+0x38/0x180
> phy_start_aneg+0x24/0x30
> ravb_nway_reset+0x3c/0x68
> dev_ethtool+0x3dc/0x2338
> dev_ioctl+0x19c/0x490
> sock_do_ioctl+0xe0/0x238
> sock_ioctl+0x254/0x460
> do_vfs_ioctl+0xb0/0x918
> ksys_ioctl+0x50/0x80
> sys_ioctl+0x34/0x48
> __sys_trace_return+0x0/0x4
>
> The root cause is that an attempt to modify ECMR and GECMR registers
> only when RX/TX function is disabled was too overcomplicated in its
> original implementation, also processing of an optional Link Change
> interrupt added even more complexity, as a result the implementation
> was error prone.
>
> The new locking scheme is confirmed to be correct by dumping driver
> specific and generic PHY framework function calls with aid of ftrace
> while running more or less advanced tests.
>
> Please note that sh_eth patches from the series were built-tested only.
>
> On purpose I do not add Fixes tags, the reused PHY handlers were added
> way later than the fixed problems were firstly found in the drivers.
>
> Changes from v1 to v2:
> * the original patches are split to bugfixes and enhancements only,
> both v1 and v2 series are absolutely equal in total, thus I omit
> description of changes in individual patches,
> * the latter implies that there should be no strict need for retesting,
> but because formally two series are different, I have to drop the tags
> given by Geert and Andrew, please send your tags again.
Thank for your series!
I've retested this on both R-Car M2-W (sh_eth) and R-Car H3 ES2.0 (ravb),
and the BUG disappeared.
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-04 8:12 [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers Vladimir Zapolskiy
` (15 preceding siblings ...)
2018-07-05 7:21 ` Geert Uytterhoeven
@ 2018-07-07 1:47 ` David Miller
2018-07-07 15:22 ` Sergei Shtylyov
16 siblings, 1 reply; 45+ messages in thread
From: David Miller @ 2018-07-07 1:47 UTC (permalink / raw)
To: vladimir_zapolskiy
Cc: sergei.shtylyov, andrew, geert, netdev, linux-renesas-soc
From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Date: Wed, 4 Jul 2018 11:12:38 +0300
> For ages trivial changes to RAVB and SuperH ethernet links by means of
> standard 'ethtool' trigger a 'sleeping function called from invalid
> context' bug, to visualize it on r8a7795 ULCB:
...
> The root cause is that an attempt to modify ECMR and GECMR registers
> only when RX/TX function is disabled was too overcomplicated in its
> original implementation, also processing of an optional Link Change
> interrupt added even more complexity, as a result the implementation
> was error prone.
>
> The new locking scheme is confirmed to be correct by dumping driver
> specific and generic PHY framework function calls with aid of ftrace
> while running more or less advanced tests.
>
> Please note that sh_eth patches from the series were built-tested only.
>
> On purpose I do not add Fixes tags, the reused PHY handlers were added
> way later than the fixed problems were firstly found in the drivers.
>
> Changes from v1 to v2:
> * the original patches are split to bugfixes and enhancements only,
> both v1 and v2 series are absolutely equal in total, thus I omit
> description of changes in individual patches,
> * the latter implies that there should be no strict need for retesting,
> but because formally two series are different, I have to drop the tags
> given by Geert and Andrew, please send your tags again.
Series applied with Fixes: tags of patches 8 and 9 fixed up.
Thanks.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-07 1:47 ` David Miller
@ 2018-07-07 15:22 ` Sergei Shtylyov
2018-07-07 23:58 ` David Miller
0 siblings, 1 reply; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-07 15:22 UTC (permalink / raw)
To: David Miller, vladimir_zapolskiy; +Cc: andrew, geert, netdev, linux-renesas-soc
On 7/7/2018 4:47 AM, David Miller wrote:
>> For ages trivial changes to RAVB and SuperH ethernet links by means of
>> standard 'ethtool' trigger a 'sleeping function called from invalid
>> context' bug, to visualize it on r8a7795 ULCB:
> ...
>> The root cause is that an attempt to modify ECMR and GECMR registers
>> only when RX/TX function is disabled was too overcomplicated in its
>> original implementation, also processing of an optional Link Change
>> interrupt added even more complexity, as a result the implementation
>> was error prone.
>>
>> The new locking scheme is confirmed to be correct by dumping driver
>> specific and generic PHY framework function calls with aid of ftrace
>> while running more or less advanced tests.
>>
>> Please note that sh_eth patches from the series were built-tested only.
>>
>> On purpose I do not add Fixes tags, the reused PHY handlers were added
>> way later than the fixed problems were firstly found in the drivers.
>>
>> Changes from v1 to v2:
>> * the original patches are split to bugfixes and enhancements only,
>> both v1 and v2 series are absolutely equal in total, thus I omit
>> description of changes in individual patches,
>> * the latter implies that there should be no strict need for retesting,
>> but because formally two series are different, I have to drop the tags
>> given by Geert and Andrew, please send your tags again.
>
> Series applied with Fixes: tags of patches 8 and 9 fixed up.
So you applied the whole series to net.git... that was somewhat
unexpected, at least by me. Care to share your reasoning?
> Thanks.
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-07 15:22 ` Sergei Shtylyov
@ 2018-07-07 23:58 ` David Miller
2018-07-08 23:24 ` Sergei Shtylyov
0 siblings, 1 reply; 45+ messages in thread
From: David Miller @ 2018-07-07 23:58 UTC (permalink / raw)
To: sergei.shtylyov
Cc: vladimir_zapolskiy, andrew, geert, netdev, linux-renesas-soc
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 7 Jul 2018 18:22:08 +0300
> So you applied the whole series to net.git... that was somewhat
> unexpected, at least by me. Care to share your reasoning?
It's fixes a sleep in atomic which is a serious bug.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-07 23:58 ` David Miller
@ 2018-07-08 23:24 ` Sergei Shtylyov
2018-07-08 23:33 ` Sergei Shtylyov
2018-07-09 18:16 ` David Miller
0 siblings, 2 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-08 23:24 UTC (permalink / raw)
To: David Miller; +Cc: vladimir_zapolskiy, andrew, geert, netdev, linux-renesas-soc
Hello!
On 7/8/2018 2:58 AM, David Miller wrote:
>> So you applied the whole series to net.git... that was somewhat
>> unexpected, at least by me. Care to share your reasoning?
>
> It's fixes a sleep in atomic which is a serious bug.
Do you realize that only patches 0, 1, 8, and 9 were real fixes, others
were just follow-up cleanups? (Although the wording of the cover letter could
have made people think that the whole series is the fixes...)
OK, I understand what's done is done and it's not me that would have to
justify the cleanups in a middle of the 4.18-rc's. But still, it was not a
pleasant surprise (and I told Vladimir he shouldn't mix the real fixes and
cleanups in reply to his cover letter, IIRC).
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-08 23:24 ` Sergei Shtylyov
@ 2018-07-08 23:33 ` Sergei Shtylyov
2018-07-09 18:16 ` David Miller
1 sibling, 0 replies; 45+ messages in thread
From: Sergei Shtylyov @ 2018-07-08 23:33 UTC (permalink / raw)
To: David Miller; +Cc: vladimir_zapolskiy, andrew, geert, netdev, linux-renesas-soc
On 7/9/2018 2:24 AM, Sergei Shtylyov wrote:
>>> So you applied the whole series to net.git... that was somewhat
>>> unexpected, at least by me. Care to share your reasoning?
>>
>> It's fixes a sleep in atomic which is a serious bug.
>
> Do you realize that only patches 0, 1, 8, and 9 were real fixes, others
1, 2, 8, and 9, of course. :-)
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
2018-07-08 23:24 ` Sergei Shtylyov
2018-07-08 23:33 ` Sergei Shtylyov
@ 2018-07-09 18:16 ` David Miller
1 sibling, 0 replies; 45+ messages in thread
From: David Miller @ 2018-07-09 18:16 UTC (permalink / raw)
To: sergei.shtylyov
Cc: vladimir_zapolskiy, andrew, geert, netdev, linux-renesas-soc
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Mon, 9 Jul 2018 02:24:45 +0300
> Hello!
>
> On 7/8/2018 2:58 AM, David Miller wrote:
>
>>> So you applied the whole series to net.git... that was somewhat
>>> unexpected, at least by me. Care to share your reasoning?
>> It's fixes a sleep in atomic which is a serious bug.
>
> Do you realize that only patches 0, 1, 8, and 9 were real fixes,
> others were just follow-up cleanups? (Although the wording of the
> cover letter could have made people think that the whole series is the
> fixes...)
> OK, I understand what's done is done and it's not me that would have
> to justify the cleanups in a middle of the 4.18-rc's. But still, it
> was not a pleasant surprise (and I told Vladimir he shouldn't mix the
> real fixes and
> cleanups in reply to his cover letter, IIRC).
I was disappointed the series was so large as well.
Let's try to work together to handle this better next time, ok?
^ permalink raw reply [flat|nested] 45+ messages in thread