From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C61B3C433EF for ; Thu, 5 May 2022 14:09:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1380428AbiEEONP (ORCPT ); Thu, 5 May 2022 10:13:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36132 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347604AbiEEONH (ORCPT ); Thu, 5 May 2022 10:13:07 -0400 Received: from vps0.lunn.ch (vps0.lunn.ch [185.16.172.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D513F5A0B4; Thu, 5 May 2022 07:09:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=6Lmbk4iO3bAEj7owhMXQsEqPqr87gMH5In94FkOVBfw=; b=Aoqc9T79xRfKYVs3id4satwXyq zthalTtBJQEWrhGGjEiZ8LID9PItEJ8UPFI23dYGREnp3kQPPQXWx4ETJs0RgVlduyzoFsiwc4qcZ Dxt0BWtweS8zIeRBXEg0wRIywxyO7O+OvPIQ+bn5qfr+KxPsC0U00cUesUHbVIXlKQLk=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1nmcAE-001N3s-Tm; Thu, 05 May 2022 16:09:06 +0200 Date: Thu, 5 May 2022 16:09:06 +0200 From: Andrew Lunn To: Jiabing Wan Cc: Jiabing Wan , Heiner Kallweit , Russell King , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] net: phy: micrel: Remove unnecessary comparison in lan8814_handle_interrupt Message-ID: References: <20220505030217.1651422-1-wanjiabing@vivo.com> <2ec61428-d9af-7712-b008-cf6b7e445aaa@vivo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > I write the coccicheck and find these reports: Nice! > For directly call __phy_read(): > > ./drivers/net/phy/micrel.c:1969:59-60: WARNING: __phy_read() assigned to an > unsigned int 'data' This one we know about. > ./drivers/net/phy/mscc/mscc_macsec.c:49:50-51: WARNING: __phy_read() > assigned to an unsigned int 'val' > ./drivers/net/phy/mscc/mscc_macsec.c:52:51-52: WARNING: __phy_read() > assigned to an unsigned int 'val_l' > ./drivers/net/phy/mscc/mscc_macsec.c:53:51-52: WARNING: __phy_read() > assigned to an unsigned int 'val_h' > ./drivers/net/phy/mscc/mscc_macsec.c:89:50-51: WARNING: __phy_read() > assigned to an unsigned int 'val' These are all in the same function. Looking at the first check, it has been decided that if something goes wrong, return 0. Not the best solution, but better than returning something random. So the do/while loop can goto failed: val_l and val_h are a bit more messy, but a check would be nice, return 0 on error. Actually returning an error code is not going to be easy. The rest of the code assumes vsc8584_macsec_phy_read() never fails :-( > ./drivers/net/phy/mscc/mscc_main.c:1511:50-51: WARNING: __phy_read() > assigned to an unsigned int 'addr' > ./drivers/net/phy/mscc/mscc_main.c:1514:47-48: WARNING: __phy_read() > assigned to an unsigned int 'val' More code which assumes a read can never fail. vsc8514_probe() calls this, so it should be reasonable easy to catch the error, return it, and make the probe fail. > ./drivers/net/phy/mscc/mscc_main.c:366:54-55: WARNING: __phy_read() assigned > to an unsigned int 'reg_val' > ./drivers/net/phy/mscc/mscc_main.c:370:55-56: WARNING: __phy_read() assigned > to an unsigned int 'pwd [ 0 ]' > ./drivers/net/phy/mscc/mscc_main.c:371:53-54: WARNING: __phy_read() assigned > to an unsigned int 'pwd [ 1 ]' > ./drivers/net/phy/mscc/mscc_main.c:372:55-56: WARNING: __phy_read() assigned > to an unsigned int 'pwd [ 2 ]' > ./drivers/net/phy/mscc/mscc_main.c:317:54-55: WARNING: __phy_read() assigned > to an unsigned int 'reg_val' Another void function which assumes it cannot fail. Error checks would be nice, don't return random data in the wol structure. Thanks Andrew