-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add busio support for ADI MAX32xxx port #10413
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
base: main
Are you sure you want to change the base?
Add busio support for ADI MAX32xxx port #10413
Conversation
- Enabled BUSIO in mpconfigport.mk - Stubs added for busio.I2C & busio.SPI - Implemented busio.UART to basic working functionality - Added MCU-specific UART data in peripherals/<mcu>/max32_uart.c & max32_uart.h - Still have a couple issues where UART generates garbage characters or does not respond to timeouts / generate asynchronous IRQs
- Add debug-dap.sh to spawn OpenOCD process & gdb-multiarch process for debugging. Requires MaximSDK install & path settings inside this script. Also depends on gdb-multiarch and connected CMSIS-DAP probe over USB. - debug-dap.gdb simply holds the gdb commands for this script + small style fix on common-hal/microcontroller/Pin.c
- Graceful recovery from timeouts added - Remaining issue still exists where a timeout occurs after a while
- No ISR for UART writes, but uses timeouts - Reads use ISRs, but also incorporate timeouts
- Formatting & codespell fixes from pre-commit
- Call NVIC_SetRAM in supervisor/port.c to move NVIC table to RAM, enabling ISR RAM remapping. - Rename UART ISR to be generic for any UART IRQn. - Remove background led writes from background.c.
- Data structure and constructor for I2C (tested) - Lock and probing functions complete and tested TODO: Implement I2C Write, Read, Wr/Rd Signed-off-by: Brandon-Hurst <[email protected]>
Currently coupled a bit to APARD32690-SL board. Will refactor after testing BUSIO.SPI Signed-off-by: Brandon-Hurst <[email protected]>
TODO: - Figure out assigning CS Pins for SPI - Test & Debug SPI / I2C Signed-off-by: Brandon-Hurst <[email protected]>
Signed-off-by: Brandon-Hurst <[email protected]>
…ipherals. Signed-off-by: Brandon-Hurst <[email protected]>
Also reformatted pinmaps for SPI/I2C/UART on MAX32690 Signed-off-by: Brandon Hurst <[email protected]>
- Fixed errors with peripheral bus hardware descriptions in MAX32690 "peripherals/" - Resolved all FIXME comments - Removed unused ringbuf API from common-hal/busio/UART.c Signed-off-by: Brandon Hurst <[email protected]>
Signed-off-by: Brandon Hurst <[email protected]>
- Add correct ringbuffer handling code to ports/analog/common-hal/UART.c Signed-off-by: Brandon Hurst <[email protected]>
Signed-off-by: Brandon Hurst <[email protected]>
- Used direct hardware values to rely on common HW implementation - Accounted for TX Lockout conditions in Transmit FIFO causing errors Signed-off-by: Brandon Hurst <[email protected]>
Signed-off-by: Brandon Hurst <[email protected]>
- Probe function caused infinite loops / unreliable ACK values - Replaced with probe-function with low-level routines Signed-off-by: Brandon Hurst <[email protected]>
Signed-off-by: Brandon Hurst <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for continuing to flesh this out. The changes here are stylistic, to match how we do error messages, etc. elsewhere.
if ((scl != NULL) && (sda != NULL)) { | ||
MXC_GPIO_Config(&i2c_maps[self->i2c_id]); | ||
err = MXC_I2C_Init(self->i2c_regs, 1, 0x00); | ||
if (err) { | ||
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("Failed to init I2C. ERR: %d\n"), err); | ||
} | ||
err = MXC_I2C_SetFrequency(self->i2c_regs, frequency); | ||
if (err < 0) { | ||
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("Failed to set I2C frequency. ERR: %d\n"), err); | ||
} | ||
} else if (scl != NULL) { | ||
mp_raise_NotImplementedError(MP_ERROR_TEXT("I2C needs SDA & SCL")); | ||
} else if (sda != NULL) { | ||
mp_raise_NotImplementedError(MP_ERROR_TEXT("I2C needs SDA & SCL")); | ||
} else { | ||
// Should not get here, as shared-bindings API should not call this way | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validate_obj_is_free_pin()
calls in shared-bindings/busio/I2C.c
will ensure that scl
and sda
are both pins and cannot be null, so you can remove this validation.
if (((timeout < 0.0) || (timeout > 100.0))) { | ||
self->timeout = 1.0; | ||
} else { | ||
self->timeout = timeout; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout that is passed in is a uint32_t
, so don't compare against floats. Also self->timeout
is a uint32_t
, so there shouldn't be any floats here at all.
}; | ||
ret = MXC_I2C_MasterTransaction(&rd_req); | ||
if (ret) { | ||
mp_raise_RuntimeError(MP_ERROR_TEXT("ERROR during I2C Transaction\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp_raise_RuntimeError(MP_ERROR_TEXT("ERROR during I2C Transaction\n")); | |
return MP_EIO; |
}; | ||
ret = MXC_I2C_MasterTransaction(&wr_rd_req); | ||
if (ret) { | ||
mp_raise_RuntimeError(MP_ERROR_TEXT("ERROR during I2C Transaction\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp_raise_RuntimeError(MP_ERROR_TEXT("ERROR during I2C Transaction\n")); | |
return MP_EIO; |
1, 0x01, 1000000, spi_pins); | ||
MXC_GPIO_SetVSSEL(MXC_GPIO_GET_GPIO(sck->port), MXC_GPIO_VSSEL_VDDIOH, (sck->mask | miso->mask | mosi->mask | MXC_GPIO_PIN_0)); | ||
if (err) { | ||
mp_raise_RuntimeError(MP_ERROR_TEXT("Failed to init SPI.\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't add \n
at the end -- that's already taken care of. Also don't add periods: that saves space.
In general, for error messages, see if you can reuse a message from locales/circuitpython.pot
. That will save space. Note that quite a few messages are parameterized, like Invalid %q
, where the %q
is an MP_QSTR_something
which often is used elsewhere as well.
Here I am reusing an existing message:
mp_raise_RuntimeError(MP_ERROR_TEXT("Failed to init SPI.\n")); | |
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("%q init failed", MP_QSTR_SPI)); |
} | ||
// Check for errors from the callback | ||
else if (uart_err != E_NO_ERROR) { | ||
mp_printf(&mp_sys_stdout_print, "MAX32 ERR: %d\n", uart_err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debugging printout
if (isValidBaudrate(baudrate)) { | ||
self->baudrate = baudrate; | ||
} else { | ||
mp_raise_ValueError(MP_ERROR_TEXT("Baudrate invalid. Must be a standard UART baudrate.\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp_raise_ValueError(MP_ERROR_TEXT("Baudrate invalid. Must be a standard UART baudrate.\n")); | |
mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q", MP_QSTR_baudrate)); |
return i; | ||
} | ||
} | ||
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("ERR: Unable to find an SPI matching pins... \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("ERR: Unable to find an SPI matching pins... \ | |
mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q", MP_QSTR_pins); |
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("ERR: Unable to find an I2C matching pins...\nSCL: port %d mask %d\nSDA: port %d mask %d\n"), | ||
sda->port, sda->mask, scl->port, scl->mask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("ERR: Unable to find an I2C matching pins...\nSCL: port %d mask %d\nSDA: port %d mask %d\n"), | |
sda->port, sda->mask, scl->port, scl->mask); | |
mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q", MP_QSTR_pins), |
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("ERR: Unable to find a uart matching pins...\nTX: port %d mask %d\nRX: port %d mask %d\n"), | ||
tx->port, tx->mask, rx->port, rx->mask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp_raise_RuntimeError_varg(MP_ERROR_TEXT("ERR: Unable to find a uart matching pins...\nTX: port %d mask %d\nRX: port %d mask %d\n"), | |
tx->port, tx->mask, rx->port, rx->mask); | |
mp_raise_ValueError_varg(MP_ERROR_TEXT("Invalid %q", MP_QSTR_pins), |
Add BUSIO Support for supported MAX32xxx devices. Not all capable MAX32 devices are supported yet; more support for MAX32650/MAX32666/MAX78002 boards may be added in a future PR. This module was tested on a MAX32690 board, the AD-APARD32690-SL. I used a ADT7140 I2C and MAX31723 SPI temperature sensor, and also tested the UART via the debug port serial connection.
MAX31723 (SPI):

ADT7420 (I2C):

A simple echo test was done with UART as well with no issues: