RE: [Intel-wired-lan] [v4][PATCH] e1000e: continue to init phy even when failed to disable ULP
> From: Intel-wired-lan On Behalf Of
> Aaron Ma
> Sent: Wednesday, June 17, 2020 11:55 PM
> To: k...@kernel.org; Kirsher, Jeffrey T ;
> da...@davemloft.net; intel-wired-...@lists.osuosl.org;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Lifshits, Vitaly
> ; kai.heng.f...@canonical.com; Neftin, Sasha
>
> Subject: [Intel-wired-lan] [v4][PATCH] e1000e: continue to init phy even when
> failed to disable ULP
>
> After 'commit e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows
> for ME systems")',
> ThinkPad P14s always failed to disable ULP by ME.
> 'commit 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")'
> break out of init phy:
>
> error log:
> [ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
> [ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast
> Packet
> [ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>
> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
> If continue to init phy like before, it can work as before.
> iperf test result good too.
>
> Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
> Signed-off-by: Aaron Ma
> ---
> drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
> 1 file changed, 1 deletion(-)
I never did find a system that triggered the initial problem, but from a
compatibility with the set of systems I do have working...
Tested-by: Aaron Brown
[v4][PATCH] e1000e: continue to init phy even when failed to disable ULP
After 'commit e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows
for ME systems")',
ThinkPad P14s always failed to disable ULP by ME.
'commit 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")'
break out of init phy:
error log:
[ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.
Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..be7475c5529d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -303,7 +303,6 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
e1000_hw *hw)
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
e_warn("Failed to disable ULP\n");
- goto out;
}
ret_val = hw->phy.ops.acquire(hw);
--
2.26.2
[v3][PATCH] e1000e: continue to init phy even when failed to disable ULP
After commit: e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows
for ME systems").
ThinkPad P14s always failed to disable ULP by ME.
commit: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
break out of init phy:
error log:
[ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.
Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..be7475c5529d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -303,7 +303,6 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
e1000_hw *hw)
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
e_warn("Failed to disable ULP\n");
- goto out;
}
ret_val = hw->phy.ops.acquire(hw);
--
2.26.2
Re: [v2][PATCH] e1000e: continue to init phy even when failed to disable ULP
On Wed, 17 Jun 2020 19:12:48 +0800 Aaron Ma wrote:
> After commit: e086ba2fccd ("e1000e: disable s0ix entry and exit flows
> for ME systems").
> ThinkPad P14s always failed to disable ULP by ME.
> commit: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
> break out of init phy:
>
> error log:
> [ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
> [ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast
> Packet
> [ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>
> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
> If continue to init phy like before, it can work as before.
> iperf test result good too.
>
> Fixes: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
> Signed-off-by: Aaron Ma
Fixes tag: Fixes: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
Has these problem(s):
- SHA1 should be at least 12 digits long
Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11
or later) just making sure it is not set (or set to "auto").
[v2][PATCH] e1000e: continue to init phy even when failed to disable ULP
After commit: e086ba2fccd ("e1000e: disable s0ix entry and exit flows
for ME systems").
ThinkPad P14s always failed to disable ULP by ME.
commit: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
break out of init phy:
error log:
[ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.
Fixes: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..be7475c5529d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -303,7 +303,6 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
e1000_hw *hw)
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
e_warn("Failed to disable ULP\n");
- goto out;
}
ret_val = hw->phy.ops.acquire(hw);
--
2.26.2
Re: [PATCH] e1000e: continue to init phy even when failed to disable ULP
On 6/16/20 7:23 PM, Kai-Heng Feng wrote:
>
>
>> On Jun 16, 2020, at 18:05, Aaron Ma wrote:
>>
>> After commit "e1000e: disable s0ix entry and exit flows for ME systems",
>> some ThinkPads always failed to disable ulp by ME.
>> commit "e1000e: Warn if disabling ULP failed" break out of init phy:
>>
>> error log:
>> [ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
>> [ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast
>> Packet
>> [ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>>
>> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
>> If continue to init phy like before, it can work as before.
>> iperf test result good too.
>>
>> Chnage e_warn to e_dbg, in case it confuses.
>>
>> Signed-off-by: Aaron Ma
>> ---
>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> index f999cca37a8a..63405819eb83 100644
>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> @@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
>> e1000_hw *hw)
>> hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
>> ret_val = e1000_disable_ulp_lpt_lp(hw, true);
>
> If si0x entry isn't enabled, maybe skip calling e1000_disable_ulp_lpt_lp()
> altogether?
> We can use e1000e_check_me() to check that.
>
No, s0ix is different feature with ULP mode.
Aaron
>> if (ret_val) {
>> -e_warn("Failed to disable ULP\n");
>> -goto out;
>> +e_dbg("Failed to disable ULP\n");
>> }
>
> The change of "e1000e: Warn if disabling ULP failed" is intentional to catch
> bugs like this.
>
> Kai-Heng
>
>>
>> ret_val = hw->phy.ops.acquire(hw);
>> --
>> 2.26.2
>>
>
Re: [PATCH] e1000e: continue to init phy even when failed to disable ULP
> On Jun 16, 2020, at 18:05, Aaron Ma wrote:
>
> After commit "e1000e: disable s0ix entry and exit flows for ME systems",
> some ThinkPads always failed to disable ulp by ME.
> commit "e1000e: Warn if disabling ULP failed" break out of init phy:
>
> error log:
> [ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
> [ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast
> Packet
> [ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>
> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
> If continue to init phy like before, it can work as before.
> iperf test result good too.
>
> Chnage e_warn to e_dbg, in case it confuses.
>
> Signed-off-by: Aaron Ma
> ---
> drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> index f999cca37a8a..63405819eb83 100644
> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
> @@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
> e1000_hw *hw)
> hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
> ret_val = e1000_disable_ulp_lpt_lp(hw, true);
If si0x entry isn't enabled, maybe skip calling e1000_disable_ulp_lpt_lp()
altogether?
We can use e1000e_check_me() to check that.
> if (ret_val) {
> - e_warn("Failed to disable ULP\n");
> - goto out;
> + e_dbg("Failed to disable ULP\n");
> }
The change of "e1000e: Warn if disabling ULP failed" is intentional to catch
bugs like this.
Kai-Heng
>
> ret_val = hw->phy.ops.acquire(hw);
> --
> 2.26.2
>
Re: [Intel-wired-lan] [PATCH] e1000e: continue to init phy even when failed to disable ULP
On 6/16/20 6:20 PM, Paul Menzel wrote:
> Dear Aaron,
>
>
> Thank you for your patch.
>
> (Rant: Some more fallout from the other patch, which nobody reverted.)
>
Would you like a revert?
Thanks,
Aaron
> Am 16.06.20 um 12:05 schrieb Aaron Ma:
>> After commit "e1000e: disable s0ix entry and exit flows for ME systems",
>> some ThinkPads always failed to disable ulp by ME.
>
> Please add the (short) commit hash from the master branch.
>
> s/ulp/ULP/
>
> Please list one ThinkPad as example.
>
>> commit "e1000e: Warn if disabling ULP failed" break out of init phy:
>
> 1. Please add the closing quote ".
> 2. Please add the commit hash.
>
>> error log:
>> [ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
>> [ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast
>> Packet
>> [ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>>
>> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
>> If continue to init phy like before, it can work as before.
>> iperf test result good too.
>>
>> Chnage e_warn to e_dbg, in case it confuses.
>
> s/Chnage/Change/
>
> Please leave the level warning, and improve the warning message instead, so a
> user knows what is going on.
>
> Could you please add a `Fixes:` tag and the URL to the bug report?
>
>> Signed-off-by: Aaron Ma
>> ---
>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> index f999cca37a8a..63405819eb83 100644
>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> @@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
>> e1000_hw *hw)
>> hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
>> ret_val = e1000_disable_ulp_lpt_lp(hw, true);
>> if (ret_val) {
>> - e_warn("Failed to disable ULP\n");
>> - goto out;
>> + e_dbg("Failed to disable ULP\n");
>> }
>> ret_val = hw->phy.ops.acquire(hw);
>>
>
> Kind regards,
>
> Paul
Re: [Intel-wired-lan] [PATCH] e1000e: continue to init phy even when failed to disable ULP
Dear Aaron,
Thank you for your patch.
(Rant: Some more fallout from the other patch, which nobody reverted.)
Am 16.06.20 um 12:05 schrieb Aaron Ma:
After commit "e1000e: disable s0ix entry and exit flows for ME systems",
some ThinkPads always failed to disable ulp by ME.
Please add the (short) commit hash from the master branch.
s/ulp/ULP/
Please list one ThinkPad as example.
commit "e1000e: Warn if disabling ULP failed" break out of init phy:
1. Please add the closing quote ".
2. Please add the commit hash.
error log:
[ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.
Chnage e_warn to e_dbg, in case it confuses.
s/Chnage/Change/
Please leave the level warning, and improve the warning message instead,
so a user knows what is going on.
Could you please add a `Fixes:` tag and the URL to the bug report?
Signed-off-by: Aaron Ma
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..63405819eb83 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
e1000_hw *hw)
hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
- e_warn("Failed to disable ULP\n");
- goto out;
+ e_dbg("Failed to disable ULP\n");
}
ret_val = hw->phy.ops.acquire(hw);
Kind regards,
Paul
[PATCH] e1000e: continue to init phy even when failed to disable ULP
After commit "e1000e: disable s0ix entry and exit flows for ME systems",
some ThinkPads always failed to disable ulp by ME.
commit "e1000e: Warn if disabling ULP failed" break out of init phy:
error log:
[ 42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[ 42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[ 42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.
Chnage e_warn to e_dbg, in case it confuses.
Signed-off-by: Aaron Ma
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..63405819eb83 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct
e1000_hw *hw)
hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
- e_warn("Failed to disable ULP\n");
- goto out;
+ e_dbg("Failed to disable ULP\n");
}
ret_val = hw->phy.ops.acquire(hw);
--
2.26.2
