From 80224770316966e4339d414c60aea6a5020caa16 Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 00:36:31 +0800 Subject: [PATCH 1/8] dcd_pic: add readme --- src/portable/microchip/pic/README.md | 51 ++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 src/portable/microchip/pic/README.md diff --git a/src/portable/microchip/pic/README.md b/src/portable/microchip/pic/README.md new file mode 100644 index 0000000000..7ba6a4a969 --- /dev/null +++ b/src/portable/microchip/pic/README.md @@ -0,0 +1,51 @@ +# Microchip PIC Chipidea FS Driver + +This driver adds support for Microchip PIC microcontrollers with full-speed Chipidea USB peripheral to the TinyUSB stack. It supports the following families: + +- PIC32MX (untested) +- PIC32MM +- PIC32MK (untested) +- PIC24FJ +- PIC24EP (untested) +- dsPIC33EP (untested) + +Currently only the device mode is supported. + + +## Important Notes + +### Handling of shared VBUS & GPIO pin + +Some PICs have the USB VBUS pin bonded with a GPIO pin in the chip package. This driver does **NOT** handle the potential conflict between the VBUS and GPIO functionalities. + +Developers must ensure that the GPIO pin is tristated when the VBUS pin is managed by the USB peripheral in order to prevent damaging the chip. + +This design choice allows developers the flexibility to use the GPIO functionality for controlling VBUS in device mode if desired. + + +## TODO + +### Handle USB remote wakeup timing correctly + +The Chipidea FS IP doesn't handle the RESUME signal automatically and it must be managed in software. It needs to be asserted for exactly 10ms, and this is impossible to do without per-device support due to BSP differences. For now, a simple for-based loop is used. + +### 8-bit PIC support + +The 8-bit PICs also uses the Chipidea FS IP. Technically it's possible to support them as well. + +Possible difficulties: +- Memory size constraints (1KB/8KB ballpark) +- A third BDT layout (now we have two) +- Different compiler-specific directives +- Compiler bugs if you use SDCC + + +## Author +[ReimuNotMoe](https://github.com/ReimuNotMoe) at SudoMaker, Ltd. + + +## Credits + +This driver is based on: +- Microchip's USB driver (usb_device.c) +- TinyUSB's NXP KHCI driver (dcd_khci.c) From 99e6b32a7da695f16a2fed93c59bfa0d5a1952dc Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 00:36:42 +0800 Subject: [PATCH 2/8] dcd_pic: change license header and credit people accordingly --- src/portable/microchip/pic/dcd_pic.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index d40c4794a8..461c652597 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -1,8 +1,11 @@ /* * The MIT License (MIT) * - * Copyright (c) 2020 Koji Kitayama - * Copyright (c) 2022 Reimu NotMoe + * Copyright (c) 2022-2024 SudoMaker, Ltd. + * Author: Mike Yang (Reimu NotMoe) + * + * Based on usb_device.c - Copyright (c) 2015 Microchip Technology Inc. + * Based on dcd_khci.c - Copyright (c) 2020 Koji Kitayama * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal From 8907a817a2a782423bf8d6db2136147b6e334225 Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 00:50:32 +0800 Subject: [PATCH 3/8] dcd_pic: handle bus resume correctly --- src/portable/microchip/pic/dcd_pic.c | 58 +++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index 461c652597..0dac185238 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -741,8 +741,11 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) //--------------------------------------------------------------------+ void dcd_int_handler(uint8_t rhport) { - uint32_t is = U1IR; - uint32_t msk = U1IE; + uint32_t is, msk; + + // Part 1 - "USB interrupts" + is = U1IR; + msk = U1IE; U1IR = is & ~msk; is &= msk; @@ -750,7 +753,7 @@ void dcd_int_handler(uint8_t rhport) if (is & _U1IR_UERRIF_MASK) { uint32_t es = U1EIR; U1EIR = es; - U1IR = is; /* discard any pending events */ + U1IR = is; /* discard any pending events */ } if (is & _U1IR_URSTIF_MASK) { @@ -761,29 +764,66 @@ void dcd_int_handler(uint8_t rhport) if (is & _U1IR_IDLEIF_MASK) { // Note Host usually has extra delay after bus reset (without SOF), which could falsely // detected as Sleep event. Though usbd has debouncing logic so we are good + + /* + * NOTE: Do not clear U1OTGIRbits.ACTVIF here! + * Reason: + * ACTVIF is only generated once an IDLEIF has been generated. + * This is a 1:1 ratio interrupt generation. + * For every IDLEIF, there will be only one ACTVIF regardless of + * the number of subsequent bus transitions. + * + * If the ACTIF is cleared here, a problem could occur when: + * [ IDLE ][bus activity -> + * <--- 3 ms -----> ^ + * ^ ACTVIF=1 + * IDLEIF=1 + * # # # # (#=Program polling flags) + * ^ + * This polling loop will see both + * IDLEIF=1 and ACTVIF=1. + * However, the program services IDLEIF first + * because ACTIVIE=0. + * If this routine clears the only ACTIVIF, + * then it can never get out of the suspend + * mode. + */ + U1OTGIESET = _U1OTGIE_ACTVIE_MASK; U1IR = _U1IR_IDLEIF_MASK; process_bus_sleep(rhport); } - if (is & _U1IR_RESUMEIF_MASK) { - U1IR = _U1IR_RESUMEIF_MASK; - process_bus_resume(rhport); - } - if (is & _U1IR_SOFIF_MASK) { U1IR = _U1IR_SOFIF_MASK; dcd_event_bus_signal(rhport, DCD_EVENT_SOF, true); } if (is & _U1IR_STALLIF_MASK) { - U1IR = _U1IR_STALLIF_MASK; process_stall(rhport); + U1IR = _U1IR_STALLIF_MASK; } if (is & _U1IR_TRNIF_MASK) { process_tokdne(rhport); } + // Part 2 - "USB OTG interrupts" + is = U1OTGIR; + msk = U1OTGIE; + + U1OTGIR = is & ~msk; + is &= msk; + + if (is & _U1OTGIR_ACTVIF_MASK) { +#if TU_PIC_INT_SIZE == 4 + U1OTGIECLR = _U1OTGIE_ACTVIE_MASK; +#else + U1OTGIE &= ~_U1OTGIE_ACTVIE_MASK; +#endif + U1OTGIR = _U1OTGIR_ACTVIF_MASK; + process_bus_resume(rhport); + } + intr_clear(rhport); } From f409472998cfba4f6be0534912c1afd48e0d6816 Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 00:57:22 +0800 Subject: [PATCH 4/8] dcd_pic: handle remote wakeup more correctly --- src/portable/microchip/pic/dcd_pic.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index 0dac185238..9ffeb08110 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -532,8 +532,25 @@ void dcd_remote_wakeup(uint8_t rhport) #else U1CONbits.RESUME = 1; #endif - unsigned cnt = 25000000 / 1000; + + // FIXME: Assert RESUME signal correctly, requires device-specific handling + // For now we use a hardcoded cycle-based delay which attempts to delay 10ms + // at the most common CPU frequencies. On PIC32s we assume the loop body + // takes 3 cycles. On 16-bit PICs we assume the XC16 compiler is in use and + // use its `__delay_ms' function. + +#if CFG_TUSB_MCU == OPT_MCU_PIC32MM + uint32_t cnt = 24000000 / 1000 / 3; + while (cnt--) asm volatile("nop"); +#elif CFG_TUSB_MCU == OPT_MCU_PIC32MX + uint32_t cnt = 40000000 / 1000 / 3; while (cnt--) asm volatile("nop"); +#elif CFG_TUSB_MCU == OPT_MCU_PIC32MK + uint32_t cnt = 120000000 / 1000 / 3; + while (cnt--) asm volatile("nop"); +#else + __delay_ms(10); +#endif #if TU_PIC_INT_SIZE == 4 U1CONCLR = _U1CON_RESUME_MASK; From 0192b2a9b0a8a7a300326c27ebd59f15cb34790e Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 00:59:45 +0800 Subject: [PATCH 5/8] dcd_pic: implement dcd_deinit() --- src/portable/microchip/pic/dcd_pic.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index 9ffeb08110..04c54059f7 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -508,6 +508,19 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { return true; } +bool dcd_deinit(uint8_t rhport) +{ + U1CON = 0; + U1IE = 0; + U1OTGIE = 0; +#if TU_PIC_INT_SIZE == 4 + U1PWRCCLR = _U1PWRC_USUSPEND_MASK | _U1PWRC_USBPWR_MASK; +#else + U1PWRC &= ~(_U1PWRC_USUSPEND_MASK | _U1PWRC_USBPWR_MASK); +#endif + return true; +} + void dcd_int_enable(uint8_t rhport) { intr_enable(rhport); From 6e1140683140eb7bb80dfae0d207a920dec6d12c Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 01:02:10 +0800 Subject: [PATCH 6/8] dcd_pic: handle EP0 timeout/stall correctly --- src/portable/microchip/pic/dcd_pic.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index 04c54059f7..4e0bd735e1 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -316,12 +316,23 @@ static void prepare_next_setup_packet(uint8_t rhport) { const unsigned out_odd = _dcd.endpoint[0][0].odd; const unsigned in_odd = _dcd.endpoint[0][1].odd; - TU_ASSERT(0 == _dcd.bdt[0][0][out_odd].own, ); + + // Abandon any previous control transfers that might have been using EP0. + // Ordinarily, nothing actually needs abandoning, since the previous control + // transfer would have completed successfully prior to the host sending the + // next SETUP packet. However, in a timeout error case, or after an EP0 + // STALL event, one or more UOWN bits might still be set. If so, we should + // clear the UOWN bits, so the EP0 IN/OUT endpoints are in a known inactive + // state, ready for re-arming by the `dcd_edpt_xfer' function that will be + // called next. _dcd.bdt[0][0][out_odd].data = 0; + _dcd.bdt[0][0][out_odd].own = 0; _dcd.bdt[0][0][out_odd ^ 1].data = 1; _dcd.bdt[0][1][in_odd].data = 1; + _dcd.bdt[0][1][in_odd].own = 0; _dcd.bdt[0][1][in_odd ^ 1].data = 0; + _dcd.bdt[0][1][in_odd ^ 1].own = 0; dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_OUT), _dcd.setup_packet, sizeof(_dcd.setup_packet)); } From a4169114ec6acb07cf8e26a1718e61c0c7af69fa Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 01:04:29 +0800 Subject: [PATCH 7/8] dcd_pic: let the user manage shared GPIO/VBUS pin --- src/portable/microchip/pic/dcd_pic.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index 4e0bd735e1..a4fa0e8406 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -489,10 +489,6 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { intr_disable(rhport); intr_clear(rhport); -#if CFG_TUSB_MCU == OPT_MCU_PIC32MM - TRISBbits.TRISB6 = 1; -#endif - tu_memclr(&_dcd, sizeof(_dcd)); #if TU_PIC_INT_SIZE == 4 From 655092d568720188a3a34b9376a7ebcf274dab79 Mon Sep 17 00:00:00 2001 From: Reimu NotMoe Date: Fri, 27 Dec 2024 04:17:26 +0800 Subject: [PATCH 8/8] dcd_pic: check USBBUSY bit on PIC32s --- src/portable/microchip/pic/dcd_pic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/portable/microchip/pic/dcd_pic.c b/src/portable/microchip/pic/dcd_pic.c index a4fa0e8406..b4a6981992 100644 --- a/src/portable/microchip/pic/dcd_pic.c +++ b/src/portable/microchip/pic/dcd_pic.c @@ -492,6 +492,9 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { tu_memclr(&_dcd, sizeof(_dcd)); #if TU_PIC_INT_SIZE == 4 + // The USBBUSY bit is present on PIC32s and we're required to check it + // prior to powering on the USB peripheral (see DS61126F page 27) + while (U1PWRCbits.USBBUSY); U1PWRCSET = _U1PWRC_USBPWR_MASK; #else U1PWRCbits.USBPWR = 1;