Add support for Silabs EFR32MG12P device and Thunderboard Sense 2 #9042

diegosueiro · 2018-07-20T07:15:48Z

This PR adds the support for Silabs EFR32MG-SLTB004A (Thunderboard Sense 2) board in Zephyr.

It adds the Gecko SDK for EFR32MG12P devices into the ext/hal for Silabs, introduces leuart and i2c drivers for Gecko Devices and a sample application to read Relative Humidity and Temperature from Silabs
SI7021 chip via I2C interface.

This PR depends on #8930 (first 3 commits) and as soon as it gets merged I will update this one to remove the redundant commits.

diegosueiro · 2018-07-20T07:17:28Z

Ping @chrta .

chrta · 2018-07-20T07:24:49Z

Are the emlib and the other gecko sdk files are still on 5.1.2, but the efm32mg files are from gecko sdk 5.5.0? Does this work? Or should we first update the gecko sdk to 5.5.0 for all already merged silabs boards?

codecov-io · 2018-07-20T07:48:17Z

Codecov Report

Merging #9042 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #9042   +/-   ##
======================================
  Coverage    53.3%   53.3%           
======================================
  Files         215     215           
  Lines       25908   25908           
  Branches     5711    5711           
======================================
  Hits        13810   13810           
  Misses       9777    9777           
  Partials     2321    2321

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a19ae13...bddc8d7. Read the comment docs.

diegosueiro · 2018-07-20T08:00:03Z

@chrta ,

It is working without any issues. If you compare the CMSIS headers between these versions you will see that it is more about adding defines and declarations.

The idea is to update the Gecko SDK in the near future.

dbkinder · 2018-07-20T19:58:20Z

boards/arm/efr32mg_sltb004a/doc/efr32mg_sltb004a.rst

+.. _efr32mg_sltb004a:
+
+EFR32MG-SLTB004A
+###############


The title underline must be at least as long as the title above it, so add one more #

dbkinder · 2018-07-20T19:59:22Z

boards/arm/efr32mg_sltb004a/doc/efr32mg_sltb004a.rst

+- One bi-color LED and two push buttons
+- Power enable signals for fine grained power-control
+- On-board SEGGER J-Link debugger for easy programming and debugging, which
+includes a USB virtual COM port


this line should be indented to the O on the previous line

dbkinder · 2018-07-20T19:59:47Z

boards/arm/efr32mg_sltb004a/doc/efr32mg_sltb004a.rst

+- On-board SEGGER J-Link debugger for easy programming and debugging, which
+includes a USB virtual COM port
+- Mini Simplicity connector for access to energy profiling and advanced wireless
+network debugging


same here, indent the line to the M

dbkinder · 2018-07-20T20:00:57Z

boards/arm/efr32mg_sltb004a/doc/efr32mg_sltb004a.rst

+
+The default configuration can be found in the defconfig file:
+
+	``boards/arm/efr32mg_sltb004a/efr32mg_sltb004a_defconfig``


delete the blank line and indent:

The default configuration can be found in the defconfig file: ``boards/arm/efr32mg_sltb004a/efr32mg_sltb004a_defconfig``.

dbkinder

+1 for docs, thanks.

diegosueiro · 2018-08-03T11:17:50Z

@galak and @MaureenHelm ,

Any feedbacks?

galak

Number of dts related comments.

galak · 2018-09-12T05:15:42Z

arch/arm/soc/silabs_exx32/efr32mg12p/Kconfig.series

+config SOC_SERIES_EFR32MG12P
+	bool "EFR32MG12P Series MCU"
+	select CPU_CORTEX_M4
+	select CPU_HAS_FPU


duplicated in config SOC_EFR32MG12P

galak · 2018-09-12T05:16:35Z

drivers/serial/Kconfig.gecko

@@ -6,7 +6,7 @@
 #

 menuconfig UART_GECKO
-	bool "Gecko uart driver"
+	bool "Gecko UART/USART driver"


Can you merge this with the rework of the driver patch.

galak · 2018-09-12T05:17:24Z

arch/arm/soc/silabs_exx32/efm32wg/dts.fixup


-#define CONFIG_UART_GECKO_0_BAUD_RATE	SILABS_EFM32_USART_4000C000_CURRENT_SPEED
-#define CONFIG_UART_GECKO_0_IRQ_PRI	SILABS_EFM32_USART_4000C000_IRQ_0_PRIORITY
+#define USART_0_RX_IRQ			SILABS_GECKO_USART_4000C000_IRQ_0


We've used CONFIG_ to prefix such defines. Want to keep this consistent with other DTS support for now.

I'm trying to keep the consistency with the define names automatically generated by the alias. I can check if I can do something in the devicetree yaml to have these defines automatically generated without needing a fixup.
Doesn't look nice to me to have in the driver a mixture between defines with and without CONFIG_ prefix.
I even sent an email to the mailing list about cleaning up these fixups and make usage of the aliases. Have you seen it?

@galak,

I got rid of these fixups by adding the "interrupt-names" for the uarts nodes.

galak · 2018-09-12T05:18:48Z

arch/arm/soc/silabs_exx32/efr32fg1p/soc_pinmap.h

@@ -18,13 +18,13 @@

 #ifdef CONFIG_SOC_PART_NUMBER_EFR32FG1P133F256GM48
 #ifdef CONFIG_USART_GECKO_0
-#if (CONFIG_USART_GECKO_0_GPIO_LOC == 0)
+#if (USART_0_LOC == 0)
 #define PIN_USART0_TXD {gpioPortA, 0, gpioModePushPull, 1}


Why can't these just be:

