On 6/22/2017 4:34 PM, Mathias Nyman wrote:
> On 22.06.2017 13:44, Shyam Sundar S K wrote:
>>
>> On 6/19/2017 9:42 AM, Shyam Sundar S K wrote:
>>> on AMD platforms with SNPS 3.1 USB controller if stop endpoint command is
>>> issued the controller does not respond, when the EP is not in running
>>> state. HW completes the command execution and reports
>>> "Context State Error" completion code. This is as per the spec. However
>>> HW on receiving the second command additionally marks EP to Flow control
>>> state in HW which is RTL bug. This bug causes the HW not to respond
>>> to any further doorbells that are rung by the driver. This makes the EP
>>> to not functional anymore and causes gross functional failures.
>>>
>>> As a workaround, not to hit this problem, its better we check the EP state
>>> and issue the stop EP command only when the EP is in running state.
>>>
>>> Also, with the inclusion of this patch, there maybe a possible race
>>> condition that may get triggered as mentioned in the xHCI spec 4.6.9
>>>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
>>> Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@xxxxxxx>
>>> ---
>>> drivers/usb/host/xhci-hub.c | 30 +++++++++++++++++++++---------
>>> 1 file changed, 21 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>>> index 0dde49c..d39b1e0 100644
>>> --- a/drivers/usb/host/xhci-hub.c
>>> +++ b/drivers/usb/host/xhci-hub.c
>>> @@ -398,17 +398,29 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
>>> spin_lock_irqsave(&xhci->lock, flags);
>>> for (i = LAST_EP_INDEX; i > 0; i--) {
>>> if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
>>> - struct xhci_command *command;
>>> - command = xhci_alloc_command(xhci, false, false,
>>> - GFP_NOWAIT);
>>> - if (!command) {
>>> - spin_unlock_irqrestore(&xhci->lock, flags);
>>> - xhci_free_command(xhci, cmd);
>>> - return -ENOMEM;
>>>
>>> + /* Check endpoint is running before stopping it,
>>> + * AMD SNPS 3.1 xHC may misbehave otherwise
>>> + */
>>> +
>>> + struct xhci_ep_ctx *ep_ctx;
>>> +
>>> + ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, i);
>>> +
>>> + if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING) {
>>> + struct xhci_command *command;
>>> +
>>> + command = xhci_alloc_command(xhci, false, false,
>>> + GFP_NOWAIT);
>>> + if (!command) {
>>> + spin_unlock_irqrestore(&xhci->lock,
>>> + flags);
>>> + xhci_free_command(xhci, cmd);
>>> + return -ENOMEM;
>>> + }
>>> + xhci_queue_stop_endpoint(xhci, command,
>>> + slot_id, i, suspend);
>>> }
>>> - xhci_queue_stop_endpoint(xhci, command, slot_id, i,
>>> - suspend);
>>> }
>>> }
>>> xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
>> Hi Mathias,
>>
>> I think, all your recommendations were taken care in the above patch. Any feedback on this ?
>>
>
> Looks good so far,
> I'll start looking/applying/testing it more after the midsummer festivities.
> I'll let you know if any new issues come up, otherwise I'll just apply it and send
> it forward
Thank you. Will be looking forward for an ACK from you :-)
>
> Thanks -Mathias
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
