From patchwork Thu May 18 22:22:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Gustavo A. R. Silva" X-Patchwork-Id: 764488 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wTgnF57Qfz9s7k for ; Fri, 19 May 2017 18:14:09 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 40101872E4; Fri, 19 May 2017 08:14:08 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id j71nUoOPfulq; Fri, 19 May 2017 08:14:06 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by whitealder.osuosl.org (Postfix) with ESMTP id 37EAF874C7; Fri, 19 May 2017 08:14:05 +0000 (UTC) X-Original-To: intel-wired-lan@lists.osuosl.org Delivered-To: intel-wired-lan@lists.osuosl.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id A2B0A1BFBD2 for ; Thu, 18 May 2017 22:47:12 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 9A769881A8 for ; Thu, 18 May 2017 22:47:12 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5P6oCJGJrbay for ; Thu, 18 May 2017 22:47:11 +0000 (UTC) X-Greylist: delayed 00:24:46 by SQLgrey-1.7.6 Received: from gateway20.websitewelcome.com (gateway20.websitewelcome.com [192.185.54.2]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 44DAD88174 for ; Thu, 18 May 2017 22:47:11 +0000 (UTC) Received: from cm4.websitewelcome.com (unknown [108.167.139.16]) by gateway20.websitewelcome.com (Postfix) with ESMTP id 88790400C44D3 for ; Thu, 18 May 2017 17:22:24 -0500 (CDT) Received: from gator4166.hostgator.com ([108.167.133.22]) by cm4.websitewelcome.com with id MyNP1v00j0V9LXg01yNQN7; Thu, 18 May 2017 17:22:24 -0500 Received: from gator4166.hostgator.com ([108.167.133.22]:34605) by gator4166.hostgator.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.87) (envelope-from ) id 1dBToN-000PC9-9m; Thu, 18 May 2017 17:22:23 -0500 Received: from 189.152.179.187 ([189.152.179.187]) by gator4166.hostgator.com (Horde Framework) with HTTPS; Thu, 18 May 2017 17:22:23 -0500 Date: Thu, 18 May 2017 17:22:23 -0500 Message-ID: <20170518172223.Horde.5kLLwxMJWThQKv0bAxTsk4D@gator4166.hostgator.com> From: "Gustavo A. R. Silva" To: Jeff Kirsher User-Agent: Horde Application Framework 5 MIME-Version: 1.0 Content-Disposition: inline X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - gator4166.hostgator.com X-AntiAbuse: Original Domain - lists.osuosl.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - embeddedor.com X-BWhitelist: no X-Source-IP: 108.167.133.22 X-Exim-ID: 1dBToN-000PC9-9m X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: gator4166.hostgator.com [108.167.133.22]:34605 X-Source-Auth: garsilva@embeddedor.com X-Email-Count: 6 X-Source-Cap: Z3V6aWRpbmU7Z3V6aWRpbmU7Z2F0b3I0MTY2Lmhvc3RnYXRvci5jb20= X-Mailman-Approved-At: Fri, 19 May 2017 08:14:04 +0000 Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org Subject: [Intel-wired-lan] [net-intel-e1000e] question about value overwrite X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" Hello everybody, While looking into Coverity ID 1226905 I ran into the following piece of code at drivers/net/ethernet/intel/e1000e/ich8lan.c:2400 2400/** 2401 * e1000_hv_phy_workarounds_ich8lan - A series of Phy workarounds to be 2402 * done after every PHY reset. 2403 **/ 2404static s32 e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw) 2405{ 2406 s32 ret_val = 0; 2407 u16 phy_data; 2408 2409 if (hw->mac.type != e1000_pchlan) 2410 return 0; 2411 2412 /* Set MDIO slow mode before any other MDIO access */ 2413 if (hw->phy.type == e1000_phy_82577) { 2414 ret_val = e1000_set_mdio_slow_mode_hv(hw); 2415 if (ret_val) 2416 return ret_val; 2417 } 2418 2419 if (((hw->phy.type == e1000_phy_82577) && 2420 ((hw->phy.revision == 1) || (hw->phy.revision == 2))) || 2421 ((hw->phy.type == e1000_phy_82578) && (hw->phy.revision == 1))) { 2422 /* Disable generation of early preamble */ 2423 ret_val = e1e_wphy(hw, PHY_REG(769, 25), 0x4431); 2424 if (ret_val) 2425 return ret_val; 2426 2427 /* Preamble tuning for SSC */ 2428 ret_val = e1e_wphy(hw, HV_KMRN_FIFO_CTRLSTA, 0xA204); 2429 if (ret_val) 2430 return ret_val; 2431 } 2432 2433 if (hw->phy.type == e1000_phy_82578) { 2434 /* Return registers to default by doing a soft reset then 2435 * writing 0x3140 to the control register. 2436 */ 2437 if (hw->phy.revision < 2) { 2438 e1000e_phy_sw_reset(hw); 2439 ret_val = e1e_wphy(hw, MII_BMCR, 0x3140); 2440 } 2441 } 2442 2443 /* Select page 0 */ 2444 ret_val = hw->phy.ops.acquire(hw); 2445 if (ret_val) 2446 return ret_val; 2447 2448 hw->phy.addr = 1; 2449 ret_val = e1000e_write_phy_reg_mdic(hw, IGP01E1000_PHY_PAGE_SELECT, 0); 2450 hw->phy.ops.release(hw); 2451 if (ret_val) 2452 return ret_val; 2453 2454 /* Configure the K1 Si workaround during phy reset assuming there is 2455 * link so that it disables K1 if link is in 1Gbps. 2456 */ 2457 ret_val = e1000_k1_gig_workaround_hv(hw, true); 2458 if (ret_val) 2459 return ret_val; 2460 2461 /* Workaround for link disconnects on a busy hub in half duplex */ 2462 ret_val = hw->phy.ops.acquire(hw); 2463 if (ret_val) 2464 return ret_val; 2465 ret_val = e1e_rphy_locked(hw, BM_PORT_GEN_CFG, &phy_data); 2466 if (ret_val) 2467 goto release; 2468 ret_val = e1e_wphy_locked(hw, BM_PORT_GEN_CFG, phy_data & 0x00FF); 2469 if (ret_val) 2470 goto release; 2471 2472 /* set MSE higher to enable link to stay up when noise is high */ 2473 ret_val = e1000_write_emi_reg_locked(hw, I82577_MSE_THRESHOLD, 0x0034); 2474release: 2475 hw->phy.ops.release(hw); 2476 2477 return ret_val; 2478} The issue is that the value stored in variable _ret_val_ at line 2439 is overwritten by the one stored at line 2444, before it can be used. My question is if the original intention was to return this value immediately after the assignment at line 2439, something like in the following patch: index 68ea8b4..d6d4ed7 100644 What do you think? I'd really appreciate any comment on this. Thank you! --- Gustavo A. R. Silva --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c @@ -2437,6 +2437,8 @@ static s32 e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw) if (hw->phy.revision < 2) { e1000e_phy_sw_reset(hw); ret_val = e1e_wphy(hw, MII_BMCR, 0x3140); + if (ret_val) + return ret_val; } }