Skip to content

netusb: wait for the transfer to complete before retrying. #6369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

nzmichaelh
Copy link
Collaborator

In the same way as cdc_acm, set a semaphore in the transfer complete
interrupt and wait for it in the main loop before retrying.

This fixes a bug with >= 64 byte frames when logging is off. If
logging is on, then the time to print logging is enough for the
transmit to complete. If logging is off, then the 10 retries complete
quickly and try_write() gives up.

Signed-off-by: Michael Hope [email protected]

In the same way as cdc_acm, set a semaphore in the transfer complete
interrupt and wait for it in the main loop before retrying.

This fixes a bug with >= 64 byte frames when logging is off.  If
logging is on, then the time to print logging is enough for the
transmit to complete.  If logging is off, then the 10 retries complete
quickly and try_write() gives up.

Signed-off-by: Michael Hope <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #6369 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6369   +/-   ##
=======================================
  Coverage   53.08%   53.08%           
=======================================
  Files         430      430           
  Lines       41040    41040           
  Branches     7908     7908           
=======================================
  Hits        21785    21785           
  Misses      16032    16032           
  Partials     3223     3223

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0985fa...bbf0470. Read the comment docs.

@nzmichaelh
Copy link
Collaborator Author

Friendly ping...

@jfischer-no jfischer-no requested a review from finikorg March 14, 2018 17:38
@jfischer-no jfischer-no added the area: USB Universal Serial Bus label Mar 14, 2018
@@ -188,6 +196,8 @@ int try_write(u8_t ep, u8_t *data, u16_t len)
*/
if (tries--) {
SYS_LOG_WRN("Error: EAGAIN. Another try");
/* Wait for the transfer to complete */
k_sem_take(&netusb.bulk_in_sem, K_MSEC(100));
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check, could the issue be solved by k_yield() (or even k_sleep() despite it does not look nice)? BTW try_write() would be replaced by usb_transfer(), there is a patch for ECM in PR #5983 .

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, thanks.

re: k_yield() it might reduce the problem but it's hard to say. USB is fast so you don't need to spend much time for the transfer to have completed - for example, if logging is on then the SYS_LOG_WRN() is enough.

re: transfers, I'll look into it.

@nzmichaelh
Copy link
Collaborator Author

This was obsoleted by ccf0b46

@nzmichaelh nzmichaelh closed this Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: USB Universal Serial Bus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants