search

CommunityGD32Cores / ArduinoCore-GD32

Arduino core for GD32 devices, community developed, based on original GigaDevice's core
Other
89 stars 33 forks source link

"Add" support for gd32f10x board #109

Open dmlambo opened 1 year ago

dmlambo commented 1 year ago

Don't merge, please! This is for discussion of adding the F10x boards to the Arduino core!

Sorry for the mess, not my best work, but these are some hacks and required changes to get a build compiling for a gd32f103rc (specifically. you may have to add your own variant directories from your platform-gd32 build to get the others to work).

dmlambo commented 1 year ago

Assume everything is done just for getting the one board compiling. I posted this at the behest of @maxgerhardt just to get the ball rolling. Best thing to do would be to ignore the logic, but note the requirement.

dmlambo commented 1 year ago

Another little update. The USBD implementation for the F103 is subtly different from other MCUs. In the end there are only a handful of changes required to get CDC/Serial to work, but the amount of work to find those changes was soul-crushing. Before I push the changes I'm just going to describe what I did real quick:

The real bugger was Windows (or maybe my USB host) was clever enough to read the device descriptor and continue onto reading the config descriptor, which failed... so from my perspective the device was "behaving" up until config when it really wasn't. Thank god for Raspberry Pis, I was able to use one to debug the packets that the Windows USBPcap wasn't showing me.

Now that it's working, I'll need to take a pass at cleaning up the code and making more other-MCU friendly. 😄

obra commented 1 year ago

Only tangentially related - we've done a ton of work on the USB core in Keyboardio's fork of the arduino core. I don't have the cycles to put together a PR, but I'd love to see all of it upstreamed. We've fixed a wide variety of USB bugs since we submitted the first implementation.

On Thu, Sep 7, 2023, at 12:33 PM, Dave Lamb wrote:

Another little update. The USBD implementation for the F103 is subtly different from other MCUs. In the end there are only a handful of changes required to get CDC/Serial to work, but the amount of work to find those changes was soul-crushing. Before I push the changes I'm just going to describe what I did real quick:

• On my board there is an open-drain software controlled disconnect connected to D+. It's connected through a 150ohm resistor. This overpowers the 1.5k pullup that's in place to bring the D+ low. usb.c assumes this to be a push-pull behind a 1.5k pullup, so I had to mess around a bit to get it to behave. • Thus I had to add some definitions to variant.h (USB_PULLUP, etc.) and/or add some ifdef guards around the errant code. This might need to be moved to the boards generator and added in platformbuild.py. • I had to disable the USB peripheral clock before init, since the bootloader was using a different PLL clock multiplier, and changing it didn't seem to take effect without first turning off the clock. • Finally, and most importantly, I had to mess with the usbd state machine from USBCore.c. There's an additional field, ctl_state, which is usually set just before returning filling out the ep0 buffers and returning from the ISR. Normally done in usb_setup_transc (which has been superseded by USBCore::transcSetup). If this isn't set then the host doesn't get responses to ZLPs, and ep0 stalls. The real bugger was Windows (or maybe my USB host) was clever enough to read the device descriptor and continue onto reading the config descriptor, which failed... so from my perspective the device was "behaving" up until config when it really wasn't. Thank god for Raspberry Pis, I was able to use one to debug the packets that the Windows USBPcap wasn't showing me.

Now that it's working, I'll need to take a pass at cleaning up the code and making more other-MCU friendly. 😄

— Reply to this email directly, view it on GitHub https://github.com/CommunityGD32Cores/ArduinoCore-GD32/pull/109#issuecomment-1710671024, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2FUBFKNNUWKNTT6AULXZIOOZANCNFSM6AAAAAA4B3IMUM. You are receiving this because you commented.Message ID: @.***>

dmlambo commented 1 year ago

Nice. At first glance it looks quite a bit cleaner. It may be worth pulling the code in before I mangle what's currently there. I don't have any other gd32 devices though so I can only test so much, unfortunately.

dmlambo commented 1 year ago

Alright, well, I bit the bullet and just made changes to USBCore. I had to add some delays here and there, and importantly I also need to define some things in platformio.ini:

build_flags = 
  -DPIO_FRAMEWORK_ARDUINO_ENABLE_CDC 
  -DUSB_PULLUP=GPIOA 
  -DUSB_PULLUP_PIN=GPIO_PIN_12 
  -DUSB_PULLUP_PIN_MODE=GPIO_MODE_OUT_OD 
  -DRCC_AHBPeriph_GPIO_PULLUP=RCU_GPIOA

There is a pulldown IO pin on my board, but the bootloader I'm using (stm32duino) doesn't actually use it, and seems to work fine. They instead set PA12 (D+) to PP output, pull it low, then reconfigure to floating. So I tried doing the same to force a reconnect, and it works (in OD or PP, doesn't matter). The only thing is, I needed to add a delay after setting the gpio to open-drain to allow the host to catch the disconnect, since if we enable the peripheral too soon it forces the IO line high before the host realizes anything happened.

Similarly, I had to add a delay after usb_init (there's a note about adding a delay... without a delay) to allow the USB peripheral to... do... something? If we don't wait, I'm thinking the ISR fires too quickly and we change some values before the peripheral is ready. Anyway, it's only adding probably some microseconds to milliseconds to startup, so I don't feel too bad about it.

dmlambo commented 1 year ago

Additional changes added to support the new changes for pin remapping on F10x boards. Keep in mind I only added the GD32F103RC_GENERIC variant to keep the change reasonable.

I also added 36-pin boards, which I had forgotten in the previous change, but again, the variants aren't checked in yet.