Received-SPF: pass (google.com: domain of linux-kernel+bounces-170100-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <7f533913-fba9-4a29-86a5-d3b32ac44632@intel.com>
Date: Mon, 6 May 2024 18:53:14 +0300
User-Agent: Mozilla Thunderbird
Subject: Re: [Intel-wired-lan] [PATCH v2 1/2] e1000e: let the sleep codes run
 every time
To: Ricky Wu <en-wei.wu@canonical.com>
CC: <netdev@vger.kernel.org>, <rickywu0421@gmail.com>,
	<linux-kernel@vger.kernel.org>, <edumazet@google.com>,
	<intel-wired-lan@lists.osuosl.org>, <kuba@kernel.org>,
	<anthony.l.nguyen@intel.com>, <pabeni@redhat.com>, <davem@davemloft.net>,
	"Brandeburg, Jesse" <jesse.brandeburg@intel.com>, "Ruinskiy, Dima"
	<dima.ruinskiy@intel.com>, "Lifshits, Vitaly" <vitaly.lifshits@intel.com>
References: <20240503101824.32717-1-en-wei.wu@canonical.com>
Content-Language: en-US
From: Sasha Neftin <sasha.neftin@intel.com>
In-Reply-To: <20240503101824.32717-1-en-wei.wu@canonical.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SHVINTdVWHVTNU9UZGNvS29UWlVFcklFT1RDaUVzbVBkcXVCOG5VS1hHaHRT?=
 =?utf-8?B?RVkzc1pVYVRhYTVEZFJsOUk0N3gzY211TDFYQmZHOWxjQUFWRVB6d3lVa2Z3?=
 =?utf-8?B?a2VQL1RkSDQvU2VXOEtWSzY5OGNFVnczdG1udXZUaERDaGJvaXZUZ0crdXhU?=
 =?utf-8?B?UDhicmZJaklaM2FWTzZ6SENPdzRVemw1eUY4bmdUMFhpN1RKeTRJYUpoQVRJ?=
 =?utf-8?B?MGsvTnhsMGZvTUNwenl0U1laWDdheGt3VVhsdXU4UHJlMkxJWExyVXNrenYr?=
 =?utf-8?B?RUFXdlZQaHA2TzBPUkNOVmhXZnk1UG5SQktGSTUyRGRpVjVpSHdpbkxtREcy?=
 =?utf-8?B?RW0rWlVMQlo0SFhzSWVncmtydUF4V0dOOE50bXFVYkNDL1BVV1BuVDdMQVFk?=
 =?utf-8?B?VXdicGt3MFRmMHhUdGVBYWU4Zmt0ZmlIRlRsTmZ6N3NBRU10NXpQZWJ2dUlU?=
 =?utf-8?B?K2h4R2E3OVY1KzNCN1dqNDJPR2lEQkwwekh4MGJpVVphNlYrYmU0TWZKajN3?=
 =?utf-8?B?dDBzcWppUys3aGNSYVVxT3B4bFhKemdjY2d6eDVSaURNSU1jSDV3bnFTT0NX?=
 =?utf-8?B?QjVkaC9CczNHN0MwQ3RNNzVnOFRXM01najFiVVVwemp5dHN0MzVoN0UxUHYz?=
 =?utf-8?B?bXptSHBLQmpEQXlIaWZrK2w3c0ZXT2xLL0h2T2NJZ0dJTy8reVNwQ050a0FF?=
 =?utf-8?B?cDJVaktMZEdIb2hDT3ZSb0lXRGd2Y2JrV1lTRTgrVTdMVldxTW0vaWpHNFpV?=
 =?utf-8?B?K2N4YVZIbFVrekkvYnozY216aXZDYkE1ZnJnSEJrMDFJRm40NDA3RUllalFv?=
 =?utf-8?B?YXJJdkp1YStBMXJnZkF6L3BtRGVFbXJYa284a0ErTkJXdDFFcTVWSFhaSkQ2?=
 =?utf-8?B?ejRjb29sWkpNWHFvajJ1Q3ZaV2plMCtsQXVxeVBUYk5BWnJEdWVwWHI0TU1l?=
 =?utf-8?B?S0ZDMGZJWmRmYm5Qc1U1RGoxRGIvcnJldThRT0NBRHBsY3hJbDZBNzlGQW5y?=
 =?utf-8?B?cGwwTGdvNVRiR1FZMGNjNFkvTmFrSmtwRmh6ZmFSRTBLVXMwMG0zMUpVTXBF?=
 =?utf-8?B?bG5wQVhBOUxpMkd4TnViYzRyNFUwYWk3S2lURHM3SlpaZWtnSGxIdkkwVWw1?=
 =?utf-8?B?U3dMcmdPWGRCSkVjbnlTRFo5OFNqZmhQeWVoN1Y3TEgxUE9TUmhNNTFmZm9a?=
 =?utf-8?B?Q3JLWUg0UHp0MW5kWm92MXNGeWhzRzdObEZ0S2hGOEZVL2hlVjY1UllvSURY?=
 =?utf-8?B?SUxLYVBYbVJhVFJ5Skk5bXBTTm9Oa3QvL2pteWNaR2hlemVvd1FDZEdVWUxk?=
 =?utf-8?B?ZmFYVm5IZFpieVA0QXRhdFYxbTZaL01ReHBPMHVTbEY1S2dVYXBvaEtmY3Nw?=
 =?utf-8?B?L1ZsRVF6ZEVEcVRiTE10ZG1EREVTcStTeWRST1o1YnZZL1M3TkxDUFJkTjhB?=
 =?utf-8?B?aDdSSDZ6MStsSFY5c0pXUmVSSklOdm1nS1RobEtxYmg1NVZqQThLdGJ2S1Iv?=
 =?utf-8?B?NGVpVzVQTEdCZXBMakhXUm9xS0Y3WTlLTnY3SEk1VWR2T0FSZGx5SzVrU0Qv?=
 =?utf-8?B?dmZDN0RFOUZKekVDeWgxRDA5Mi9ERlVaTEJpUzJxOUloQldrQStsVk9sVmNi?=
 =?utf-8?B?UkRPTDM3TEtVWTZ6NmxySTdtQUJCcmlwYlViVHlweWVuMkpscm5RSzZJVWJh?=
 =?utf-8?B?K292MEw5ZUlEak1hYUJIZVQycExwa2VJTHp3cXZ6NlBMUE1nbnNtd2FxaklD?=
 =?utf-8?B?RTVTQnB3RkdMdWdWelNEV3RRQTZDTWhnRlJDLy9iMlJsTEx5RnVveWQwS0Fr?=
 =?utf-8?B?bzVsUWI3bFZsbzVZSFNIV2kvOFV3MVVzZENrd3NhTnlIOUo2cGFqT2JGcDBv?=
 =?utf-8?B?aXh3WndnNnVJZ01zZUFIajJCZW9lTTNvNHNoNHJnMUNHdndmNFZpSlBTVjdL?=
 =?utf-8?B?Y0dHdnhaYy9pVnlIWUNyY2lTeWtYV2hXaUJhVlJXSmlCZ1laaUlDMlZzbEQ2?=
 =?utf-8?B?VjE5d1cvZ09VNEMwTStydy85WExoeW9YMHlzdm5DTlUwVVRiUEc5UmF0NGUw?=
 =?utf-8?B?U3VLb3lZTityaFF0bUo2ak43Q3BSVVVleXlUUWdPR1YvVnpjRjFYMUtDUVVN?=
 =?utf-8?Q?pqgMo1hrIUTIjmHzKT3PjhILi?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8015a659-ab48-4bbd-64f6-08dc6de4a7fb
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB6738.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2024 15:53:22.0960
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: APSkA+GQTmaZ+R+KjT3OBXVWlTUSb+zGvMC0qBrHZgaBaqpL1m8DwJYH7XoxES4iXkTX+GtpV5XBv7Q+AUXy1g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7443
X-OriginatorOrg: intel.com

On 03/05/2024 13:18, Ricky Wu wrote:
> Originally, the sleep codes being moved forward only
> ran if we met some conditions (e.g. BMSR_LSTATUS bit
> not set in phy_status). Moving these sleep codes forward
> makes the usec_interval take effect every time.
> 
> Signed-off-by: Ricky Wu <en-wei.wu@canonical.com>
> ---
> 
> In v2:
> * Split the sleep codes into this patch
> 
>   drivers/net/ethernet/intel/e1000e/phy.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
> index 93544f1cc2a5..4a58d56679c9 100644
> --- a/drivers/net/ethernet/intel/e1000e/phy.c
> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
> @@ -1777,6 +1777,11 @@ s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, u32 iterations,
>   
>   	*success = false;
>   	for (i = 0; i < iterations; i++) {
> +		if (usec_interval >= 1000)
> +			msleep(usec_interval / 1000);
> +		else
> +			udelay(usec_interval);
> +

I do not understand this approach. Why wait before first 
reading/accessing the PHY IEEE register?

For further discussion, I would like to introduce Dima Ruinskiy (architect)

>   		/* Some PHYs require the MII_BMSR register to be read
>   		 * twice due to the link bit being sticky.  No harm doing
>   		 * it across the board.
> @@ -1799,10 +1804,6 @@ s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, u32 iterations,
>   			*success = true;
>   			break;
>   		}
> -		if (usec_interval >= 1000)
> -			msleep(usec_interval / 1000);
> -		else
> -			udelay(usec_interval);
>   	}
>   
>   	return ret_val;