Hi,

Lu Baolu <baolu...@linux.intel.com> writes:
> On 12/29/2016 07:00 PM, Felipe Balbi wrote:
>> COMP_SUCCESS should only be asserted on a *true* sucessful transfer. Any
>> other cases are bogus and we should aim to catch them. One easy way to
>> get bug reports is to trigger a WARN_ONCE() on such cases.
>>
>> Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
>> ---
>>  drivers/usb/host/xhci-ring.c | 28 +++++++++++++++-------------
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 17b878b53b46..772e07e2ef16 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -1934,6 +1934,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
>> struct xhci_td *td,
>>  {
>>      struct xhci_virt_device *xdev;
>>      struct xhci_ring *ep_ring;
>> +    struct device *dev;
>>      unsigned int slot_id;
>>      int ep_index;
>>      struct xhci_ep_ctx *ep_ctx;
>> @@ -1950,15 +1951,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
>> struct xhci_td *td,
>>      trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
>>      requested = td->urb->transfer_buffer_length;
>>      remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
>> +    dev = xhci_to_hcd(xhci)->self.controller;
>>  
>>      switch (trb_comp_code) {
>>      case COMP_SUCCESS:
>> -            if (trb_type != TRB_STATUS) {
>> -                    xhci_warn(xhci, "WARN: Success on ctrl %s TRB without 
>> IOC set?\n",
>> -                                    (trb_type == TRB_DATA) ? "data" : 
>> "setup");
>> -                    *status = -ESHUTDOWN;
>> -                    break;
>> -            }
>> +            dev_WARN_ONCE(dev, trb_type != TRB_STATUS,
>> +                            "ep%d%s: unexpected success! TRB Type %d\n",
>> +                            usb_endpoint_num(&td->urb->ep->desc),
>> +                            usb_endpoint_dir_in(&td->urb->ep->desc) ?
>> +                            "in" : "out", trb_type);
>>              *status = 0;
>
> Still setting status to 0 even "trb_type != TRB_STATUS"?

yeah, the reason is that I don't think this will *ever* happen. If it
does, I really wanna know about it. Moreover, it shouldn't be such a
fatal error if that happens. Mathias, any comments?

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to