Skip to content
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.

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

Add support for ADI MAX32690 microcontroller #9667

Open
wants to merge 37 commits into
base: main
Choose a base branch
from

Conversation

Brandon-Hurst
Copy link

@Brandon-Hurst Brandon-Hurst commented Sep 27, 2024

Description

Add support for 2 boards: MAX32690EVKIT and AD-APARD32690-SL.

Status

At the moment, this port only includes digitalio in addition to the supervisor and USB workflow required to get CircuitPython functioning. Pinouts listed are currently just the MCU pins; happy to change as needed to better match any requirements for form factor or pin function. I plan to follow this up with a busio PR soon, but will be more tied up in the next couple months. Still happy to make changes as needed to this PR though.

USB Endpoint Enumeration Change

There is a small important bit I had to change to the way CircuitPython handles settings up USB Endpoints:

  • TUSB has some devices (such as MAX32) which are not capable of having full-duplex endpoints. This is true for a few devices besides MAX32, but also for MAX32 (in particular a lot of others share the same IP).
  • There is a #define for TUD_ENDPOINT_ONE_DIRECTION_ONLY accounted for by TinyUSB, but not by the way CircuitPython initializes an endpoint on these devices. The change itself is very minimal -- it just increments the endpoint after setting up either an IN our OUT instead of both to an endpoint pair. I made it for all device classes and it should cover all cases for other devices as well.

Many thanks to @tannewt and @dhalbert for the help :)

@tannewt tannewt self-requested a review September 27, 2024 16:38
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks for the new port! I'm excited to try it. I've added a few comments. Nothing major.

ports/analog/Makefile Outdated Show resolved Hide resolved
ports/analog/boards/apard32690/README.md Outdated Show resolved Hide resolved
ports/analog/boards/apard32690/board.c Outdated Show resolved Hide resolved
ports/analog/boards/apard32690/pins.c Show resolved Hide resolved
ports/analog/boards/max32690evkit/board.c Outdated Show resolved Hide resolved
ports/analog/boards/max32690evkit/board.c Outdated Show resolved Hide resolved
ports/analog/common-hal/digitalio/DigitalInOut.c Outdated Show resolved Hide resolved
ports/analog/mpconfigport.h Show resolved Hide resolved
ports/analog/supervisor/port.c Show resolved Hide resolved
ports/analog/supervisor/port.c Outdated Show resolved Hide resolved
@tannewt
Copy link
Member

tannewt commented Nov 25, 2024

@Brandon-Hurst Do you want a review on this? I'm back from paternity leave for the time being.

@Brandon-Hurst
Copy link
Author

Brandon-Hurst commented Nov 25, 2024

It should be ready as far as all the requested changes and I've tested it pretty thoroughly. I had to rebase to fix the last erring CI check on the readthedocs task, and now it shows 300+ commits instead of like 30 so that may need to be fixed first 😅 sorry about that.

U-ANALOG\BHurst and others added 24 commits November 25, 2024 15:28
- Added simple status LED for initial testing
- Generally cleaned up & implemented supervisor/port.c
- Added some additional commenting from supervisor headers
- Fixed a linkerscript issue copying .text into FLASH_ISR
- (build): added hex & bin targets for executable.
- (build): temporarily modified .ld due to issue exiting MAX32 ROM code
- (flash): added flash target for jlink & msdk, along with helper files under tools/
- Reorganized mpconfigport.mk for clarity
- Moved flash driver & LED/PB definitions to board files (mpconfigboard._)
…Placed in supervisor/port.c

- Moved LEDs & PBs from supervisor/port.c to boards/$(BOARD)/board.c for APARD
- Reviewed Processor.c in common-hal/microcontroller
- Prepared stubs for common-hal/microcontroller/Pin.c
…. - Enabled USB-based modules & dependencies in mpconfigport.mk
…umerates but has some issues starting interfaces for CDC/MSC/HID.
…be usable. - Had to adjust some shared modules to account for MAX32 devices not supporting IN/OUT endpoints on the same EP #
… Fixed bugs with Internal Flash filesystem. Files now write & read back correctly. - Added copyright headers for all files.
…h interrupt_after_ticks. - Move LED inidcator to STATUS_LED code.
… GPIO Port 4 (different on MAX32690). - Added MCR defs to max32_port.h.
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Two small things. I'm excited to get this merged in!

Comment on lines +75 to +95
// Enable GPIO (enables clocks + common init for ports)
for (int i = 0; i < MXC_CFG_GPIO_INSTANCES; i++) {
err = MXC_GPIO_Init(0x1 << i);
if (err) {
return SAFE_MODE_PROGRAMMATIC;
}
}

// Init Board LEDs
/* setup GPIO for the LED */
for (int i = 0; i < num_leds; i++) {
// Set the output value
MXC_GPIO_OutClr(led_pin[i].port, led_pin[i].mask);

if (MXC_GPIO_Config(&led_pin[i]) != E_NO_ERROR) {
return SAFE_MODE_PROGRAMMATIC;
}
}

// Turn on one LED to indicate Sign of Life
MXC_GPIO_OutSet(led_pin[2].port, led_pin[2].mask);
Copy link
Member

Choose a reason for hiding this comment

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

Did you get the status LED working? It should replace this code.

Comment on lines +37 to +45
volatile uint32_t system_ticks = 0;

void SysTick_Handler(void) {
system_ticks++;
}

uint32_t board_millis(void) {
return system_ticks;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why is this board specific?

Copy link
Member

Choose a reason for hiding this comment

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

It seems unusual to have to provide board_millis. Unless I'm mistaken, this identifier is not otherwise used in CircuitPython. Rather, it seems it may be defined & used in tinyusb:

// lib/tinyusb/hw/bsp/max32690/family.c
#if CFG_TUSB_OS == OPT_OS_NONE
volatile uint32_t system_ticks = 0;

void SysTick_Handler(void) {
  system_ticks++;
}

uint32_t board_millis(void) {
  return system_ticks; 
}
#endif

Perhaps there's a mistake with the #defines for tinyusb?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants