drivers: Rework Silabs Gecko UART Driver #8930

diegosueiro · 2018-07-13T17:34:34Z

This patch series introduces a new config HAS_DTS_SERIAL and reworks the Silabs Gecko UART driver and associated soc and board files to enable using values defined by the device tree configuration.

codecov-io · 2018-07-13T18:04:31Z

Codecov Report

Merging #8930 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8930      +/-   ##
==========================================
- Coverage   53.34%   53.19%   -0.16%     
==========================================
  Files         211      211              
  Lines       25828    25828              
  Branches     5686     5686              
==========================================
- Hits        13779    13738      -41     
- Misses       9734     9781      +47     
+ Partials     2315     2309       -6

Impacted Files	Coverage Δ
include/posix/posix_sched.h	`0% <0%> (-100%)`	⬇️
lib/posix/pthread_barrier.c	`0% <0%> (-100%)`	⬇️
lib/posix/pthread_cond.c	`62.5% <0%> (-20.84%)`	⬇️
lib/posix/pthread.c	`68.03% <0%> (-12.33%)`	⬇️
lib/posix/pthread_mutex.c	`76.38% <0%> (-1.39%)`	⬇️
lib/posix/pthread_sched.c	`62.5% <0%> (+20.83%)`	⬆️

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 c0a6801...231cf44. Read the comment docs.

chrta · 2018-07-15T12:05:49Z

drivers/serial/Kconfig.gecko

@@ -26,7 +26,7 @@ if UART_GECKO_0
 config UART_GECKO_0_GPIO_LOC
 	int "Pin Locations"
 	range 0 3
-	depends on UART_GECKO
+	depends on UART_GECKO && !HAS_DTS_SERIAL


Or could we enforce HAS_DTS_SERIAL somehow, so all gecko mcus must use it? In that case all _LOC config options could be removed from Kconfig.gecko.

@chrta,

I totally agree with you.

@galak,

What do you think? Can we remove the *_LOC from Kconfig?

diegosueiro · 2018-07-24T04:43:27Z

@galak and @MaureenHelm ,

Any feedbacks?

diegosueiro · 2018-08-03T11:15:36Z

@galak and @MaureenHelm ,

Any feedbacks?

diegosueiro · 2018-09-12T05:09:36Z

@galak, can we proceed with the review?

galak · 2018-09-12T05:41:06Z

These changes seem part of the PR #9042. I made comments over there.

galak · 2018-10-10T22:48:41Z

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

-title: EFM32 UART
-id: silabs,efm32-uart
+title: GECKO UART
+id: silabs,gecko-uart


Trying to remove id: so lets leave that alone

galak

Use CONFIG_ for dt generated defines. This makes it easier for us to know what is being generated by DT rather than dropping the CONFIG_ prefix. Eventually this will go away when we do more codegen and remove dts fixups.

Also, might need updating now that we merged the EFM32HG support

galak · 2018-10-10T22:49:02Z

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

-title: EFM32 USART
-id: silabs,efm32-usart
+title: GECKO USART
+id: silabs,gecko-usart


same comment about id.

galak · 2018-10-10T22:49:38Z

soc/arm/silabs_exx32/efr32fg1p/soc_pinmap.h

@@ -33,15 +33,15 @@
 #endif /* CONFIG_USART_GECKO_0 */

 #ifdef CONFIG_USART_GECKO_1
-#if (CONFIG_USART_GECKO_1_GPIO_LOC == 0)
+#if (USART_1_LOCATION == 0)


leave these as CONFIG_

galak · 2018-10-10T22:49:43Z

soc/arm/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_LOCATION == 0)


leave as CONFIG_

galak · 2018-10-10T22:49:56Z

soc/arm/silabs_exx32/efr32fg1p/dts.fixup

@@ -7,9 +7,5 @@
 /* SoC level DTS fixup file */

 #define CONFIG_NUM_IRQ_PRIO_BITS	ARM_V7M_NVIC_E000E100_ARM_NUM_IRQ_PRIORITY_BITS
-#define CONFIG_USART_GECKO_0_NAME	SILABS_EFM32_USART_40010000_LABEL


