lkml.org 
[lkml]   [2022]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v3] usb: cdnsp: fix issue with ZLP - added TD_SIZE = 1
Date
>
>> Pawel, with your change, the TD_SIZE is 1 or 0, but not like the
>> kernel doc defined like below:
>>
>
>Please omit my comments, I did not check the code carefully.
>
>> /*
>> * TD size is the number of max packet sized packets remaining in the
>> TD
>> * (*not* including this TRB).
>> *
>
>With your current change, it may work. But your change conflicts with the
>xHCI spec that described above.
>With ZLP, the last useful trb's TD size should be 0, but if it is 0, the controller
>will be confused.
>
>With your change, it makes the code more different with xhci's. Do you
>consider handling ZLP packet at another TD instead of current at the same TD
>?

Yes, I considered it, but to simplify the driver I resigned from handling multiple TD
per usb_request.
Current solution is simpler and supported by device controller.

Additionally, the next ZLP fix must be added for control endpoint for Data Stage
and probably it will not work with 2 TDs.

Pawel

>
>> * Total TD packet count = total_packet_count =
>> * DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
>> *
>> * Packets transferred up to and including this TRB = packets_transferred =
>> * rounddown(total bytes transferred including this TRB /
>wMaxPacketSize)
>> *
>> * TD size = total_packet_count - packets_transferred
>> *
>> * It must fit in bits 21:17, so it can't be bigger than 31.
>> * This is taken care of in the TRB_TD_SIZE() macro
>> *
>> * The last TRB in a TD must have the TD size set to zero.
>> */
>>
>> Peter
>>
>> > /* One TRB with a zero-length data packet. */
>> > if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
>> > trb_buff_len == td_total_len) @@ -1960,7 +1965,8 @@ int
>> > cdnsp_queue_bulk_tx(struct cdnsp_device *pdev, struct cdnsp_request
>*preq)
>> > /* Set the TRB length, TD size, and interrupter fields. */
>> > remainder = cdnsp_td_remainder(pdev, enqd_len, trb_buff_len,
>> > full_len, preq,
>> > - more_trbs_coming);
>> > + more_trbs_coming,
>> > + zero_len_trb);
>> >
>> > length_field = TRB_LEN(trb_buff_len) | TRB_TD_SIZE(remainder)
>|
>> > TRB_INTR_TARGET(0); @@ -2025,7 +2031,7 @@
>> > int cdnsp_queue_ctrl_tx(struct cdnsp_device *pdev, struct
>> > cdnsp_request *preq)
>> >
>> > if (preq->request.length > 0) {
>> > remainder = cdnsp_td_remainder(pdev, 0, preq->request.length,
>> > - preq->request.length, preq, 1);
>> > + preq->request.length,
>> > + preq, 1, 0);
>> >
>> > length_field = TRB_LEN(preq->request.length) |
>> > TRB_TD_SIZE(remainder) |
>> > TRB_INTR_TARGET(0); @@ -2225,7 +2231,7 @@ static int
>cdnsp_queue_isoc_tx(struct cdnsp_device *pdev,
>> > /* Set the TRB length, TD size, & interrupter fields. */
>> > remainder = cdnsp_td_remainder(pdev, running_total,
>> > trb_buff_len, td_len, preq,
>> > - more_trbs_coming);
>> > + more_trbs_coming, 0);
>> >
>> > length_field = TRB_LEN(trb_buff_len) |
>> > TRB_INTR_TARGET(0);
>> >
>> > --
>> > 2.25.1
>> >
\
 
 \ /
  Last update: 2022-11-17 07:22    [W:0.129 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site