netdev - Re: [PATCH] e1000e: move force SMBUS near the end of enable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <985b38e3-2188-490c-a112-b7b7c37ad503@intel.com>
Date: Wed, 17 Apr 2024 21:38:36 +0300
From: "Lifshits, Vitaly" <vitaly.lifshits@...el.com>
To: Hui Wang <hui.wang@...onical.com>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>, <jesse.brandeburg@...el.com>,
	<anthony.l.nguyen@...el.com>, <dima.ruinskiy@...el.com>
Subject: Re: [PATCH] e1000e: move force SMBUS near the end of enable_ulp
 function



On 4/13/2024 12:27 PM, Hui Wang wrote:
> The commit 861e8086029e ("e1000e: move force SMBUS from enable ulp
> function to avoid PHY loss issue") introduces a regression on
> CH_MTP_I219_LM18 (PCIID: 0x8086550A). Without this commit, the
> ethernet works well after suspend and resume, but after applying the
> commit, the ethernet couldn't work anymore after the resume and the
> dmesg shows that the NIC Link changes to 10Mbps (1000Mbps originally):
> [   43.305084] e1000e 0000:00:1f.6 enp0s31f6: NIC Link is Up 10 Mbps Full Duplex, Flow Control: Rx/Tx
> 
> Without the commit, the force SMBUS code will not be executed if
> "return 0" or "goto out" is executed in the enable_ulp(), and in my
> case, the "goto out" is executed since FWSM_FW_VALID is set. But after
> applying the commit, the force SMBUS code will be ran unconditionally.
> 
> Here move the force SMBUS code back to enable_ulp() and put it
> immediate ahead of hw->phy.ops.release(hw), this could allow the
> longest settling time as possible for interface in this function and
> doesn't change the original code logic.
> 
> Fixes: 861e8086029e ("e1000e: move force SMBUS from enable ulp function to avoid PHY loss issue")
> Signed-off-by: Hui Wang <hui.wang@...onical.com>

After discussing this patch offline with Hui this solution is the 
safest, therefore, I suggest to accept it.

Acked-by: Vitaly Lifshits <vitaly.lifshits@...el.com>