Leave this mapping

galak · 2018-10-10T22:52:13Z

soc/arm/silabs_exx32/efm32wg/soc_pinmap.h

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

 #ifdef CONFIG_SOC_PART_NUMBER_EFM32WG990F256
 #ifdef CONFIG_UART_GECKO_0
-#if (CONFIG_UART_GECKO_0_GPIO_LOC == 0)
+#if (UART_0_LOCATION == 0)


keep these as CONFIG_

ekarlso · 2018-10-15T12:12:16Z

Hi, can we rebase this ontop https://github.com/ekarlso/zephyr/tree/silabs-uart @diegosueiro ?

carlescufi · 2018-10-16T09:30:36Z

@ekarlso this was rebased on top of your tree? do you have a PR outstanding or is this the only Gecko UART driver PR open?

diegosueiro · 2018-10-16T17:27:34Z

@carlescufi and @galak,

Branch rebased and all checks have passed. ;-)

Use CONFIG_UART_GECKO instead of CONFIG_SERIAL_HAS_DRIVER to compile the low level uart driver. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

Add more uart/usart instances for Silabs Gecko Devices and remove the *_GPIO_LOC configs. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

Introduces the location property and adds the ability to use values generated by the device tree configuration. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com> Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

galak

I've made some updates, dropped board.h from driver. Used standard CONFIG_ naming convention for now, dropped 'id:' from yaml.

Can you test this out to make sure it still works.

galak · 2018-10-16T17:37:18Z

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

@@ -1,16 +1,17 @@
 ---
-title: EFM32 USART
+title: GECKO USART
+id: silabs,efm32-usart


galak · 2018-10-16T17:37:27Z

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

@@ -1,16 +1,17 @@
 ---
-title: EFM32 UART
+title: GECKO UART
+id: silabs,efm32-uart


galak · 2018-10-16T17:37:56Z

drivers/serial/uart_gecko.c

@@ -10,6 +10,7 @@
 #include <em_gpio.h>
 #include <em_cmu.h>
 #include <soc.h>
+#include <board.h>


why do we need board.h included?

Because for efr32mg_sltb004a board the board.h includes the "soc_pinmap.h", inside the board folder, which holds the UART pin settings.

The other silabs boards are setting the pins in the "soc_pinmap.h" header, inside the soc folder, which is included by the "soc.h" header, and I think is not the best approach since pin configuration/muxing should be set at board level, not soc. What do you think?

The mapping which location corresponds to which pin is a property of the soc. But the location that is actually chosen is part of the board. Or am I missing something here?

Its a 50/50 case, in that we set some standard pin cfg in the soc headers for a number of SoCs. The board ends up select a default or pre-cfg option in a lot of cases. Eventually we will move pin cfg info into the device tree. For now I'd keep with the same pattern that other SiLabs boards/SoC are using.

Ok.

I'll update the PR #9042 as soon as this one gets merged.

diegosueiro · 2018-10-16T19:39:25Z

@galak,

I've made some updates, dropped board.h from driver. Used standard CONFIG_ naming convention for now, dropped 'id:' from yaml.

Can you test this out to make sure it still works.

The only hardware that I have is the thunderboard from the other PR #9042.

But the changes looks fine to me apart from dropping the board.h as I explained here

galak · 2018-10-16T20:04:51Z

But the changes looks fine to me apart from dropping the board.h as I explained here

ok, let me look at that.

chrta

This is working fine on the EFM32WG-STK3800

diegosueiro requested review from galak and MaureenHelm as code owners July 13, 2018 17:34

diegosueiro force-pushed the uart_gecko branch from 9474341 to 74a2080 Compare July 14, 2018 06:48

chrta reviewed Jul 17, 2018

View reviewed changes

diegosueiro force-pushed the uart_gecko branch 2 times, most recently from d840aa7 to 688125a Compare July 19, 2018 05:35

diegosueiro mentioned this pull request Jul 20, 2018

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

Merged

diegosueiro force-pushed the uart_gecko branch from 688125a to 258804c Compare August 16, 2018 02:41

galak added area: UART Universal Asynchronous Receiver-Transmitter area: Devicetree platform: Silabs Silicon Labs labels Sep 12, 2018

diegosueiro force-pushed the uart_gecko branch 3 times, most recently from 9f0ff44 to d925896 Compare September 16, 2018 19:38

diegosueiro force-pushed the uart_gecko branch from d925896 to f74ce50 Compare September 23, 2018 06:18

galak reviewed Oct 10, 2018

View reviewed changes

galak requested changes Oct 10, 2018

View reviewed changes

diegosueiro force-pushed the uart_gecko branch 3 times, most recently from 2e63807 to 231cf44 Compare October 16, 2018 06:47

diegosueiro force-pushed the uart_gecko branch 2 times, most recently from 2910ccf to 2bc4ffa Compare October 16, 2018 16:51

diegosueiro added 2 commits October 16, 2018 14:26

ext: Silabs Gecko use a different config for uart

d356850

Use CONFIG_UART_GECKO instead of CONFIG_SERIAL_HAS_DRIVER to compile the low level uart driver. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

drivers: Add more uart instances for Silabs Devices

bbf4d64

Add more uart/usart instances for Silabs Gecko Devices and remove the *_GPIO_LOC configs. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com>

galak force-pushed the uart_gecko branch from 2bc4ffa to 9fbffe9 Compare October 16, 2018 19:26

drivers: serial: Rework Silabs Gecko UART Driver

eaa1d6c

Introduces the location property and adds the ability to use values generated by the device tree configuration. Signed-off-by: Diego Sueiro <diego.sueiro@gmail.com> Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

galak force-pushed the uart_gecko branch from 9fbffe9 to eaa1d6c Compare October 16, 2018 19:27

galak reviewed Oct 16, 2018

View reviewed changes

galak approved these changes Oct 16, 2018

View reviewed changes

chrta approved these changes Oct 16, 2018

View reviewed changes

galak merged commit 0c7a28c into zephyrproject-rtos:master Oct 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: Rework Silabs Gecko UART Driver #8930

drivers: Rework Silabs Gecko UART Driver #8930

diegosueiro commented Jul 13, 2018

codecov-io commented Jul 13, 2018 •

edited

Loading

chrta Jul 15, 2018

diegosueiro Jul 17, 2018

diegosueiro commented Jul 24, 2018

diegosueiro commented Aug 3, 2018

diegosueiro commented Sep 12, 2018

galak commented Sep 12, 2018

galak Oct 10, 2018

galak left a comment

galak Oct 10, 2018

galak Oct 10, 2018

galak Oct 10, 2018

galak Oct 10, 2018

galak Oct 10, 2018

ekarlso commented Oct 15, 2018

carlescufi commented Oct 16, 2018

diegosueiro commented Oct 16, 2018

galak left a comment

galak Oct 16, 2018

galak Oct 16, 2018

galak Oct 16, 2018

diegosueiro Oct 16, 2018 •

edited

Loading

chrta Oct 16, 2018

galak Oct 16, 2018

diegosueiro Oct 16, 2018

diegosueiro commented Oct 16, 2018 •

edited

Loading

galak commented Oct 16, 2018

chrta left a comment

drivers: Rework Silabs Gecko UART Driver #8930

drivers: Rework Silabs Gecko UART Driver #8930

Conversation

diegosueiro commented Jul 13, 2018

codecov-io commented Jul 13, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegosueiro commented Jul 24, 2018

diegosueiro commented Aug 3, 2018

diegosueiro commented Sep 12, 2018

galak commented Sep 12, 2018

Choose a reason for hiding this comment

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

ekarlso commented Oct 15, 2018

carlescufi commented Oct 16, 2018

diegosueiro commented Oct 16, 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

diegosueiro Oct 16, 2018 • edited Loading

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 Oct 16, 2018 • edited Loading

galak commented Oct 16, 2018

chrta left a comment

Choose a reason for hiding this comment

codecov-io commented Jul 13, 2018 •

edited

Loading

diegosueiro Oct 16, 2018 •

edited

Loading

diegosueiro commented Oct 16, 2018 •

edited

Loading