`
#define PIN_USART0_TXD {gpioPortA, 0 + CONFIG_USART_0_LOC, gpioModePushPull, 1}
#define PIN_USART0_RXD {gpioPortA, 1 + CONFIG_USART_0_LOC, gpioModeInput, 1}

@galak,

The "loc" property is not directly related to the pin number.

Yeah, ignore my comments on loc. However we should rename to location.

galak · 2018-09-12T05:18:59Z

arch/arm/soc/silabs_exx32/efr32fg1p/soc_pinmap.h

 #error ("Serial Driver for Gecko MCUs not implemented for this location index")
-#elif (CONFIG_USART_GECKO_1_GPIO_LOC == 1)
+#elif (USART_1_LOC == 1)
 #define PIN_USART1_TXD {gpioPortF, 10, gpioModePushPull, 1}


Similar comment as above.

galak · 2018-09-12T05:21:09Z

boards/arm/efm32wg_stk3800/efm32wg_stk3800.dts

@@ -20,5 +24,6 @@

 &uart0 {
 	current-speed = <115200>;
+	loc = <1>;


We should have this be tx-pin and rx-pin.

Do you mean we need to have a loc (location) property for the tx-pin and rx-pin separately?

I'm thinking we should, it would be consistent with what the NRF SoC is doing in dts for the pins for rx & tx.

Looking at the manual, and maybe loc makes sense. I was tracing the code and came to soc_gpio_configure/GPIO_PinModeSet. So I'm confused about what location means for which pins are used.

The loc is used to instruct which pin (ball) is going to be used. As a consequence, we need to pass which port will be used and the alternate configuration (location). Take a look at the section "6.7 Alternate Functionality Overview" in this document:
https://www.silabs.com/documents/public/data-sheets/efr32fg1-datasheet.pdf

@galak,

What do you say?

Take a look at the function uart_gecko_init_pins in drivers/serial/uart_gecko.c file to understand how the "loc" property is used. If you want we can slipt the loc for rx and tx because they can be eventually different.

For the EFM32WG (and all the older EFM32 i think), there is only one loc per U(S)ART, see e.g. https://www.silabs.com/documents/public/reference-manuals/EFM32WG-RM.pdf chapter 17.5.22 USARTn_ROUTE - I/O Routing Register.

This should be considered (one loc per peripheral vs. one loc per pin in a peripheral), if the driver implementation is changed.

dts/bindings/serial/silabs,gecko-uart.yaml

galak · 2018-09-12T05:22:40Z

dts/bindings/serial/silabs,gecko-usart.yaml

@@ -27,4 +27,10 @@ properties:
      category: required
      description: required interrupts
      generation: define
+
+    loc:


should be rx-pin & tx-pin.

dts/bindings/serial/silabs,gecko-leuart.yaml

diegosueiro · 2018-09-16T06:09:55Z

@galak ,

Comments addressed.
Note that this PR has a dependency on PR #8930 and as soon as it gets merged I'll rebase this one.

diegosueiro · 2018-09-16T07:16:40Z

@galak,

I'm not sure how to solve the merging conflicts in the Shippable testing environment.

chrta · 2018-09-16T09:12:40Z

@galak,

I'm not sure how to solve the merging conflicts in the Shippable testing environment.

You have to rebase your branch on the current master. Because the soc files moved to another directory, you have to change a few things (board dts file, create soc dtsi file, change path to drivers is board cmake file etc).

diegosueiro · 2018-09-16T10:11:20Z

@chrta,

I already rebased and I'm using the new SoC path.
The test failures are not related to this.

chrta · 2018-09-16T10:18:04Z

@diegosueiro Your SoC commit 10045194958167858bf93db5c74b8405501d32fe is still based on an older master branch, e.g. there is no file dts/arm/silabs/mem.h any more.

diegosueiro · 2018-09-16T10:38:37Z

@chrta,

You are right. I missed the 912b4a2 commit.
I'll rebase and push again.

galak · 2018-10-19T17:14:18Z

@pfalcon can you review the uart driver.

galak · 2018-10-19T17:22:30Z

I opened up PR #10728 to merge the base SoC support. The board code needs a little updating (for moving gpio info into device tree). Also would like to get a little more review on the uart/i2c drivers.

galak

Seems like the I2C driver doesn't use interrupts, so we should remove the code associated w/setting up interrupts and the isr handler.

galak · 2018-10-19T16:50:09Z

drivers/serial/Kconfig.gecko

@@ -52,4 +52,4 @@ config USART_GECKO_3
 	  Enable support for Gecko USART3 port in the driver. Say y here if you
 	  want to use USART3 device.

-endif # UART_GECKO
+endif # UART_GECKO


Is this commit doing anything now? (drivers: Add more uart instances for Silabs Devices)

This is showing as outdated. I think this is gone after I rebased it the master instead of the PR that was changing this file.

galak · 2018-10-19T17:26:40Z

drivers/i2c/i2c_gecko.c

+}
+
+
+static void i2c_gecko_isr(void *arg)


Why have isr code at all if it doesn't do anything?

I'll remove that after getting the others reviews.

Ok. I found the problem. I forgot to drop 2 commits when rebasing this PR against the master.
I'll rebase it dropping these two commits and remove the i2c interrupt function.

Ok, I've merged the base SoC support, and add a few minor comments on the board code.

galak · 2018-10-19T18:49:08Z

boards/arm/efr32mg_sltb004a/board.h

+
+
+/* Push button PB0 (SW0) */
+#define PB0_GPIO_NAME	CONFIG_GPIO_GECKO_PORTD_NAME


Move the button and LED into device tree out of board.h

pfalcon · 2018-10-22T10:55:01Z

drivers/serial/Kconfig.leuart_gecko

+
+if LEUART_GECKO
+
+config LEUART_GECKO_0


Sorry, but is that we still don't have means to deal with that in device tree?

I'm following the same pattern for drivers/serial/Kconfig.gecko

pfalcon · 2018-10-22T11:02:41Z

drivers/serial/leuart_gecko.c

+{
+	LEUART_TypeDef *base = DEV_BASE(dev);
+
+	LEUART_Tx(base, c);


I would suggest to add comment here that the blocking until a char can be sent happens in LEUART_Tx(). (Because majority of our drivers have explicit busy-wait loop in poll_out; so, the above looks confusing.)

Added a comment.

pfalcon · 2018-10-22T12:03:03Z

drivers/serial/leuart_gecko.c

+	LEUART_TypeDef *base = DEV_BASE(dev);
+	u32_t flags = LEUART_IntGet(base);
+
+	LEUART_IntClear(base, LEUART_IF_TXC);


Why do you clear interrupt here? This function is supposed to be side effect free.

pfalcon

(Github glitches today, let's see if my comments were recorded.)

Some questions/suggestions.

pfalcon

Just a note.

pfalcon · 2018-10-23T08:42:32Z

drivers/serial/leuart_gecko.c

+static void leuart_gecko_irq_tx_enable(struct device *dev)
+{
+	LEUART_TypeDef *base = DEV_BASE(dev);
+	u32_t mask = LEUART_IEN_TXBL | LEUART_IEN_TXC;


My understanding that this enables both "fifo has empty space" and "transmission complete" interrupts. Just a note that many other drivers don't enable "transmission complete" IRQ, because it's not really much useful.

Again, this is just a note of difference to other drivers. (If someone later would need to investigate any issues and path which led to them.
)

I've just followed the same pattern for drivers/serial/uart_gecko.c driver.

pfalcon · 2018-10-23T08:50:07Z

@pfalcon can you review the uart driver.

Well, I tried. If there's something to guaranteedly change, is to add more comments ;-).

Otherwise, it's prompted me to prepare #10761 , because docs there still leave much to be desired (i.e. were partially updated previously).

@diegosueiro : Did you try to run samples/subsys/console/echo with this driver? That would a litmus test whether this driver does everything "good enough" or "not good enough". Please be sure to also test clipboard-pasting of reasonable amount of data into the sample (at least CONFIG_CONSOLE_GETCHAR_BUFSIZE should go without any char loss, but on many boards I tried many more than that can go w/o loss, let's say at least 2x).

Adds the Low Energy UART shim serial driver for Silabs Gecko Devices. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

Adds the I2C shim driver for Silabs Gecko Devices. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

diegosueiro · 2018-10-30T19:03:32Z

@pfalcon

@diegosueiro : Did you try to run samples/subsys/console/echo with this driver? That would a litmus test whether this driver does everything "good enough" or "not good enough". Please be sure to also test clipboard-pasting of reasonable amount of data into the sample (at least CONFIG_CONSOLE_GETCHAR_BUFSIZE should go without any char loss, but on many boards I tried many more than that can go w/o loss, let's say at least 2x).

I tested as you suggested and it is working fine.

pfalcon · 2018-10-30T19:14:36Z

I tested as you suggested and it is working fine.

Great! Let me approve from my side.

pfalcon

+1 for UART driver.

galak · 2018-10-30T20:30:15Z

boards/arm/efr32mg_sltb004a/board.c

+
+#include <init.h>
+
+static int efr32mg_sltb004a_init(struct device *dev)


If this doesn't do anything, why do we need it?

galak · 2018-10-30T20:30:40Z

boards/arm/efr32mg_sltb004a/board.h

+#endif
+
+/* Environment Sensors Enable Pin */
+#define ENV_SENSE_ENABLE_NAME	CONFIG_GPIO_GECKO_PORTF_NAME


where is this used?

This one I used for testing an i2c sensor in the board to validate the i2c driver.
I'll remove it.

galak

do we need board.c and some defines in board.h don't seem to be used.

The EFR32™ Mighty Gecko Starter Kit EFR32MG-SLTB004A (a.k.a Thunderboard Sense 2) contains a MCU from the EFR32MG12P family built on ARM® Cortex®-M4F processor with low power capabilities. There is an On-Board J-Link Debugger that presents a virtual COM port for general purpose application serial data transfer with this interface and a Mass Storage for firmware flashing. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

diegosueiro requested review from dbkinder, galak and MaureenHelm as code owners July 20, 2018 07:15

diegosueiro changed the title ~~Thunderboard sense 2~~ Add support for Silabs EFR32MG device and Thunderboard Sense 2 Jul 20, 2018

diegosueiro changed the title ~~Add support for Silabs EFR32MG device and Thunderboard Sense 2~~ Add support for Silabs EFR32MG12P device and Thunderboard Sense 2 Jul 20, 2018

diegosueiro force-pushed the thunderboard_sense_2 branch from 453d1ee to 8cea032 Compare July 20, 2018 12:30

dbkinder suggested changes Jul 20, 2018

View reviewed changes

diegosueiro force-pushed the thunderboard_sense_2 branch 2 times, most recently from e92f489 to 187026b Compare July 24, 2018 05:21

dbkinder approved these changes Jul 25, 2018

View reviewed changes

galak added area: ARM ARM (32-bit) Architecture area: Boards EXT Has change or related to ext/ (obsolete) labels Sep 12, 2018

galak requested changes Sep 12, 2018

View reviewed changes

galak added area: I2C area: UART Universal Asynchronous Receiver-Transmitter labels Sep 12, 2018

galak mentioned this pull request Sep 12, 2018

drivers: Rework Silabs Gecko UART Driver #8930

Merged

galak added the platform: Silabs Silicon Labs label Sep 12, 2018

diegosueiro force-pushed the thunderboard_sense_2 branch from 187026b to 266b741 Compare September 16, 2018 06:07

galak force-pushed the thunderboard_sense_2 branch from f3d4657 to 39e8e31 Compare October 19, 2018 17:12

galak requested a review from pfalcon October 19, 2018 17:14

galak requested changes Oct 19, 2018

View reviewed changes

diegosueiro force-pushed the thunderboard_sense_2 branch from 39e8e31 to a632c53 Compare October 19, 2018 18:32

galak force-pushed the thunderboard_sense_2 branch from a632c53 to f8b2625 Compare October 19, 2018 18:48

galak reviewed Oct 19, 2018

View reviewed changes

pfalcon reviewed Oct 22, 2018

View reviewed changes

pfalcon requested changes Oct 22, 2018

View reviewed changes

pfalcon reviewed Oct 23, 2018

View reviewed changes

chrta mentioned this pull request Oct 23, 2018

Add initial support for Silicon Labs EFM32PG-STK3402A board #7043

Closed

pfalcon mentioned this pull request Oct 23, 2018

include: uart: Another pass on improving docstrings #10761

Merged

ekarlso mentioned this pull request Oct 30, 2018

drivers: flash_gecko: Add flash driver for SiLabs Gecko SoCs #9945

Merged

diegosueiro added 2 commits October 30, 2018 18:26

drivers: Introduce Silabs leuart shim serial driver

a0e25aa

Adds the Low Energy UART shim serial driver for Silabs Gecko Devices. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

drivers: Introduce Silabs i2c shim driver

6ff9acc

Adds the I2C shim driver for Silabs Gecko Devices. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

diegosueiro force-pushed the thunderboard_sense_2 branch from f8b2625 to 5b200fe Compare October 30, 2018 19:00

pfalcon approved these changes Oct 30, 2018

View reviewed changes

galak reviewed Oct 30, 2018

View reviewed changes

galak requested changes Oct 30, 2018

View reviewed changes

diegosueiro force-pushed the thunderboard_sense_2 branch from 5b200fe to bddc8d7 Compare October 30, 2018 20:57

galak approved these changes Oct 31, 2018

View reviewed changes

galak merged commit 5540016 into zephyrproject-rtos:master Oct 31, 2018

nikolaushuber mentioned this pull request Dec 2, 2020

sensor: Add Silabs SI7021 support. #30185

Closed


		The default configuration can be found in the defconfig file:

		``boards/arm/efr32mg_sltb004a/efr32mg_sltb004a_defconfig``



		/* Push button PB0 (SW0) */
		#define PB0_GPIO_NAME CONFIG_GPIO_GECKO_PORTD_NAME


		#include <init.h>

		static int efr32mg_sltb004a_init(struct device *dev)

Add support for Silabs EFR32MG12P device and Thunderboard Sense 2 #9042

Add support for Silabs EFR32MG12P device and Thunderboard Sense 2 #9042

Conversation

diegosueiro commented Jul 20, 2018

diegosueiro commented Jul 20, 2018

chrta commented Jul 20, 2018 • edited Loading

codecov-io commented Jul 20, 2018 • edited Loading

Codecov Report

diegosueiro commented Jul 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbkinder left a comment

Choose a reason for hiding this comment

diegosueiro commented Aug 3, 2018

galak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegosueiro commented Sep 16, 2018 • edited Loading

diegosueiro commented Sep 16, 2018

chrta commented Sep 16, 2018

diegosueiro commented Sep 16, 2018

chrta commented Sep 16, 2018

diegosueiro commented Sep 16, 2018

galak commented Oct 19, 2018

galak commented Oct 19, 2018

galak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfalcon left a comment • edited Loading

Choose a reason for hiding this comment

pfalcon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfalcon commented Oct 23, 2018

diegosueiro commented Oct 30, 2018

pfalcon commented Oct 30, 2018

pfalcon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrta commented Jul 20, 2018 •

edited

Loading

codecov-io commented Jul 20, 2018 •

edited

Loading

diegosueiro commented Sep 16, 2018 •

edited

Loading

pfalcon left a comment •

edited

Loading