drivers: usb: Add RP2040 USB device support. #42506

petejohanson · 2022-02-06T05:02:59Z

Initial work on the USB device driver for RaspberryPi Pico SoC.

I'm quite certain there are some gaps and missing bits, and some that just need cleaning up, perhaps with feedback from others that know the stack better than I.

drivers/usb/device/usb_dc_rpi_pico.c

jfischer-no · 2022-02-10T08:40:24Z

drivers/usb/device/usb_dc_rpi_pico.c

+
+/* Helper macros to make it easier to work with endpoint numbers */
+#define EP0_IDX 0
+#define EP0_IN (EP0_IDX | USB_EP_DIR_IN)


Please use USB_CONTROL_EP_OUT, USB_CONTROL_EP_OUT, USB_CONTROL_EP_MPS from include/usb/usb_ch9.h

Done. Did keep the separate DATA_BUFFER_SIZE define, since that's used beyond just EP0 usage.

drivers/usb/device/usb_dc_rpi_pico.c

jfischer-no · 2022-02-10T09:09:33Z

drivers/usb/device/usb_dc_rpi_pico.c

+	if (status & USB_INTS_SETUP_REQ_BITS) {
+		handled |= USB_INTS_SETUP_REQ_BITS;
+		usb_hw_clear->sie_status = USB_SIE_STATUS_SETUP_REC_BITS;
+		usb_dc_rpi_pico_handle_setup();


Status and endpoint callbacks processing should be offloaded to a thread/workqueue, otherwise device stack, class or application code would be executed in drivers ISR context, see usb_dc_mcux or usb_dc_kinetis driver

Before doing this.... Any reason to use a dedicated thread, versus submitting to the main system work queue?

Reviewed this, and went w/ a dedicated thread, like the other drivers. Feedback welcome.

dts/arm/rpi_pico/rp2040.dtsi

jfischer-no

Sorry for delay, I finally had time to prepare the hardware so that I can flash it using DAPLink probe and pyOCD and test this driver. Basically it seems to work, let it polish and get in ASAP.

drivers/usb/device/usb_dc_rpi_pico.c

jfischer-no · 2022-03-09T11:42:30Z

drivers/usb/device/usb_dc_rpi_pico.c

+		msg.cb = usb_hw->sie_status & USB_SIE_STATUS_CONNECTED_BITS ?
+			USB_DC_CONNECTED :
+			USB_DC_DISCONNECTED;


This, CONNECTED vs. DISCONNECTED, seems to be reversed.

I'm not sure it is? Seems like the connected status bit being present should be USB_DC_CONNECTED.

I observed it like this.

Ok, I will swap, I haven't had a chance to test this myself directly.

drivers/usb/device/usb_dc_rpi_pico.c

jfischer-no · 2022-03-09T11:51:59Z

drivers/usb/device/usb_dc_rpi_pico.c

+		} else if (state.status_cb) {
+			switch (msg.cb) {
+			case USB_DC_RESET:
+				state.status_cb(USB_DC_RESET, NULL);
+				break;
+			case USB_DC_ERROR:
+				state.status_cb(USB_DC_ERROR, NULL);
+				break;
+			case USB_DC_SUSPEND:
+				state.status_cb(USB_DC_SUSPEND, NULL);
+				break;
+			case USB_DC_RESUME:
+				state.status_cb(USB_DC_RESUME, NULL);
+				break;
+			default:
+				LOG_ERR("unknown msg");
+				break;
+			}


diff --git a/drivers/usb/device/usb_dc_rpi_pico.c b/drivers/usb/device/usb_dc_rpi_pico.c index 84bc8f0cbea..48577e668be 100644 --- a/drivers/usb/device/usb_dc_rpi_pico.c +++ b/drivers/usb/device/usb_dc_rpi_pico.c @@ -793,26 +793,12 @@ static void udc_rpi_thread_main(void *arg1, void *unused1, void *unused2) } break; default: - LOG_ERR("unknown msg"); + LOG_ERR("unknown ep msg"); break; } - } else if (state.status_cb) { - switch (msg.cb) { - case USB_DC_RESET: - state.status_cb(USB_DC_RESET, NULL); - break; - case USB_DC_ERROR: - state.status_cb(USB_DC_ERROR, NULL); - break; - case USB_DC_SUSPEND: - state.status_cb(USB_DC_SUSPEND, NULL); - break; - case USB_DC_RESUME: - state.status_cb(USB_DC_RESUME, NULL); - break; - default: - LOG_ERR("unknown msg"); - break; + } else { + if (state.status_cb) { + state.status_cb(msg.cb, NULL); } } }

