-
Notifications
You must be signed in to change notification settings - Fork 2k
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
boards: slwstk6000b: add support #8824
Conversation
Asking @kaibeckmann to test this PR, since he has access to the boards. |
Hey I found a couple of things.
I like the submodule system, but I think the new definition for the pins is not necessary. First the names are derived from the slwrb4162a, but the datesheets of the other modules have different names for the pins with the same GPIO. That is quite confusing. And than it's not necessary, because the GPIOs seem to be the same (at least for the 3 or 4 datasheets I checked). The main difference of the modules are the cpu and additional periphery, like SPI-Flash, Touch-Slider, radio transceiver and if there are separate GPIOs to enable sensors, dispay etc. |
Thanks @kaibeckmann for testing. The mapping of modules to CPU is based on the list on the page of the SLWSTK6000B itself. However, looking at the reference manuals, I do see different MCU's being used. I'll ask Silicon Labs to update their list, somehow. Regarding the pin numbers: it is needed. If you look at the reference manuals, you can see that some MCU pins map to different module pins. For instance, the The pin numbers are confusing at first, because they don't link to the reference manuals, where you can see the actual module pins. Here are some: |
Hmm I see your point, but the module pin names are confusing. It makes the understanding and using of the boards for someone new more complex. Maybe a more elaborate name schema would help? Something like MODULE_PIN_P0 ? If the MCU pin for the basic peripherals really can change, than the multiplexing part should properly be moved to the module header as well (UART, I2C, SPI, ...). |
I can change it to I haven't encountered a reason for moving the multiplexing part as well, but if it is necessary, I'll do it. I want to keep as much as possible in |
ad5bfa8
to
5e0e83d
Compare
I have fixed the board name to module mapping, based on the reference manuals and renamed the pin names. |
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.
I think I should have pressed the "submit review" button ...
Some smaller issues / bugs.
boards/slwstk6000b/Makefile.include
Outdated
|
||
# setup JLink for flashing | ||
export JLINK_DEVICE := $(CPU_MODEL) | ||
include $(RIOTMAKE)/tools/jlink.inc.mk |
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 does not work for me, as long as the DBG variable is not set to the gdb program.
I had added following lines between line 18 and 19 in my version:
include $(RIOTMAKE)/tools/gdb.inc.mk
export DBG:=$(GDB)
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.
I think this is a bug. I compared JLink with OpenOCD, and OpenOCD sets DBG=GDB, but JLink doesn't. Will look into this tonight.
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.
#8853 should address this.
}; | ||
|
||
#define I2C_NUMOF PERIPH_NUMOF(i2c_config) | ||
#define I2C_0_ISR isr_i2c1 |
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.
that is the wrong isr handler, must be isr_i2c0
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.
Fixed.
{ | ||
.dev = USART0, | ||
.rx_pin = F6, | ||
.tx_pin = F7, |
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.
RX and TX are swaped.
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.
Fixed.
boards/slwstk6000b/Makefile.include
Outdated
include $(RIOTMAKE)/tools/serial.inc.mk | ||
|
||
# setup JLink for flashing | ||
export JLINK_DEVICE := $(CPU_MODEL) |
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 $(CPU_MODEL) does not work for the EFR32MG1P family. For these the string EFR32MG1PxxxF256 is needed.
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.
I've added additional data in modules.txt
.
5e0e83d
to
ae3b910
Compare
ae3b910
to
1aa560e
Compare
@kaibeckmann do the latest adaptions work for you? The debug problem should now be addressed. |
OK I tested it for the slwrb4150a and slwrb4162a with the default app and the tests-core. It works fine. |
1aa560e
to
e53967d
Compare
Updated vendor headers per #8873. |
OK I ran the tests again, everything is working as expected. |
#define MODULE_PIN_P43 GPIO_PIN(PF, 10) | ||
#define MODULE_PIN_P44 GPIO_PIN(PF, 11) | ||
#define MODULE_PIN_P45 GPIO_PIN(PF, 12) | ||
/** @} */ |
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.
How different are these definitions to the ones for the other module?
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.
So far as I can tell, different enough. There are modules that only define half of these 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.
I was just wondering if we could merge them in a single file (with #ifdef
s), since I see that at least the beginning of the definitions are the same.
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.
I propose to start this way, and change if it turns out to be easier to combine in the future.
I expect that there might be additional features or possibilities, looking at the list of supported modules (bottom three for example).
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.
Ok, please squash.
@@ -0,0 +1,135 @@ | |||
/* | |||
* Copyright (C) 2015-2018 Freie Universität Berlin | |||
* |
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.
Freie Universität?
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.
Yes, file was copy-pasted from EZR32WG, by @haukepetersen.
e53967d
to
2a8712a
Compare
Squashed and rebased. Murdock is happy. |
@kYc0o @kaibeckmann Good to go? |
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 for me, ACK.
Thanks! |
Contribution description
This PR add supports for the Silicon Labs SLWSTK6000B development kit for wireless applications. Although the files are very similar to the other PRs I have submitted, this board is actually a base board that can host different CPU modules. All CPU modules have fixed pins for certain functions, including ones for the onboard LCD and Si7021 temperature sensor.
I came up with an idea similar to how I supported the EFM32 family, by using
modules.txt
as a 'database' for mapping the base board to a CPU module. I don't think duplicating this board for all the different modules would work out.To compile, you actually have to override the
BOARD_MODULE
(defaults toSLWBR4162a
) using something like this:BOARD=slwstk6000b BOARD_MODULE=slwrb4162a make -j8
I currently have two CPU modules supported:
SLWBR4150A
andSLWBR4162A
. However, there are 15+ more modules that fit this base board.Issues/PRs references
#8585
#8520
#8630
#8569