Skip to content

nrf: Add pca10059 with tinyusb adaptation. #5112

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

nrf: Add pca10059 with tinyusb adaptation. #5112

wants to merge 3 commits into from

Conversation

glennrub
Copy link
Contributor

No description provided.

@glennrub glennrub mentioned this pull request Sep 23, 2019
Closed
@glennrub glennrub force-pushed the pca10059_tinyusb branch 3 times, most recently from a14e8e8 to cda9077 Compare September 26, 2019 18:37
@glennrub
Copy link
Contributor Author

Rebased and amended some documentation.

I also realized that the SoftDevice enable hack i had to do to get SD working already was in the PR. So, the current code here should work for both with and without SD.

glennrub
glennrub commented Sep 26, 2019
View reviewed changes
ports/nrf/drivers/usb/usb_cdc.c

#if BLUETOOTH_SD
// Initialize the clock and BLE stack.
ble_drv_stack_enable();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mentioned hack to get USB events propagated in case of compilation with SD=s140.

glennrub
glennrub commented Sep 26, 2019
View reviewed changes
ports/nrf/drivers/usb/usb_cdc.c Outdated
static void cdc_task(void);

uint8_t rx_ringbuf_array[4096];
uint8_t tx_ringbuf_array[4096];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've put quite large buffers to handle good REPL paste feeling. This might be a bit too much.

Copy link
Member

Choose a reason for hiding this comment

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

stm32 uses 1024 bytes for each, so probably that would be plenty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amended change to use 1024, ringbuffer size. Also made this group of variables static. Removed unused rx_buf and tx_buf.

glennrub
glennrub commented Sep 26, 2019
View reviewed changes
ports/nrf/boards/pca10059/mpconfigboard.h Outdated
#define MICROPY_PY_MACHINE_TEMP (1)
#define MICROPY_PY_RANDOM_HW_RNG (1)

#define USB_CDC (1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestion on better name for this define? I know it is a bit inconsistent, but i could not really settle on what to call it.

Copy link
Member

Choose a reason for hiding this comment

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

On stm32 it would be MICROPY_HW_USB_CDC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! I wonder how i slipped on that one. I tried to find it =) Changed the use of USB_CDC to MICROPY_HW_USB_CDC.

@dpgeorge
Copy link
Member

Thanks @glennrub, nice feature! I want to test this with a Particle Xenon board.

@glennrub
Copy link
Contributor Author

I want to test this with a Particle Xenon board.

Great @dpgeorge! I have also used pca10056 during development. It's a matter of setting the MICROPY_HW_USB_CDC (1), and it should spin on that board as well.

@glennrub
Copy link
Contributor Author

On some Linux systems the Modem Manager (MM) kicks in. To prevent the MM from sending AT commands to the device at initialization, a udev rule has to be applied.

My current /etc/udev/rules.d/99-tinyusb.rules

ACTION!="add", SUBSYSTEM!="usb_device", GOTO="tinyusb_rules_end"
ATTR{idProduct}=="9802", ATTR{idVendor}=="f055", MODE="666"
ATTR{idProduct}=="9802", ATTR{idVendor}=="f055", ENV{ID_MM_DEVICE_IGNORE}="1"
LABEL="tinyusb_rules_end"

I'm not sure, should this be part of the source? Or is this kind of well known?

@dpgeorge
Copy link
Member

I'm not sure, should this be part of the source? Or is this kind of well known?

It's kind of well known. At least the stm32 port has had USB CDC since the beginning and this has not been a big issue. Of course, we can document it if needed.

@dhylands
Copy link
Contributor

It is sort of documented here: https://github.com/micropython/micropython/wiki/Board-STM32F407-Discovery#programming-from-linux-via-dfu
And I think that there are links to that in the forum.

@dpgeorge
Copy link
Member

dpgeorge commented Oct 9, 2019

I want to test this with a Particle Xenon board.

I tested this PR with a Xenon (using #5158) and it didn't work. The PC could see that there was a new USB device attached but it could not enumerate it. I'm not sure what is wrong.

@glennrub I don't want to let this failure hold up the PR. If it works for you on the PCA 56/59 boards then I'm happy to put this code in master.

It'll need to be rebased and conflicts resolved (they are minor).

Also, does it make sense to split this PR into two commits, one for adding PCA10059 and one for integrating TinyUSB? (It doesn't matter which order, maybe add TinyUSB first so PCA10059 can use it.)

@glennrub
Copy link
Contributor Author

glennrub commented Oct 9, 2019

@dpgeorge ,

I tested this PR with a Xenon (using #5158) and it didn't work. The PC could see that there was a new USB device attached but it could not enumerate it. I'm not sure what is wrong.

You might have been hit by the "SD enable" issue i try to workaround. Did you compile it with SD=s140?

Also, does it make sense to split this PR into two commits, one for adding PCA10059 and one for integrating TinyUSB?

Sure, will do! Good suggestion!

I would also like to remove the text i added about "PogoProg" in the readme before we merge. The name of the tool is a bit false leading, as it is just a board with pins, not including the programmer itself. This might get confusing if people actually ordered the PogoProg and expected a programmer. I'll try to do the update today.

@dpgeorge
Copy link
Member

dpgeorge commented Oct 9, 2019

You might have been hit by the "SD enable" issue i try to workaround. Did you compile it with SD=s140?

No, I didn't use SD at all. Just compiled with make BOARD=particle_xenon.

@glennrub
nrf: Add tinyusb support for nrf52840.
09a414c 
Add nrf-port finyusb driver files. USB CDC can be activated
by board configuration files using the MICROPY_HW_USB_CDC.

Updating BLE driver, Makefile, nrfx-glue and main.c to plug
in the tinyusb stack.
@glennrub
nrf/boards: Add support for pca10059.
5d7fc9d 
Add support for pca10059 with REPL over tinyusb USB CDC.

The board also includes a board specific module that will
recover UICR->REGOUT0 in case this has been erased.

This initial support does not preserve any existing bootloader
on the pca10090 in case this was present, and expects to use all
available flash on the device.
@dpgeorge
Copy link
Member

Thanks for splitting the commits, looks good!

@glennrub
Copy link
Contributor Author

Merged in 1571120 to 01a3110.

@glennrub glennrub closed this Oct 10, 2019
@glennrub glennrub deleted the pca10059_tinyusb branch October 10, 2019 19:47
cdwilson pushed a commit to cdwilson/micropython that referenced this pull request Aug 10, 2021
@microdev1
Merge pull request micropython#5112 from NathanY3G/add-board-led-to-gcm4
f6d7b87 
Add board.LED for Grand Central M4 Express
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpgeorge dpgeorge dpgeorge left review comments

Assignees
No one assigned
Labels
port-nrf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@glennrub @dpgeorge @dhylands