Making SPI Loopback Test more generic #5885

dwagenk · 2018-01-29T23:26:14Z

When I wanted to add a configuration for stm32f072b_disco board to SPI loopback test in #5405 @ydamigos pointed out, that most of the configuration options where uneeded (because they already derived from boards KConfig.defconfig). This also applies to most of the existing prf_<board>.conf files for this test.

I modified the test, to not have many (almost) identical conf files and a bunch of conditional compilation in the source, but use kconfig and a prj_base.conf instead.

I'd appreciate comments, especially regarding these aspects:

CMakeLists.txt

selection of conf-file(s) is inspired by @superna9999 approach for the i2c_loopback test (#5224), but slightly modified.
Is this or should this be standardized?

QUARK_SE_C1000_DEVBOARD

These changes schouldn't have any effect when building for this board. Can somebody verify that? ~~I have no knowledge of this board, so I didn't change it.~~ There was a conditional compilation block relying on CONFIG_SOC_QUARK_SE_C1000_SS, but no project configuration for any of the sensor-subsystem boards that define it. I dropped it.

Frequency when running slow spi test

The default SPI frequency for the slow test was set to 500k for STM32 boards and to 128k for all other boards. STM32 can't go that slow. ~~Would it be OK, to set a default frequency for all boards, that is a little faster (e.g. 256k)?~~ Default now is 500k, boards that had project configuration files for this test and used the 128k speed are configured this way after the Kconfig transition as well.

Loglevel

Different boards hat different loglevels set, ~~maybe that could be harmonized?~~ Harmonized

Fixes #4576

codecov-io · 2018-01-29T23:43:33Z

Codecov Report

Merging #5885 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5885      +/-   ##
==========================================
+ Coverage   52.88%   52.88%   +<.01%     
==========================================
  Files         412      412              
  Lines       40274    40274              
  Branches     7801     7801              
==========================================
+ Hits        21297    21298       +1     
+ Misses      15761    15760       -1     
  Partials     3216     3216

Impacted Files	Coverage Δ
samples/philosophers/src/main.c	`98.43% <0%> (+1.56%)`	⬆️

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 bc5b395...a19b1b1. Read the comment docs.

superna9999

Apart these small cosmetic changes, ok with this !

superna9999 · 2018-01-29T23:51:59Z

tests/drivers/spi/spi_loopback/Kconfig

+config SPI_LOOPBACK_CS_GPIO
+        bool "SPI port CS pin is controlled via a GPIO port during test"
+        depends on GPIO
+        default n


These does seem to be aligned the same

ydamigos

Could you also add a testcase.yaml file?

dwagenk · 2018-01-31T23:10:21Z

@ydamigos I will, but won't get to it this week

dwagenk · 2018-01-31T23:36:34Z

can somebody of the quark devboard users (@SavinayDharmappa that's you, right?) also take a look to verify the changes don't have any (negative) effect on it?

nashif · 2018-02-09T16:12:49Z

please rebase and add a testcase.yaml file per this issue #4576.

dwagenk · 2018-02-10T13:56:04Z

I need assistance on the QUARK_SE_C1000_SS here. (@tbursztyka you introduced this test, right?)
The old test had a section like this:

#elif defined(CONFIG_SOC_QUARK_SE_C1000_SS)

#define SPI_DRV_NAME CONFIG_SPI_0_NAME
#define SPI_SLAVE 0
#define CS_CTRL_GPIO_DRV_NAME CONFIG_GPIO_DW_0_NAME

struct spi_cs_control spi_cs = {
	.gpio_pin = 0,
	.delay = 0
};

So i guess the test is meant to support the sensor subsystem, but there's no project configuration to actually compile it for QUARK_SE_C1000_SS_DEVBOARD. Tried builduing it and it failed (latest maaster and this pr with some further modifications).

Can I "drop" support for this SoC, when changing the test?

Right now in the testcase.yaml I need to blacklist QUARK_SE_C1000_SS_DEVBOARD and ARDUINO_101_SSS for sanitycheck to pass localy

Convert this test to using KConfig instead of conditional compilation. Standard configuration is provided by prj_base.conf and KConfig default values. Remove prj_<board>.conf files. Signed-off-by: Daniel Wagenknecht <[email protected]>

This restores the board specific configurations that were present before moving the test to using KConfig. <board>.conf files from boards subdirectory get merged with prj_base.conf file. Signed-off-by: Daniel Wagenknecht <[email protected]>

Add a testcase.yaml configuration to spi_loopback test to include it into sanitycheck. Fixes #4576 Signed-off-by: Daniel Wagenknecht <[email protected]>

tbursztyka · 2018-02-13T10:44:26Z

@dwagenk Blacklist them for now on. quark_se is going to switch to native driver by default soonish. If the switch works: I'll un-blacklist this test for relevant boards.

dwagenk · 2018-02-14T08:07:38Z

@tbursztyka OK, that's what I did in last commit. Any other comments?
@superna9999 @ydamigos please review again.

tbursztyka

Nice changes

superna9999

Nice changeset, finally my work goes somewhere !

superna9999 suggested changes Jan 29, 2018

View reviewed changes

ydamigos reviewed Jan 30, 2018

View reviewed changes

Daniel Wagenknecht added 3 commits February 11, 2018 02:40

tests: spi_loopback: Move to using KConfig

4498db3

Convert this test to using KConfig instead of conditional compilation. Standard configuration is provided by prj_base.conf and KConfig default values. Remove prj_<board>.conf files. Signed-off-by: Daniel Wagenknecht <[email protected]>

tests: spi_loopback: add testcase.yaml

a19b1b1

Add a testcase.yaml configuration to spi_loopback test to include it into sanitycheck. Fixes #4576 Signed-off-by: Daniel Wagenknecht <[email protected]>

dwagenk changed the title ~~[RFC][DNM] Making SPI Loopback Test more generic~~ Making SPI Loopback Test more generic Feb 11, 2018

tbursztyka approved these changes Feb 14, 2018

View reviewed changes

ydamigos approved these changes Feb 14, 2018

View reviewed changes

superna9999 approved these changes Feb 14, 2018

View reviewed changes

nashif merged commit 0aef903 into zephyrproject-rtos:master Feb 14, 2018

dwagenk deleted the spi_loopback branch February 15, 2018 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making SPI Loopback Test more generic #5885

Making SPI Loopback Test more generic #5885

dwagenk commented Jan 29, 2018 •

edited by nashif

Loading

codecov-io commented Jan 29, 2018 •

edited

Loading

superna9999 left a comment

superna9999 Jan 29, 2018

dwagenk Jan 30, 2018

ydamigos left a comment

dwagenk commented Jan 31, 2018

dwagenk commented Jan 31, 2018

nashif commented Feb 9, 2018

dwagenk commented Feb 10, 2018

tbursztyka commented Feb 13, 2018

dwagenk commented Feb 14, 2018

tbursztyka left a comment

superna9999 left a comment

Making SPI Loopback Test more generic #5885

Making SPI Loopback Test more generic #5885

Conversation

dwagenk commented Jan 29, 2018 • edited by nashif Loading

CMakeLists.txt

QUARK_SE_C1000_DEVBOARD

Frequency when running slow spi test

Loglevel

codecov-io commented Jan 29, 2018 • edited Loading

Codecov Report

superna9999 left a comment

Choose a reason for hiding this comment

superna9999 Jan 29, 2018

Choose a reason for hiding this comment

dwagenk Jan 30, 2018

Choose a reason for hiding this comment

ydamigos left a comment

Choose a reason for hiding this comment

dwagenk commented Jan 31, 2018

dwagenk commented Jan 31, 2018

nashif commented Feb 9, 2018

dwagenk commented Feb 10, 2018

tbursztyka commented Feb 13, 2018

dwagenk commented Feb 14, 2018

tbursztyka left a comment

Choose a reason for hiding this comment

superna9999 left a comment

Choose a reason for hiding this comment

dwagenk commented Jan 29, 2018 •

edited by nashif

Loading

codecov-io commented Jan 29, 2018 •

edited

Loading