Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] usb: xhci: Increase timeout for HC halt
To:     Mathias Nyman <mathias.nyman@linux.intel.com>,
        Mathias Nyman <mathias.nyman@intel.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210511002933.1612871-1-luzmaximilian@gmail.com>
 <828bf140-1be6-9d92-1598-bfdf689bbdae@linux.intel.com>
From:   Maximilian Luz <luzmaximilian@gmail.com>
Message-ID: <c1b24d1a-497d-a303-7b54-e1d65402f08c@gmail.com>
Date:   Tue, 11 May 2021 10:42:14 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.0
MIME-Version: 1.0
In-Reply-To: <828bf140-1be6-9d92-1598-bfdf689bbdae@linux.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 5/11/21 9:19 AM, Mathias Nyman wrote:
> On 11.5.2021 3.29, Maximilian Luz wrote:
>> On some devices (specifically the SC8180x based Surface Pro X with
>> QCOM04A6) HC halt / xhci_halt() times out during boot. Manually binding
>> the xhci-hcd driver at some point later does not exhibit this behavior.
>> To work around this, double XHCI_MAX_HALT_USEC, which also resolves this
>> issue.
>>
>> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
>> ---
>>   drivers/usb/host/xhci-ext-caps.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
>> index fa59b242cd51..fb591e41cd50 100644
>> --- a/drivers/usb/host/xhci-ext-caps.h
>> +++ b/drivers/usb/host/xhci-ext-caps.h
>> @@ -7,8 +7,8 @@
>>    * Author: Sarah Sharp
>>    * Some code borrowed from the Linux EHCI driver.
>>    */
>> -/* Up to 16 ms to halt an HC */
>> -#define XHCI_MAX_HALT_USEC	(16*1000)
>> +/* Up to 32 ms to halt an HC */
>> +#define XHCI_MAX_HALT_USEC	(32 * 1000)
> 
> xHCI spec has a 16ms limit stated in several places, for example section 5.4.1
> "xHC is forced to halt within 16 ms. of software clearing the R/S bit to ‘0’,
> irrespective of any queued Transfer or Command Ring activity"

Right, thanks, I wasn't aware of this.

> To make sure hosts work we could increase it to 32, but comment could be
> changed to make sure it doean't get optimized back to 16 ms later.
> 
> How about:
> /* HC should halt within 16 ms, but use 32 ms as some in reality take longer */
> 
> If that's ok I can take this and modify the comment

That makes sense, yes. Please feel free to change that comment as you wish.

Regards,
Max