ch32 USBHD/USBHS driver implementation#893
ch32 USBHD/USBHS driver implementation#893Copper280z wants to merge 88 commits intoZigEmbeddedGroup:mainfrom
Conversation
…into origin/umain
…oards. ch32 USBHS still sends packets twice.
…ual mode. CDC Works without extra packets!
…ed code in usb_cdc.zig
Lint ResultsFound 12 issues on changed lines in 4 files:
|
…ng to board definitions, add max packet size to usb device hal.
Grazfather
left a comment
There was a problem hiding this comment.
Reviewed everything except for the HALs themselves (since this is a draft). Will get to those when you think they're ready.
| .bcd_usb = config.bcd_usb, | ||
| .device_triple = config.device_triple, | ||
| .max_packet_size0 = @max(config.max_supported_packet_size, 64), | ||
| .max_packet_size0 = @min(config.max_supported_packet_size, 64), |
There was a problem hiding this comment.
I could find it in the spec, but this is correct now. For LS and FS devices endpoint 0 can be 8 to 64 bytes, for HS it's always 64.
Unofficial, but useful reference: https://www.beyondlogic.org/usbnutshell/usb4.shtml
| .{ .target = mb.ports.ch32v.boards.ch32v203.nano_ch32v203, .name = "nano_ch32v203_blinky", .file = "src/board_blinky.zig" }, | ||
| .{ .target = mb.ports.ch32v.boards.ch32v203.nano_ch32v203, .name = "nano_ch32v203_usb_cdc", .file = "src/usb_cdc.zig" }, | ||
| .{ .target = mb.ports.ch32v.boards.ch32v203.suzuduino_uno_v1b, .name = "suzuduino_blinky", .file = "src/board_blinky.zig" }, | ||
| .{ .target = mb.ports.ch32v.boards.ch32v203.suzuduino_uno_v1b, .name = "suzuduino_usb_cdc", .file = "src/usb_cdc.zig" }, |
There was a problem hiding this comment.
nano_ch32v203 is tested, suzuduino is not, but the way the peripheral and pins work there's no board specific configuration. It automatically configures the pins, and there's only one option for the pair, so if usb is exposed on the board and there's an external crystal, it should work.
More testing is always better though, so test away.
| .{ .target = mb.ports.ch32v.boards.ch32v203.lana_tny, .name = "lana_tny_spi_loopback", .file = "src/spi_loopback.zig" }, | ||
| .{ .target = mb.ports.ch32v.boards.ch32v203.lana_tny, .name = "lana_tny_spi_flash_w25q", .file = "src/spi_flash_w25q.zig" }, | ||
| .{ .target = mb.ports.ch32v.boards.ch32v203.lana_tny, .name = "lana_tny_sharp_niceview", .file = "src/sharp_niceview.zig" }, | ||
| .{ .target = mb.ports.ch32v.boards.ch32v203.lana_tny, .name = "lana_tny_usb_cdc", .file = "src/usb_cdc.zig" }, |
There was a problem hiding this comment.
and this one? I have this so I can test it for you.
There was a problem hiding this comment.
Same as above, there's only one set of pins, and the peripheral configures them automatically, but more testing is better, so please do so!
There was a problem hiding this comment.
As mentioned on discord: This device has the x6 bin which is the 'usbd' variant of the usbfs, so it's not (yet) supported.
| pub const usart = @import("usart.zig"); | ||
| pub const spi = @import("spi.zig"); | ||
| pub const dma = @import("dma.zig"); | ||
| pub const usb = @import("usbfs.zig"); |
There was a problem hiding this comment.
Can you add this in the test block as well
There was a problem hiding this comment.
Also, as you noticed: Some variants of this have both USBD and USBHD. Somewhat confusingly you're calling hal.usb usbfs here, but for 3xx usb is hs and usbfs is ufbfs.
| /// HSI (High Speed Internal) oscillator frequency | ||
| /// This is the fixed internal RC oscillator frequency for CH32V30x | ||
| pub const hsi_frequency: u32 = 8_000_000; // 8 MHz | ||
| pub const hse_frequency: u32 = 8_000_000; // 8 MHz |
There was a problem hiding this comment.
This should be board level, since it's external to the chip and may not be present.
| /// selects whether the USBHS 48MHz clock comes from the system PLL clock or the USB PHY, | ||
| /// and enables the AHB clock gate for USBHS. | ||
| /// | ||
| /// Note: The SVD names bit31 as `USBFSSRC`, but the reference manual |
There was a problem hiding this comment.
Can we patch this? Which SVD? All of them?
There was a problem hiding this comment.
At this point it's unclear which name is correct. I should probably either remove the comment or figure it out.
| const tog_pin = gpio.Pin.init(0, 14); | ||
| const mco_pin = gpio.Pin.init(0, 8); |
There was a problem hiding this comment.
Might as well keep with the comments e.g. PA14 and PA8
There was a problem hiding this comment.
These will be removed before I mark it as ready, MCO and toggle are things I was using for testing. MCO is useful for validating PLL stuff with a scope.
| .configurations = &.{.{ | ||
| .attributes = .{ .self_powered = false }, | ||
| .max_current_ma = 50, | ||
| .Drivers = struct { serial: USB_Serial }, |
There was a problem hiding this comment.
Could you maybe copy some of the extra comments I added to the cdc example for rpi?
| pub var usb_dev: hal.usb.Polled( | ||
| .{}, | ||
| ) = undefined; |
There was a problem hiding this comment.
| pub var usb_dev: hal.usb.Polled( | |
| .{}, | |
| ) = undefined; | |
| pub var usb_dev: hal.usb.Polled(.{}) = undefined; |
| // pins.tog.put(0); | ||
| usb_dev.poll(false, &usb_controller); | ||
| // pins.tog.put(1); |
There was a problem hiding this comment.
gets rid of toggle pins completely or uncomment these please
This adds a new driver for the ch32 USBHD/USBHS peripheral, USB_OTG_FS peripheral, and examples for the ch32v307, and ch32v203 boards.