-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
samd/boards: Add definitions for Generic boards, Adafruit NeoKey Trinkey and Adafruit QT PY SAMD21. #16629
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
Conversation
Code size report:
|
d371cdf
to
9b01966
Compare
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.
Regarding the generic boards:
- I think they should be named
SAMD_GENERIC_<mcu>
so that they match esp8266/esp32 generic board names. - Please add
"vendor": "Microchip"
to the board.json
It would be good to mention somewhere that this generic firmware applies to all these MCU variants. Maybe for now just add it as a comment to the top of the |
Actually, you can put some text into |
I have changed the files following your comments and added board.md files to the generic board definitions. Sorry for all the typos, copied along the boards. |
9b01966
to
2f83757
Compare
No. The way you have it at the moment, with 3 generic boards, is good. But, for the naming, how about:
? There's no repeating "SAMD" in the name, and the "X" indicates it suits multiple boards of that form. |
The chip name is SAMD21X18. How about |
I was trying to match the esp32 generic board names, like How about |
OK. Repeating the D makes it better readable. |
73c22cc
to
2598f4a
Compare
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.
Tested the SAMD_GENERIC_D21X18
firmware on an Adafruit ItsyBitsy M0 Express. It works.
The definition uses the internal oscillator for clock and only internal flash for the file system. It works at SAMD21G18 and SAMD21E18 devices as well, only that fewer pins are accessible. Tested with a SAMD21E18, SAM21G18 and SAMD21J18 board. Signed-off-by: robert-hh <robert@hammelrath.com>
The definition uses the internal oscillator for clock and only internal flash for the file system. It works at SAMD51G19 and SAMD51J19 devices as well, only that fewer pins are accessible. Tested with a SAMD51G19 and SAMD51J9 board. Signed-off-by: robert-hh <robert@hammelrath.com>
The definition uses the internal oscillator for clock and only internal flash for the file system. It works at SAMD51J20 device as well, only that fewer pins are accessible. Tested with a SAMD51J20 board. Signed-off-by: robert-hh <robert@hammelrath.com>
Tested with a Adafruit SAMD QT board, which may optionally be equipped with SPIFLASH memory. Signed-off-by: robert-hh <robert@hammelrath.com>
Supporting a variant with an optional SPIFLASH device as well. Tested both variants with a QT Py board. Signed-off-by: robert-hh <robert@hammelrath.com>
Tested with that board. Signed-off-by: robert-hh <robert@hammelrath.com>
Only pins accessible at the board are shown. Signed-off-by: robert-hh <robert@hammelrath.com>
The table shows the devices available at the pin and the respective package letter. Signed-off-by: robert-hh <robert@hammelrath.com>
- UART 0 at pins PA07/PA06, labelled RX/TX | ||
- I2C 1 at pins PA16/PA17, labelled SDA/SCL | ||
- SPI 0 at pins PA09/PA10/PA11, labelled MISO, MOSI and SCK | ||
- DAC output on pin PA02, labelled A0 |
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.
This is not a blocker for this PR, but what exactly are "default devices"? It seems that the samd port requires the user to always specify I2C pins (for example).
Maybe it would be good to follow rp2 and stm32 and allow default pins, so the user can then just do machine.I2C(1)
and get the default pin settings. Same with UART and SPI.
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.
Default devices are the ones with according labels on the silkscreen of the board, as indicated by "labelled". Theses could be assigned in a mpconfigboard.h setting, but still the device number would differ between board. Or the numbers have to be re-mapped. That is done in the MIMXRT port, and I do not like it. The mpconfigboard.h files of the MIMXRT port are way to long and complicated. Out of that experience, may aim was to have the SAMD board config files, as small as possible.
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.
It's possible to use just pins.csv
to configure default pins for peripherals. Eg ADAFRUIT_QTPY_SAMD21
has pin aliases SCL
and SDA
defined. So in machine_i2c.h
you could have something like:
#if defined(pin_SCL) && defined(pin_SDA)
// board defines default pins for I2C0, so use them as default
#endif
Or if you need to distinguish between I2C0 and I2C1 as the default you could add to pins.csv
:
-I2C1_SCL,PA16
-I2C1_SDA,PA17
Then use those symbols in machine_i2c.c
.
The point is, it should be possible to (a) use just pins.csv
and not touch mpconfigboard.h
; (b) standardise a naming scheme for default pins and use it on all boards, to make things consistent.
Note that the pin names don't have to be exposed to the user (as above, just prepend with -), it's just an internal mechanism for specifying pin configuration.
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.
Good hint, thank you. I'll look into it.
An mpbuild user just reported a bug due to the GENERIC boards not having a URL in the I can easily patch mpbuild to work around it, but should we add a url for these boards? They're the only definitions that don't have one. MicroChip doesn't provide great URLs but even pointing the the sam-d home page might be adequate. My longer-term fix is to use json-schema to enforce fields in every |
The sam-d home page is a good choice. |
@dpgeorge I have a branch now that uses default pins for UART, I2C and SPI. I have to do some testing to check, that the assignments are right and that reasonable error messages appear when needed. Still, the right ID must be supplied with the constructor, and it differs among boards. |
Sounds good!
That's OK, much easier to document a single ID per board, rather than ID and pins. Also much easier for the user to just specify the ID. For further improvements there, so you don't need to specify anything, see #16671. |
Thanks, I had seen that. That could be added; requires adding a #define e.g. to mpconfigboard.h. |
There is a PR #16735 now for supporting default UART, I2C and SPI devices. Using it e.g. as |
Summary
Add definitions for Generic boards, Adafruit NeoKey Trinkey and Adafruit QT PY SAMD21.
The Generic boards/MCUs chosen are:
The Generic board firmware uses the internal RC-oscillator for clock and the internal flash for the file system.
The firmware for the QT PY board includes a variant supporting the optional SPIFLASH. For that
purpose, the support for variants was added.
Testing
All firmware images were built and loaded to the respective boards. REPL started and scripts could be executed. The respective Generic firmware was tested with the lower pin count MCU versions as well.