* [PATCH v2] igb: Report speed and duplex as unknown when device is runtime suspended
@ 2020-05-05 4:01 ` Kai-Heng Feng
0 siblings, 0 replies; 4+ messages in thread
From: Kai-Heng Feng @ 2020-05-05 4:01 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: Kai-Heng Feng, Aaron Brown, David S. Miller,
moderated list:INTEL ETHERNET DRIVERS,
open list:NETWORKING DRIVERS, open list
igb device gets runtime suspended when there's no link partner. We can't
get correct speed under that state:
$ cat /sys/class/net/enp3s0/speed
1000
In addition to that, an error can also be spotted in dmesg:
[ 385.991957] igb 0000:03:00.0 enp3s0: PCIe link lost
Since device can only be runtime suspended when there's no link partner,
we can skip reading register and let the following logic set speed and
duplex with correct status.
The more generic approach will be wrap get_link_ksettings() with begin()
and complete() callbacks. However, for this particular issue, begin()
calls igb_runtime_resume() , which tries to rtnl_lock() while the lock
is already hold by upper ethtool layer.
So let's take this approach until the igb_runtime_resume() no longer
needs to hold rtnl_lock.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Aaron Brown <aaron.f.brown@intel.com>
Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Don't early return the routine so other info can be set.
drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 39d3b76a6f5d..2cd003c5ad43 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -143,7 +143,8 @@ static int igb_get_link_ksettings(struct net_device *netdev,
u32 speed;
u32 supported, advertising;
- status = rd32(E1000_STATUS);
+ status = pm_runtime_suspended(&adapter->pdev->dev) ?
+ 0 : rd32(E1000_STATUS);
if (hw->phy.media_type == e1000_media_type_copper) {
supported = (SUPPORTED_10baseT_Half |
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Intel-wired-lan] [PATCH v2] igb: Report speed and duplex as unknown when device is runtime suspended
@ 2020-05-05 4:01 ` Kai-Heng Feng
0 siblings, 0 replies; 4+ messages in thread
From: Kai-Heng Feng @ 2020-05-05 4:01 UTC (permalink / raw)
To: intel-wired-lan
igb device gets runtime suspended when there's no link partner. We can't
get correct speed under that state:
$ cat /sys/class/net/enp3s0/speed
1000
In addition to that, an error can also be spotted in dmesg:
[ 385.991957] igb 0000:03:00.0 enp3s0: PCIe link lost
Since device can only be runtime suspended when there's no link partner,
we can skip reading register and let the following logic set speed and
duplex with correct status.
The more generic approach will be wrap get_link_ksettings() with begin()
and complete() callbacks. However, for this particular issue, begin()
calls igb_runtime_resume() , which tries to rtnl_lock() while the lock
is already hold by upper ethtool layer.
So let's take this approach until the igb_runtime_resume() no longer
needs to hold rtnl_lock.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Aaron Brown <aaron.f.brown@intel.com>
Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Don't early return the routine so other info can be set.
drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 39d3b76a6f5d..2cd003c5ad43 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -143,7 +143,8 @@ static int igb_get_link_ksettings(struct net_device *netdev,
u32 speed;
u32 supported, advertising;
- status = rd32(E1000_STATUS);
+ status = pm_runtime_suspended(&adapter->pdev->dev) ?
+ 0 : rd32(E1000_STATUS);
if (hw->phy.media_type == e1000_media_type_copper) {
supported = (SUPPORTED_10baseT_Half |
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v2] igb: Report speed and duplex as unknown when device is runtime suspended
2020-05-05 4:01 ` [Intel-wired-lan] " Kai-Heng Feng
@ 2020-05-11 23:14 ` Brown, Aaron F
-1 siblings, 0 replies; 4+ messages in thread
From: Brown, Aaron F @ 2020-05-11 23:14 UTC (permalink / raw)
To: Kai-Heng Feng, Kirsher, Jeffrey T
Cc: David S. Miller, moderated list:INTEL ETHERNET DRIVERS,
open list:NETWORKING DRIVERS, open list
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Monday, May 4, 2020 9:02 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>; Brown, Aaron F
> <aaron.f.brown@intel.com>; David S. Miller <davem@davemloft.net>;
> moderated list:INTEL ETHERNET DRIVERS <intel-wired-lan@lists.osuosl.org>;
> open list:NETWORKING DRIVERS <netdev@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>
> Subject: [PATCH v2] igb: Report speed and duplex as unknown when device is
> runtime suspended
>
> igb device gets runtime suspended when there's no link partner. We can't
> get correct speed under that state:
> $ cat /sys/class/net/enp3s0/speed
> 1000
>
> In addition to that, an error can also be spotted in dmesg:
> [ 385.991957] igb 0000:03:00.0 enp3s0: PCIe link lost
>
> Since device can only be runtime suspended when there's no link partner,
> we can skip reading register and let the following logic set speed and
> duplex with correct status.
>
> The more generic approach will be wrap get_link_ksettings() with begin()
> and complete() callbacks. However, for this particular issue, begin()
> calls igb_runtime_resume() , which tries to rtnl_lock() while the lock
> is already hold by upper ethtool layer.
>
> So let's take this approach until the igb_runtime_resume() no longer
> needs to hold rtnl_lock.
>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Aaron Brown <aaron.f.brown@intel.com>
> Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Don't early return the routine so other info can be set.
>
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-wired-lan] [PATCH v2] igb: Report speed and duplex as unknown when device is runtime suspended
@ 2020-05-11 23:14 ` Brown, Aaron F
0 siblings, 0 replies; 4+ messages in thread
From: Brown, Aaron F @ 2020-05-11 23:14 UTC (permalink / raw)
To: intel-wired-lan
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Monday, May 4, 2020 9:02 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>; Brown, Aaron F
> <aaron.f.brown@intel.com>; David S. Miller <davem@davemloft.net>;
> moderated list:INTEL ETHERNET DRIVERS <intel-wired-lan@lists.osuosl.org>;
> open list:NETWORKING DRIVERS <netdev@vger.kernel.org>; open list <linux-
> kernel at vger.kernel.org>
> Subject: [PATCH v2] igb: Report speed and duplex as unknown when device is
> runtime suspended
>
> igb device gets runtime suspended when there's no link partner. We can't
> get correct speed under that state:
> $ cat /sys/class/net/enp3s0/speed
> 1000
>
> In addition to that, an error can also be spotted in dmesg:
> [ 385.991957] igb 0000:03:00.0 enp3s0: PCIe link lost
>
> Since device can only be runtime suspended when there's no link partner,
> we can skip reading register and let the following logic set speed and
> duplex with correct status.
>
> The more generic approach will be wrap get_link_ksettings() with begin()
> and complete() callbacks. However, for this particular issue, begin()
> calls igb_runtime_resume() , which tries to rtnl_lock() while the lock
> is already hold by upper ethtool layer.
>
> So let's take this approach until the igb_runtime_resume() no longer
> needs to hold rtnl_lock.
>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Aaron Brown <aaron.f.brown@intel.com>
> Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Don't early return the routine so other info can be set.
>
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-11 23:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 4:01 [PATCH v2] igb: Report speed and duplex as unknown when device is runtime suspended Kai-Heng Feng
2020-05-05 4:01 ` [Intel-wired-lan] " Kai-Heng Feng
2020-05-11 23:14 ` Brown, Aaron F
2020-05-11 23:14 ` [Intel-wired-lan] " Brown, Aaron F
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.