Skip to content
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

tinyUSB: Upgraded tinyUSB #6325

Closed

Conversation

alphaFred
Copy link
Contributor

Upgraded tinyUSB to latest commit of it's master branch to take over
fixes for high-speed devices With this commit the prelimiary fix done here (#6122) will be resolved.

Performed necessary integration of new library into mimxrt and samd ports.

Changes in tinyUSB API:

  • tud_isr changed to tud_int_handler
  • tud_cdc_write_flush is now returning number of transmitted bytes

@alphaFred alphaFred force-pushed the feature_tinyUSB_bugfix branch 2 times, most recently from 0a2b488 to 7dcde99 Compare August 12, 2020 18:47
@alphaFred alphaFred marked this pull request as ready for review August 12, 2020 19:54
Upgraded tinyUSB to latest commit of it's master branch to take over
fixes for high-speed devices.

Integrated new library into mimxrt and samd ports.
@iabdalkader
Copy link
Contributor

Note if this is the latest tinyusb you also need to call the IRQ handler in the nrf port:

void USBD_IRQHandler(void)
{
    dcd_int_handler(0);
}

@dpgeorge
Copy link
Member

dpgeorge commented Feb 9, 2021

Thanks for this, and sorry it got completely neglected.

I tested it on mimxrt and samd and it has reliability problems. But actually the original code had such problems and it really needs to be fixed properly.

while (!tud_cdc_write_flush()) {
__WFI();
}
(void)tud_cdc_write_flush();
Copy link
Member

Choose a reason for hiding this comment

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

This call needs to run in a loop until there is enough space to write. Otherwise there is now out-going flow control and this loop can overflow the CDC.

@dpgeorge
Copy link
Member

This PR is superseded by commits 9b7d8b8, 035d161

@dpgeorge dpgeorge closed this Feb 12, 2021
@dpgeorge
Copy link
Member

Note if this is the latest tinyusb you also need to call the IRQ handler in the nrf port:

@iabdalkader it certainly seems like the nrf port should add USBD_IRQHandler, but it seems to work correctly without it (using tinyusb 0.8.0). At least it works for me on a Particle Xenon board (nRF52840).

Do you have any other nrf-USB based boards that you can test the latest code on to see if USB CDC works for you?

@iabdalkader
Copy link
Contributor

iabdalkader commented Feb 12, 2021

Do you have any other nrf-USB based boards that you can test the latest code on to see if USB CDC works for you?

@dpgeorge Yes I do, Nano 33 BLE, with a clean checkout I tested the latest changes and it doesn't work unless I add this to mphalport.c:

#include "usb_cdc.h"
void USBD_IRQHandler(void)
{
    dcd_int_handler(0);
}

I have board files for Nano if you'd like to test.

EDIT: from the porting.md:

##### dcd_int_handler
Processes all the hardware generated events e.g Bus reset, new data packet from host etc ... It will be called by application in the MCU USB interrupt handler.

And the change log in 0.7.0:

- Rename USB IRQ Handler to `dcd_int_handler()`. Application must define IRQ handler in which it calls this API.

Note tud_int_handler is just a define for dcd_int_handler:

#define tud_int_handler   dcd_int_handler

dpgeorge added a commit that referenced this pull request Feb 13, 2021
This is needed for TinyUSB to process USB device IRQs.

Related to #6325.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Member

Yes I do, Nano 33 BLE, with a clean checkout I tested the latest changes and it doesn't work unless I add this to mphalport.c:

Ok, thanks for checking. I added this IRQ handler in 701fdca

@alphaFred alphaFred deleted the feature_tinyUSB_bugfix branch April 17, 2021 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants