Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1efe8317-ef7c-f636-71bc-02ceb28cf0fd@amd.com>
Date:   Tue, 3 Jan 2023 10:51:17 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH] net: amd-xgbe: add missed tasklet_kill
Content-Language: en-US
To:     jiguang.xiao@windriver.com, Shyam-sundar.S-k@amd.com
Cc:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, Prashant.Chikhalkar@windriver.com,
        zhaolong.zhang@windriver.com, Rick.Ilowite@windriver.com
References: <20221228081447.3400369-1-jiguang.xiao@windriver.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <20221228081447.3400369-1-jiguang.xiao@windriver.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NkM4TWZzM3o5TEdCNzVLZnJjQmtTREJWZHVPM1EyL1AvSTgrZFNpZUw5K1ps?=
 =?utf-8?B?TzdiSlUrUzhvemFMdms0eU03d1dkZ2t1d2hVVGp1SW9sZGUxQkFGMUg2S2cr?=
 =?utf-8?B?TDN3VEMzTG1rMjNpaUIyWHdmb0xNbmxqY2xVU3NzdXFoSVFTa01QNEFVYWhE?=
 =?utf-8?B?bnJ3R2N4cDFjZ1BXV1RiZWpKcVVHUE15Z0R6OEdONlZQVnJ1M2o0d1hDK1dU?=
 =?utf-8?B?ZDNTdEhjOVQwSCtPMERUR2lqZDV3ZFFCSmdjd0JjTDBmdEg5TmpudTJCQUt6?=
 =?utf-8?B?RHJDVHBGQWhOT3ZNNFJZay9obXd1VkJ5bTl6ejZ6Q2RsZDFEb3F5YnhMSzNZ?=
 =?utf-8?B?R2FYU2lsYkVHQU0xcm8yMmxlSFFIdHU4TTZ0aTNldnBFcklUdFZmNm9VQmRC?=
 =?utf-8?B?aFlmUUlMd0NyNHFvOE1jU3N6b21FSkVRZTlpQWRUd2dLWVRzMGlYK3FFcE9Y?=
 =?utf-8?B?SnVrcHB1RnNPcVgwMVlJRnZ5TlFWT0xGUmU2ZkdJZjk4dWlRNmk0c3hvbUxh?=
 =?utf-8?B?cHZrOHdLZ2YvK0h4OS83bEVSZkIybTdZL05kUDRtQW9NaDdKU1QySEt5b3B2?=
 =?utf-8?B?QmNNNTdGYUIxZ3JQcEQvZ1JUazlGNUN1cjRHbHdtTURwaGU0RDN0dzFxbldI?=
 =?utf-8?B?RUFkTmRpenNSZXZmL2I5WGtKWGkzdmlTbzdsaGEvVkNWMHNCQVBaYnpLYnFW?=
 =?utf-8?B?c1grQjZFYlRhS08yajlEakRjTlpuTXZoVzRrNGdiS001OEdod3lFajdmeWhZ?=
 =?utf-8?B?WEJqR3RibXJqZFdGM3pxRndUMTdWd1I5bHg5RitlY1V0KzBIUEdocEhVNmQy?=
 =?utf-8?B?d2wzMjQwOWtyZXhpT25jcjdKWlRwc2JKVFRQdzE2dzR5RGZYWnJES2Z4Q1dv?=
 =?utf-8?B?bHRWSUtHM0pwaU81UHg2TmF3YS9yWGd5aThyN21LdFpPTTRtT3F3M2hoZWQ2?=
 =?utf-8?B?dFdodUJlelgwYi9hMVVibGxCVUJIbk5sSDhjNlF1dnlrYU9vQ1hocnNvdDJs?=
 =?utf-8?B?NS9NbnBPNllDZXlHTDA3eVdhdklvY3N3dXMvT0laQzR4OFpWcFFSMzAvWFhN?=
 =?utf-8?B?RUEvSnlyanZ6ellPYXFpUGtTRndXYUVNbWxGL3dyQkZRWCtmSHRGTTYwVVZz?=
 =?utf-8?B?cFAxc0Z2VnZxeEFQeDhJV3VrNk9KQnh4MDdOeDhwTTZ5SVpyM1BRWVA1ZUJD?=
 =?utf-8?B?dVdoTjNhZi9vYjQ1QThnSFpFMElOK2pjeXhWaGx1TnAzdCtGaUVJTGdQVTRC?=
 =?utf-8?B?UzVqN2lNVHJOV0dvOHdCRUdLdzFqY2Rna3Y5ZkNZMklNRkhFNzVXRWt0NnEw?=
 =?utf-8?B?enFSQlp0dGJrTXNZTGJLbDFwcUhjL3ZpaUM4c25aWHJHZi9YRWpvdXN0OU1m?=
 =?utf-8?B?V1NOd01pdUxVUzN0akQwd0kwOFFqY2Y5R0d5UlUzVXlwQSthOEdRTThNZ3Zy?=
 =?utf-8?B?cFRWMlA1TEdGNEJFU1l2QTlRSjNtNlFKWDY0MTNyVzhxbU9BemFlNFBjVEhW?=
 =?utf-8?B?L0htM09IalZmemFzVUdXVlJqdUNvS3dxT3RUMWZwaVJyRU8xU1BtZ1FMM1Fk?=
 =?utf-8?B?Rm5vbVhyeUM3TzhkSjQxc2NBWmJkeTZCcE5zb2dubFB0a2ZLL1J0d3p0S0Js?=
 =?utf-8?B?NWcyUlJnYVNyQmUvZHVMeG5PNG55VDg2bDFoS1hsOEpDVTlOanlyUDV3elBs?=
 =?utf-8?B?UVVRR2ZoYVhMa3VKNHNWbWRLUFhOYUJ1Yzc2QzdiZytJRzJNQVp4OXZzZ1Iz?=
 =?utf-8?B?OHFKRlZNVit2d0wrM2JKalhjTVpyNTN3c1d5bW9DSmtxRDZETitvMTN4TU9v?=
 =?utf-8?B?UnhtajQ4a0dvdXNCUzI2Y1h6Zm91eG16dWFBZWtjTjdwRE9lV2lqTzZXNDF2?=
 =?utf-8?B?MU9pSFVXMCtVbTZETTVwcGN6bEc1WVNmV2cvOWtEZDhoM1hkbWVZWTB4emZr?=
 =?utf-8?B?cDhsbGw5WDRYUUdyRzltZUR6aGMrWU1SeHZrRDFHRytiQWMwTmNDZVlmY1Fr?=
 =?utf-8?B?bkt0emRsOWNMYVNjeGVTMUkrQ3ZKVGpRM0sxdVA2bjV2U1ROdDlwaDQ4SGtL?=
 =?utf-8?B?VGFYc051Ylo1ZHl4bUdCc1g2U3pQWC91WmlaMkc3TlI0ZHNmQU1ubjlob0Iy?=
 =?utf-8?Q?/AwbI7pHjpdZqeQjGZ+G35bnO?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 66b4caee-b2f3-4df2-fa4e-08daedaabd1d
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2023 16:51:20.1146
 (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: PhLL6Q+MFkrELhkHsIH+oILPyW8EQFcEaGiBOujkDtX5qxoxbE0LSRQyT1Rn6eAdpzjKwRpnX6lEHr8FXPFHlQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB7571
X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 12/28/22 02:14, jiguang.xiao@windriver.com wrote:
> From: Jiguang Xiao <jiguang.xiao@windriver.com>
> 
> The driver does not call tasklet_kill in several places.
> Add the calls to fix it.
> 
> Fixes: 85b85c853401 (amd-xgbe: Re-issue interrupt if interrupt status
> not cleared)
> Signed-off-by: Jiguang Xiao <jiguang.xiao@windriver.com>
> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-drv.c  | 3 +++
>   drivers/net/ethernet/amd/xgbe/xgbe-i2c.c  | 4 +++-
>   drivers/net/ethernet/amd/xgbe/xgbe-mdio.c | 4 +++-
>   3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> index 7b666106feee..614c0278419b 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -1064,6 +1064,9 @@ static void xgbe_free_irqs(struct xgbe_prv_data *pdata)
>   
>   	devm_free_irq(pdata->dev, pdata->dev_irq, pdata);
>   
> +	tasklet_kill(&pdata->tasklet_dev);
> +	tasklet_kill(&pdata->tasklet_ecc);

Should this tasklet_kill() have been put after the devm_free_irq() for the 
ecc_irq?

Maybe both tasklet_kill() calls could have been moved down a few lines.

Thanks,
Tom

> +
>   	if (pdata->vdata->ecc_support && (pdata->dev_irq != pdata->ecc_irq))
>   		devm_free_irq(pdata->dev, pdata->ecc_irq, pdata);
>   
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-i2c.c b/drivers/net/ethernet/amd/xgbe/xgbe-i2c.c
> index 22d4fc547a0a..a9ccc4258ee5 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-i2c.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-i2c.c
> @@ -447,8 +447,10 @@ static void xgbe_i2c_stop(struct xgbe_prv_data *pdata)
>   	xgbe_i2c_disable(pdata);
>   	xgbe_i2c_clear_all_interrupts(pdata);
>   
> -	if (pdata->dev_irq != pdata->i2c_irq)
> +	if (pdata->dev_irq != pdata->i2c_irq) {
>   		devm_free_irq(pdata->dev, pdata->i2c_irq, pdata);
> +		tasklet_kill(&pdata->tasklet_i2c);
> +	}
>   }
>   
>   static int xgbe_i2c_start(struct xgbe_prv_data *pdata)
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> index 4e97b4869522..0c5c1b155683 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> @@ -1390,8 +1390,10 @@ static void xgbe_phy_stop(struct xgbe_prv_data *pdata)
>   	/* Disable auto-negotiation */
>   	xgbe_an_disable_all(pdata);
>   
> -	if (pdata->dev_irq != pdata->an_irq)
> +	if (pdata->dev_irq != pdata->an_irq) {
>   		devm_free_irq(pdata->dev, pdata->an_irq, pdata);
> +		tasklet_kill(&pdata->tasklet_an);
> +	}
>   
>   	pdata->phy_if.phy_impl.stop(pdata);
>