From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752249AbdHKBS0 (ORCPT ); Thu, 10 Aug 2017 21:18:26 -0400 Received: from mga07.intel.com ([134.134.136.100]:36111 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752149AbdHKBSN (ORCPT ); Thu, 10 Aug 2017 21:18:13 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,355,1498546800"; d="scan'208";a="888820032" Subject: Re: [PATCH v2 1/5] usb: xhci: Disable slot even virt-dev is null To: Mathias Nyman References: <1501122098-10935-1-git-send-email-baolu.lu@linux.intel.com> <1501122098-10935-2-git-send-email-baolu.lu@linux.intel.com> <598AC09D.4070003@linux.intel.com> <598BAA3F.7070103@linux.intel.com> <598C2EA4.7010803@linux.intel.com> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, zhengjun.xing@linux.intel.com, Guoqing Zhang From: Lu Baolu Message-ID: <598D05D2.7040403@linux.intel.com> Date: Fri, 11 Aug 2017 09:18:10 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <598C2EA4.7010803@linux.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 08/10/2017 06:00 PM, Mathias Nyman wrote: > On 10.08.2017 03:35, Lu Baolu wrote: >> Hi, >> >> On 08/09/2017 03:58 PM, Mathias Nyman wrote: >>> On 27.07.2017 05:21, Lu Baolu wrote: >>>> xhci_disable_slot() is a helper for disabling a slot when a device >>>> goes away or recovers from error situations. Currently, it checks >>>> the corespoding virt-dev pointer and returns directly (w/o issuing >>>> disable slot command) if it's null. >>>> >>>> This is unnecessary and will cause problems in case where virt-dev >>>> allocation fails and xhci_disable_slot() is called to roll back the >>>> hardware state. Refer to the implementation of xhci_alloc_dev(). >>>> >>> >>> True, checking for xhci->devs[slot_id] doesn't work if xhci_alloc_dev() >>> failed xhci_alloc_virt_device() and calls xhci_disable_slot, >>> >>> but the virt dev check is needed for test mode, which will just try >>> to disable all slots from 1 to HCS_MAX_SLOTS: >>> >>> xhci_enter_test_mode(..) >>> /* Disable all Device Slots */ >>> for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) { >>> retval = xhci_disable_slot(xhci, NULL, i); >>> >>> >> >> So, how about checking virt dev before this invoking? >> >> @@ -612,10 +612,12 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci, >> xhci_dbg(xhci, "Disable all slots\n"); >> spin_unlock_irqrestore(&xhci->lock, *flags); >> for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) { >> - retval = xhci_disable_slot(xhci, NULL, i); >> - if (retval) >> - xhci_err(xhci, "Failed to disable slot %d, %d. Enter test mode anyway\n", >> - i, retval); >> + if (xhci->devs[i]) { >> + retval = xhci_disable_slot(xhci, NULL, i); >> + if (retval) >> + xhci_err(xhci, "Failed to disable slot %d, %d. Enter test mode anyway\n", >> + i, retval); >> + } > > > Yes, something like this will work > Okay, I will submit a v3 to include this change. Best regards, Lu Baolu