Refactored this to simplify greatly. Leaving open until you review again.

yonsch · 2022-03-16T09:37:02Z

Please also add USB to the table of supported features in the board docs

bzyx · 2022-03-18T12:08:48Z

drivers/usb/device/usb_dc_rpi_pico.c

+
+		if (data) {
+			memcpy(ep_state->buf, data, len);
+			val |= USB_BUF_CTRL_FULL;


val |= USB_BUF_CTRL_FULL; should go outside of the condition, otherwise, if the packet size is a multiplicity of the buffer size then the transfer will stall. This is a zero-length-packet.

Moved this out. Thanks!

jfischer-no · 2022-04-14T15:02:24Z

Summary for the dev-review:

raspberry-pi-pico (abbreviation rpi-pico) is the name of the board "Raspberry Pi Pico is a tiny, fast, and versatile board built using RP2040"
board name in Zephyr OS is rpi_pico
SoC name is RP2040
all the drivers in the tree for RP2040 SoC have rpi_pico postfix
comment from @yonsch "The "pico" naming was chosen because the SDK is named "pico-sdk", so we assumed pico is the name of the series, not just the board." (See discussion in the review drivers/usb/device/usb_dc_rpi_pico.c)

rpi_pico postfix can be confusing if someone is explicitly looking for the SoC name. Should we keep naming scheme or rename?

dev-review result: overwhelming majority has voted to keep rpi_pico postfix.

petejohanson · 2022-05-04T02:04:38Z

@jfischer-no nudge? Anything else you'd like to see here?

jfischer-no · 2022-05-04T07:23:21Z

@jfischer-no nudge? Anything else you'd like to see here?

Sorry, too busy. I will try to finish my review today or tomorrow.

jfischer-no

LGTM, just few comments, see petejohanson#4, please squash without my comments if you agree with the changes.

boards/arm/rpi_pico/rpi_pico.dts

drivers/usb/device/Kconfig

drivers/usb/device/usb_dc_rpi_pico.c

yonsch

Just a few minor things

dts/bindings/usb/raspberrypi,pico-usbd.yaml

drivers/usb/device/usb_dc_rpi_pico.c

jfischer-no · 2022-05-13T08:50:10Z

@petejohanson today last chance to get it in v3.1.0 (I do not want to push you, just as a hint 😄)

petejohanson

Addressed the comments @jfischer-no

Thanks for the nudge on the timing!

boards/arm/rpi_pico/rpi_pico.dts

drivers/usb/device/Kconfig

dts/bindings/usb/raspberrypi,pico-usbd.yaml

petejohanson · 2022-05-13T13:22:29Z

Actually, missed the other comments. Working on those now.

petejohanson · 2022-05-13T13:34:03Z

@jfischer-no Ok, now all the comments have actually been resolved. Thanks again for the review and the nudge.

Add USB device driver for Rasberry Pico family of controllers. Signed-off-by: Peter Johanson <[email protected]>

petejohanson · 2022-05-13T13:49:37Z

(my last few pushes were just to make the compliance check happy, FYI)

yonsch

LGTM!

jfischer-no

👍

github-actions bot added area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding labels Feb 6, 2022

jfischer-no added area: Drivers area: USB Universal Serial Bus labels Feb 10, 2022

jfischer-no added this to the v3.1.0 milestone Feb 10, 2022

jfischer-no self-assigned this Feb 10, 2022

jfischer-no self-requested a review February 10, 2022 08:28

jfischer-no reviewed Feb 10, 2022

View reviewed changes

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from 651d0af to 0dd4490 Compare February 10, 2022 19:06

github-actions bot added the area: Boards label Feb 10, 2022

petejohanson force-pushed the drivers/rpi-pico-usb-device branch 10 times, most recently from c71e9b9 to 5f0b8d8 Compare February 11, 2022 05:06

petejohanson marked this pull request as ready for review February 11, 2022 05:09

petejohanson mentioned this pull request Feb 11, 2022

rp2040 (raspberry pi pico) support tracking issue zmkfirmware/zmk#1085

Open

burumaj reviewed Feb 12, 2022

View reviewed changes

dts/arm/rpi_pico/rp2040.dtsi Outdated Show resolved Hide resolved

petejohanson force-pushed the drivers/rpi-pico-usb-device branch 2 times, most recently from ac931af to 38fb1d0 Compare February 17, 2022 05:04

jfischer-no self-requested a review February 23, 2022 11:03

jfischer-no requested changes Mar 9, 2022

View reviewed changes

bzyx reviewed Mar 18, 2022

View reviewed changes

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from 38fb1d0 to 588084f Compare April 13, 2022 14:44

github-actions bot added the area: Documentation label Apr 13, 2022

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from 53665e1 to 7c73b26 Compare April 13, 2022 15:08

petejohanson requested a review from jfischer-no April 13, 2022 15:08

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from 7c73b26 to bab74cb Compare April 13, 2022 15:20

jfischer-no added the dev-review To be discussed in dev-review meeting label Apr 13, 2022

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from bab74cb to 119f26d Compare April 14, 2022 04:00

MaureenHelm removed the dev-review To be discussed in dev-review meeting label Apr 14, 2022

yonsch added the platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) label May 1, 2022

