diff mbox series

[net-next,06/15] sfc_ef100: don't call efx_reset_down()/up() on EF100

Message ID be1d33ee-4a31-f2d2-d4f8-77380a07dcc9@solarflare.com
State Changes Requested
Delegated to: David Miller
Headers show
Series sfc_ef100: driver for EF100 family NICs, part 1 | expand

Commit Message

Edward Cree July 3, 2020, 3:32 p.m. UTC
We handle everything ourselves in ef100_reset(), rather than relying on
 the generic down/up routines.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 drivers/net/ethernet/sfc/efx_common.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

kernel test robot July 4, 2020, 2:15 a.m. UTC | #1
Hi Edward,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Edward-Cree/sfc_ef100-driver-for-EF100-family-NICs-part-1/20200703-233750
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8c8278a5b1a81e099ba883d8a0f9e3df9bdb1a74
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ca464639a1c9dd3944eb055ffd2796e8c2e7639f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/sfc/efx_common.c:856:6: warning: variable 'rc2' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (efx_nic_rev(efx) != EFX_REV_EF100)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sfc/efx_common.c:858:6: note: uninitialized use occurs here
           if (rc2) {
               ^~~
   drivers/net/ethernet/sfc/efx_common.c:856:2: note: remove the 'if' if its condition is always true
           if (efx_nic_rev(efx) != EFX_REV_EF100)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/sfc/efx_common.c:818:13: note: initialize the variable 'rc2' to silence this warning
           int rc, rc2;
                      ^
                       = 0
   1 warning generated.

vim +856 drivers/net/ethernet/sfc/efx_common.c

   809	
   810	/* Reset the NIC using the specified method.  Note that the reset may
   811	 * fail, in which case the card will be left in an unusable state.
   812	 *
   813	 * Caller must hold the rtnl_lock.
   814	 */
   815	int efx_reset(struct efx_nic *efx, enum reset_type method)
   816	{
   817		bool disabled;
   818		int rc, rc2;
   819	
   820		netif_info(efx, drv, efx->net_dev, "resetting (%s)\n",
   821			   RESET_TYPE(method));
   822	
   823		efx_device_detach_sync(efx);
   824		/* efx_reset_down() grabs locks that prevent recovery on EF100.
   825		 * EF100 reset is handled in the efx_nic_type callback below.
   826		 */
   827		if (efx_nic_rev(efx) != EFX_REV_EF100)
   828			efx_reset_down(efx, method);
   829	
   830		rc = efx->type->reset(efx, method);
   831		if (rc) {
   832			netif_err(efx, drv, efx->net_dev, "failed to reset hardware\n");
   833			goto out;
   834		}
   835	
   836		/* Clear flags for the scopes we covered.  We assume the NIC and
   837		 * driver are now quiescent so that there is no race here.
   838		 */
   839		if (method < RESET_TYPE_MAX_METHOD)
   840			efx->reset_pending &= -(1 << (method + 1));
   841		else /* it doesn't fit into the well-ordered scope hierarchy */
   842			__clear_bit(method, &efx->reset_pending);
   843	
   844		/* Reinitialise bus-mastering, which may have been turned off before
   845		 * the reset was scheduled. This is still appropriate, even in the
   846		 * RESET_TYPE_DISABLE since this driver generally assumes the hardware
   847		 * can respond to requests.
   848		 */
   849		pci_set_master(efx->pci_dev);
   850	
   851	out:
   852		/* Leave device stopped if necessary */
   853		disabled = rc ||
   854			method == RESET_TYPE_DISABLE ||
   855			method == RESET_TYPE_RECOVER_OR_DISABLE;
 > 856		if (efx_nic_rev(efx) != EFX_REV_EF100)
   857			rc2 = efx_reset_up(efx, method, !disabled);
   858		if (rc2) {
   859			disabled = true;
   860			if (!rc)
   861				rc = rc2;
   862		}
   863	
   864		if (disabled) {
   865			dev_close(efx->net_dev);
   866			netif_err(efx, drv, efx->net_dev, "has been disabled\n");
   867			efx->state = STATE_DISABLED;
   868		} else {
   869			netif_dbg(efx, drv, efx->net_dev, "reset complete\n");
   870			efx_device_attach_if_not_resetting(efx);
   871		}
   872		return rc;
   873	}
   874	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
index 5667694c6514..1bccd1f2cfa6 100644
--- a/drivers/net/ethernet/sfc/efx_common.c
+++ b/drivers/net/ethernet/sfc/efx_common.c
@@ -821,7 +821,11 @@  int efx_reset(struct efx_nic *efx, enum reset_type method)
 		   RESET_TYPE(method));
 
 	efx_device_detach_sync(efx);
-	efx_reset_down(efx, method);
+	/* efx_reset_down() grabs locks that prevent recovery on EF100.
+	 * EF100 reset is handled in the efx_nic_type callback below.
+	 */
+	if (efx_nic_rev(efx) != EFX_REV_EF100)
+		efx_reset_down(efx, method);
 
 	rc = efx->type->reset(efx, method);
 	if (rc) {
@@ -849,7 +853,8 @@  int efx_reset(struct efx_nic *efx, enum reset_type method)
 	disabled = rc ||
 		method == RESET_TYPE_DISABLE ||
 		method == RESET_TYPE_RECOVER_OR_DISABLE;
-	rc2 = efx_reset_up(efx, method, !disabled);
+	if (efx_nic_rev(efx) != EFX_REV_EF100)
+		rc2 = efx_reset_up(efx, method, !disabled);
 	if (rc2) {
 		disabled = true;
 		if (!rc)