modm-io / modm

modm: a C++23 library generator for AVR and ARM Cortex-M devices
https://modm.io
Mozilla Public License 2.0
751 stars 133 forks source link

Support for SAMG55 devices #675

Closed mcbridejc closed 3 years ago

mcbridejc commented 3 years ago

I'm working on support for the SAMG famiy -- specifically SAMG55, but I think there's a lot of similarity with other SAMG5x parts.

I'm creating this issue as a place to track it, and a place for discussion.

Currently, I have updated cmsis-header-sam (see PR) to pull in the headers, and I've fixed a couple of issues with device generation in modm-devices (no PR yet -- I wanted to verify more) so that I can generate what seem like correct XML files for the devices.

Now I'm working on creating a demo app, and the first step was to get the GPIO working. Unfortunately, the GPIO registers are different from the currently supported SAM devices, so I'm adapting. These drivers have some pretty complicated C++ magic, but I am pushing my way through the variadic templates etc and struggling to keep up :).

The question on my mind now is: does anyone know if the current sam GPIO driver works for port B at all? It's entirely possible I've misunderstood, but it looks like getPortReg method just doesn't support any port other than the first ("A"). Also, there seems to be a typo in readPortReg that would use the wrong mask, even if it read the correct port. Maybe @henrikssn can comment?

Also, the SAMG55 does not support output speeds, and from what I can see in the existing driver they aren't supported for SAMDxx either; the output speed is just ignored. I assume this was to match the API of the STM32 driver?

henrikssn commented 3 years ago

I don't really remember, it indeed looks like an oversight and getPortReg should probably have a special case for port B.

salkinium commented 3 years ago

The SAMD port is not extensively tested, due to a lack of boards.

I assume this was to match the API of the STM32 driver?

Yes, it can be removed, only the architecture interface is relevant and it's (ideally) platform agnostic.

mcbridejc commented 3 years ago

Cool! I've opened draft PRs (modm-devices #69, modm #676 ). I've got the GPIO driver, and some basic clock config working. I'll also fix the getPortReg for port B. I can't really test except to look at disassembly for a simple case and make sure it writes the register I expect, but I think its pretty straightforward. I will drop the speed setting arguments.

I'll add a samg55_xplained_pro board definition, and an example app.

TinyUSB supports this part, so I intend to add USB support as well as an ADC + SPI + I2C driver. I can just leave the PR open while I add everything or do it in phases, whatever you prefer @salkinium.

salkinium commented 3 years ago

I prefer smaller scoped PRs, just to reduce the amount of contributations in the air that may become stale. So the support and BSP is one PR, but the other drivers would be separate.

mcbridejc commented 3 years ago

Going to close this. Basic support is there now.