jfischer-no reviewed May 6, 2022

View reviewed changes

yonsch requested changes May 7, 2022

View reviewed changes

dts/bindings/usb/raspberrypi,pico-usbd.yaml Outdated Show resolved Hide resolved

drivers/usb/device/usb_dc_rpi_pico.c Outdated Show resolved Hide resolved

drivers/usb/device/usb_dc_rpi_pico.c Outdated Show resolved Hide resolved

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from 119f26d to 4872dc7 Compare May 13, 2022 13:21

petejohanson commented May 13, 2022

View reviewed changes

boards/arm/rpi_pico/rpi_pico.dts Outdated Show resolved Hide resolved

drivers/usb/device/Kconfig Outdated Show resolved Hide resolved

dts/bindings/usb/raspberrypi,pico-usbd.yaml Outdated Show resolved Hide resolved

petejohanson force-pushed the drivers/rpi-pico-usb-device branch 2 times, most recently from a414774 to 1d4c746 Compare May 13, 2022 13:32

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from 1d4c746 to a8f6bef Compare May 13, 2022 13:37

jfischer-no requested review from yonsch and jfischer-no May 13, 2022 13:42

drivers: usb: Add RP2040 USB device support.

0593f37

Add USB device driver for Rasberry Pico family of controllers. Signed-off-by: Peter Johanson <[email protected]>

petejohanson force-pushed the drivers/rpi-pico-usb-device branch from a8f6bef to 0593f37 Compare May 13, 2022 13:49

yonsch approved these changes May 13, 2022

View reviewed changes

jfischer-no approved these changes May 13, 2022

View reviewed changes

carlescufi merged commit 43b77a2 into zephyrproject-rtos:main May 13, 2022

soburi mentioned this pull request Jan 16, 2023

TaskList for Supporting RaspberryPi Pico peripherals #53810

Open

52 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: usb: Add RP2040 USB device support. #42506

drivers: usb: Add RP2040 USB device support. #42506

petejohanson commented Feb 6, 2022

jfischer-no Feb 10, 2022

petejohanson Feb 10, 2022

jfischer-no Feb 10, 2022

petejohanson Feb 10, 2022

petejohanson Feb 11, 2022

jfischer-no left a comment •

edited

Loading

jfischer-no Mar 9, 2022

petejohanson Mar 10, 2022

jfischer-no Mar 15, 2022

petejohanson Apr 13, 2022

jfischer-no Mar 9, 2022

petejohanson Apr 13, 2022

yonsch commented Mar 16, 2022

bzyx Mar 18, 2022

petejohanson Apr 13, 2022

jfischer-no commented Apr 14, 2022 •

edited

Loading

petejohanson commented May 4, 2022

jfischer-no commented May 4, 2022

jfischer-no left a comment

yonsch left a comment

jfischer-no commented May 13, 2022

petejohanson left a comment

petejohanson commented May 13, 2022

petejohanson commented May 13, 2022

petejohanson commented May 13, 2022

yonsch left a comment

jfischer-no left a comment

drivers: usb: Add RP2040 USB device support. #42506

drivers: usb: Add RP2040 USB device support. #42506

Conversation

petejohanson commented Feb 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfischer-no left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yonsch commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfischer-no commented Apr 14, 2022 • edited Loading

petejohanson commented May 4, 2022

jfischer-no commented May 4, 2022

jfischer-no left a comment

Choose a reason for hiding this comment

yonsch left a comment

Choose a reason for hiding this comment

jfischer-no commented May 13, 2022

petejohanson left a comment

Choose a reason for hiding this comment

petejohanson commented May 13, 2022

petejohanson commented May 13, 2022

petejohanson commented May 13, 2022

yonsch left a comment

Choose a reason for hiding this comment

jfischer-no left a comment

Choose a reason for hiding this comment

jfischer-no left a comment •

edited

Loading

jfischer-no commented Apr 14, 2022 •

edited

Loading