Received-SPF: pass (google.com: domain of linux-kernel+bounces-8917-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <77cf96b2-9a48-ae3f-f234-1c27186b1d3f@amd.com>
Date: Thu, 21 Dec 2023 10:25:41 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH] r8169: Fix PCI error on system resume
Content-Language: en-US
To: Kai-Heng Feng <kai.heng.feng@canonical.com>, hkallweit1@gmail.com,
 nic_swsd@realtek.com
Cc: "David S. Miller" <davem@davemloft.net>,
 Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20231221044350.441285-1-kai.heng.feng@canonical.com>
From: Brett Creeley <bcreeley@amd.com>
In-Reply-To: <20231221044350.441285-1-kai.heng.feng@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?anQ0TGltL29ONmdZVXM2eWcrMXk5OXhDaWkrOVA2QnhNQWIvMzU4MFZGNDFP?=
 =?utf-8?B?blE5RFlyNHpxa3Y4aDR4QXBSNXozSElqSjBZZmU2dnZ6UlRmMEp2TElFRG5l?=
 =?utf-8?B?RXJFSzl3c1NOeHZWUTduRElxYkNIdXdvYzliOFV2elhVVmRZZ0VzYWVtOEEy?=
 =?utf-8?B?YVJaeVI0WHVOb1hCdDhhb3R5bWIzWXBzTm1WV3Z2SGhSc1BVd2hzbHVkSHFG?=
 =?utf-8?B?NW5pblZSNkN2bEgyZzhuZFVDa1Z6WTBVb0J1dWxwazJxMGxCUkN4ZkdOenc3?=
 =?utf-8?B?ZytwOFEvblhtR05kelFJVDNYRnhLeTRla3paOFZhN2g4T244QUJmTmlMYTRM?=
 =?utf-8?B?QmJZeVU4TitVREV3MENCa1BSc2tSYWROdC9SYmpoQ3NKcmJ4SGl0SVhEYURk?=
 =?utf-8?B?RjlQa2J2K0lEclo2Um9tUWswN1ROWDdNWkw4b3dFaTJHVXF1Vno0dUtYZzk5?=
 =?utf-8?B?eDh6SXZoTU9MVUxvY21hTkxqYU5kK3J3dUFkTUNtaGhKcGg4cllxTnJ6QWpz?=
 =?utf-8?B?amtBRkZ1dWdSRFV5U015UUt4MnBKS2RwZ2doSGNWL2ZORmVRSFhSZzJ2NUVG?=
 =?utf-8?B?Z2RxWFVaYnRFTE1LSUswbzVmUm1XSWgrSHlkbmszRjg3MlpRdVRYb3VBWHhS?=
 =?utf-8?B?QmFiV0pGZytXWGNPNElMWC9LMngxeVZuTlBTTFNZVDU1K0lHb2RqandiNVZB?=
 =?utf-8?B?WnAvakJ5N09RemNtL1QxcEN1YWh1eTFUa0dRVmY1aEJRdm1oL1ZOTkxvczEr?=
 =?utf-8?B?TW02MW9HN3R2NERselBSSnlZTTMyc3BoR2Y5UVBMNklSNkJoYnlFMWVvMlhk?=
 =?utf-8?B?UlhMdEhQbUk2TDRYZTBkUGhGTjJPTWhxYldOODFsUFl3blpaRU5DSzhTRnBE?=
 =?utf-8?B?bUdpcDR2eWJpZHVWc1BzTHppbXJxd2JwU3d1MExKU1BGT3NlcERrOVJILzNr?=
 =?utf-8?B?YWJqUTVEd3FmczJEMFZCMkNvZy9CTnZ5d2Z0QUlJRDZSOFMwS0VmOTQwUUZu?=
 =?utf-8?B?ZEU1aEZJRExRcTMyNkRqK3krZWUzMUc5OE54cWRMZ3BzbnFRRkUvQ3BPMmpT?=
 =?utf-8?B?aFltemtNWU5rM3BoalAzNzd5NFZOSFg2WFJIVG1rZGs1T2VIRjNrd3pYVWRX?=
 =?utf-8?B?OVp2K0hIcC9wSHBVdFdCVnpaTHgxL0M2K25vLytuVTN5dzZYZ1FnVVM0aHBR?=
 =?utf-8?B?RnhyNTN5ek1hbXB2bWdQWUoyUGJCQVdlVzVxVDhkR3VuZDEvZEgzR1lpSUps?=
 =?utf-8?B?UDdEb2pSbkxZTGFZTjBjSjFMS3F3YW1ocnhjSEVUQnFRa1F4U3E5d3REWEUw?=
 =?utf-8?B?aTZWU0ExU25pb1pSc21nWElidENZOHZLb2piTDc1Q1ZtNU1OdUlxMTdTdXFy?=
 =?utf-8?B?eFB3YU5TSWNCTDFlaHoyTHQ5d0NpMVA3UFdGcCtWZ0M1Smd2bFN0WVlZcEg2?=
 =?utf-8?B?M2FWMGw5RTh5RUFBZXJMMlZIVlRNQ1V6SHBRWm1oTmVvVGdjMmhUZHZvRjAw?=
 =?utf-8?B?TjJGZ1hhT3BtQ25OSEJiZk5pUGNmUlU4TGdVZ3hac2hCN0NpTjVMTDl0QzNZ?=
 =?utf-8?B?OUo5cEVDSllFdklEUlpoaDN4WVp6QmJ2RzBrclZzTzFGb09USFRmeHM4Nzg2?=
 =?utf-8?B?am56V3VpWGdzSklrWStCSmloazY0YUtvZk5ya1Q2NkhqY0xQT3h1YUdocnJn?=
 =?utf-8?B?MndCb0xKY0dzdEJBSS9scmdBYm0vV2NqZHM5MGhMbDd6ZzgzdEZmNlFGVG1S?=
 =?utf-8?B?L0RhdzJMc24xTWM5TU00MUZKRkc4cWQ3dEF6MmExWnowQW9OSHpoR1RJZjR0?=
 =?utf-8?B?MWU3enBmaFdmSlhkSWloNmpKWnpzbkNaOGg5ZWZ6Q3Fvb2MvVzRMdnZ4ZGtu?=
 =?utf-8?B?b2V0VURPUzdhVTdnY2ZtVWk1Q0NFNWY3TkliMUJKSXVJSUJTYktuTGFqL1Q2?=
 =?utf-8?B?Ukhtb2RSWmRhY2k4aFNVL3Q2d1dtdkdjaE1BelVwTldJN2FYS2JlQ3B2TlN5?=
 =?utf-8?B?NmJRSmpFdWVralJMd3NYazk4a0cxUzlkTkd2d291T2x2clA5eW9LVm52Z0FN?=
 =?utf-8?B?OVl4SktxUUxIblloM00yN0g1a3JGY3l3VWxxN2tWNXViNjdnSys3RTNJYkxh?=
 =?utf-8?Q?2M+0HIwnikCxrtMVAOjcV0UGD?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8e0d7810-6884-49c5-8855-08dc02523e31
X-MS-Exchange-CrossTenant-AuthSource: PH0PR12MB7982.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Dec 2023 18:25:43.5039
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8EK2W8llTxBpW3MEwvTxv1pLQ8gzEuAAKM7JWeyDfjsiH63MH7nEt16Sn5ubF0E2JvjdJTctVOs/00RUhkmbNw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN6PR12MB8565


On 12/20/2023 8:43 PM, Kai-Heng Feng wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> [  688.051096] r8169 0000:02:00.1 enp2s0f1: rtl_ep_ocp_read_cond == 0 (loop: 10, delay: 10000).
> [  688.175131] r8169 0000:02:00.1 enp2s0f1: Link is Down
> ...
> [  691.534611] r8169 0000:02:00.1 enp2s0f1: PCI error (cmd = 0x0407, status_errs = 0x0000)
> 
> Since increase the loop wait on rtl_ep_ocp_read_cond can eliminate the
> issue, so let rtl8168ep_driver_start() to wait a bit longer.
> 

This doesn't really describe what the issue is that you are fixing. I 
think a more clear commit message would be helpful. Also, why "30" loops?

Does this need a Fixes tag? If so the Fixes tag should be added and the 
subject line should start with: "[PATCH net]".

Otherwise, the subject line should start with: "[PATCH net-next]".

Thanks,

Brett
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>   drivers/net/ethernet/realtek/r8169_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index bb787a52bc75..81fd31f6fac4 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -1211,7 +1211,7 @@ static void rtl8168ep_driver_start(struct rtl8169_private *tp)
>   {
>          r8168ep_ocp_write(tp, 0x01, 0x180, OOB_CMD_DRIVER_START);
>          r8168ep_ocp_write(tp, 0x01, 0x30, r8168ep_ocp_read(tp, 0x30) | 0x01);
> -       rtl_loop_wait_high(tp, &rtl_ep_ocp_read_cond, 10000, 10);
> +       rtl_loop_wait_high(tp, &rtl_ep_ocp_read_cond, 10000, 30);
>   }
> 
>   static void rtl8168_driver_start(struct rtl8169_private *tp)
> --
> 2.34.1
> 
>