hwinfo: cleanup API doc and test #22947

alexanderwachter · 2020-02-19T17:40:56Z

Change the doxygen doc from "negative on error" to -ENOSUP only.
Enhance testing.
Fix build-errors on NRF53, STM32H7, SAM3x, SAM4e, SAM4s

Fixes #22979

galak

We should probably fix the tests/drivers/hwinfo/api/src/main.c to reflect what's documented here.

ie, -ENOTSUP is a valid return and shouldn't assert. An other negative value should assert since that would be a violation of the API.

alexanderwachter · 2020-02-19T18:04:10Z

We should probably fix the tests/drivers/hwinfo/api/src/main.c to reflect what's documented here.

ie, -ENOTSUP is a valid return and shouldn't assert. An other negative value should assert since that would be a violation of the API.

We only test on boards that have a hardware ID. It it returns -ENOSUP, we have a problem with the implementation.

galak · 2020-02-19T18:16:34Z

We only test on boards that have a hardware ID. It it returns -ENOSUP, we have a problem with the implementation.

If you want to handle that in the test I think its fine, you can add to Kconfig option to tell if a driver is enabled or not and thus distinguish if -ENOSUP is a valid response or not.

diff --git a/drivers/hwinfo/Kconfig b/drivers/hwinfo/Kconfig
index 173dfa9672..7b5f0c0b51 100644
--- a/drivers/hwinfo/Kconfig
+++ b/drivers/hwinfo/Kconfig
@@ -16,9 +16,13 @@ config HWINFO_SHELL
        help
          Enable hwinfo Shell for testing.
 
+config HWINFO_HAS_DRIVER
+       bool
+
 config HWINFO_STM32
        bool "STM32 hwinfo"
        default y
+       select HWINFO_HAS_DRIVER
        depends on SOC_FAMILY_STM32
        select USE_STM32_LL_UTILS
        help
@@ -27,6 +31,7 @@ config HWINFO_STM32
 config HWINFO_NRF
        bool "NRF device ID"
        default y
+       select HWINFO_HAS_DRIVER
        depends on SOC_FAMILY_NRF
        help
          Enable Nordic NRF hwinfo driver.
...

galak · 2020-02-19T18:18:17Z

Than we can also get ride of 'hwinfo' from the board yaml and tests/drivers/hwinfo/api/testcase.yaml and change testcase.yaml filter on CONFIG_HWINFO

galak · 2020-02-19T18:19:19Z

I'm fine approving and merging the doc update PR, just be aware I'd like to see the other changes in testing for us to have this considered a stable API.

galak

looks good, one minor tweak on the test yaml

we should also remove hwinfo from all board.yaml that have it.

galak · 2020-02-19T18:50:11Z

tests/drivers/hwinfo/api/testcase.yaml

@@ -1,4 +1,3 @@
 tests:
  drivers.device_id:
    tags: driver
-    depends_on: hwinfo


make this filter: CONFIG_HWINFO

galak

On the 'Error returned:' in the test for negative retval, should we make that message a bit more verbose?

'Unexpected negative return value' or something?

alexanderwachter · 2020-02-19T20:32:48Z

we should also remove hwinfo from all board.yaml that have it.

Why should we remove it? It states which boards have drivers for hwinfo.

alexanderwachter · 2020-02-20T09:02:28Z

we should also remove hwinfo from all board.yaml that have it.

Why should we remove it? It states which boards have drivers for hwinfo.

@galak I think I understand now. The yamls are for tests only and with the new tests, we also test for not supported. That way we can test on every board.

addressed

ioannisg · 2020-02-20T18:04:33Z

Somebody needs to test the SAM3X and SAM4X hwinfo. I don't have any of these boards.
@fallrisk @mnkp @nandojve

I help with tests on SAM4S/E variants. I don't have any SAM3 board.

I can test on SAM3X, hold on

alexanderwachter · 2020-02-20T18:47:02Z

@nandojve @ioannisg can you guys check if the ID differs on two different boards? You can use the shell module to check. Just to make sure that I got the address right.

nandojve · 2020-02-20T22:33:34Z

@nandojve @ioannisg can you guys check if the ID differs on two different boards? You can use the shell module to check. Just to make sure that I got the address right.

Hi @alexanderwachter, I already tested on both boards. I separated values for better reading on the post. I added values that I get using Atmel Studio.

SAM4E
uart:~$ hwinfo devid 
Length: 16
ID: 0x00313353_50334337_30333032_30303436
[0] 0x53333100
[1] 0x37433350
[2] 0x32303330
[3] 0x36343030
uart:~$ 

SAM4S
uart:~$ hwinfo devid
Length: 16
ID: 0x00310053_594a5135_30323032_33303830
[0] 0x53003100
[1] 0x35514a59
[2] 0x32303230
[3] 0x30383033
uart:~$

Below I executed a demo inside Atmel Studio to get Device ID.

-- Flash Read Unique Identifier Example --
-- SAM4S-XPLAINED --
-- Compiled: Feb 20 2020 18:26:05 --
-I- Reading 128 bits Unique Identifier
	    [0]		[1]	  [2]	     [3]
-I- ID: 1392521472, 894519897, 842019376, 808988723
SAM4S
	0x53003100, 0x35514a59, 0x32303230, 0x30383033
SAM4E
        0x53333100, 0x37433350, 0x32303330, 0x36343030

alexanderwachter · 2020-02-21T06:34:19Z

Thanks a lot @nandojve !

erwango

One minor related change requested, otherwise I'm fine with the change

erwango · 2020-02-26T16:50:52Z

drivers/hwinfo/Kconfig

@@ -21,55 +24,63 @@ config HWINFO_STM32
 	default y
 	depends on SOC_FAMILY_STM32
 	select USE_STM32_LL_UTILS


This could actually be removed since driver only require headers, not utils functions

Thanks. Removed.

erwango · 2020-02-27T09:42:31Z

Before approving: I've tried to run sanitycheck, and there's a strange ERROR message reported (with no impact on final result):

$ sanitycheck -T tests/drivers/hwinfo/ -p nucleo_f401re
Renaming output directory to /local/mcu/zephyrproject/zephyr/sanity-out.2
INFO    - JOBS: 8
ERROR   - /local/mcu/zephyrproject/zephyr/tests/drivers/hwinfo/api/src/main.c: found invalid #endif, #ifdef in ztest_test_suite()
INFO    - Building initial testcase list...
INFO    - 1 test configurations selected, 0 configurations discarded due to filters.
INFO    - Adding tasks to the queue...
INFO    - Total complete:    1/   1  100%  skipped:    0, failed:    0
INFO    - 1 of 1 tests passed (100.00%), 0 failed, 0 skipped with 0 warnings in 4.94 seconds
INFO    - In total 1 test cases were executed on 1 out of total 229 platforms (0.44%)

Can you check the ERROR line ?

alexanderwachter · 2020-02-27T12:33:27Z

@erwango pushed a new version that does not have the error above.
The fix is not to have a #ifdef in the test-suite but replace the tests with dummies depending on the configuration.

The test checks for a correct implementation if HWINFO_HAS_DRIVER is set, otherwise it checks for -ENOTSUP return value. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

Make the NRF hwinfo driver depending on !TRUSTED_EXECUTION_NONSECURE because the FICR registers are not accessible from the non-secure world. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

Remove all "supported: -hwinfo" definitions from the boards yaml files and documentation. hwinfo can generally be tested on every board because it returns -ENOTSUP if not supported. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

Include stm32XXxx_ll_utils.h in soc.h for every stm32 SoC, if CONFIG_HWINFO_STM32 is selected. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

CLOE (Code Loop Optimization) does not exist on SAM3x. Make the EEFC_FMR_CLOE disable depending on CONFIG_SOC_SERIES_SAM3X. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

This commit adds Device-Tree instances of the Flash controller to the SAM3X, SAM4E and SAM4S series. The Flash-Controller is used to get the unique device identifier. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

carlescufi · 2020-02-27T15:37:27Z

@jhedberg this should be ready

carlescufi · 2020-02-27T15:37:47Z

@jhedberg this should be ready

carlescufi · 2020-02-27T15:38:04Z

@jhedberg this should be ready

jhedberg · 2020-02-27T15:55:26Z

@jhedberg this should be ready

@carlescufi ok, I'll merge once CI completes.

alexanderwachter · 2020-02-27T18:40:08Z

@jhedberg the test are timing out. tests/drivers/hwinfo/api/drivers.device_id tests are all green.

jhedberg · 2020-02-27T18:41:27Z

@jhedberg the test are timing out. tests/drivers/hwinfo/api/drivers.device_id tests are all green.

Yes, I've had to restart Shippable a few times because of this. Fingers crossed that it wont time out again.

alexanderwachter added area: Documentation Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. area: HWINFO Hardware Information Driver labels Feb 19, 2020

alexanderwachter requested review from nashif and carlescufi February 19, 2020 17:40

alexanderwachter force-pushed the hwinfo_doc_retval branch from 2fa5a8b to 43a159e Compare February 19, 2020 17:41

carlescufi requested a review from galak February 19, 2020 17:41

zephyrbot added the area: API Changes to public APIs label Feb 19, 2020

carlescufi approved these changes Feb 19, 2020

View reviewed changes

galak requested changes Feb 19, 2020

View reviewed changes

zephyrbot added the area: Tests Issues related to a particular existing or missing test label Feb 19, 2020

galak requested changes Feb 19, 2020

View reviewed changes

galak changed the title ~~doc: hwinfo Clarify return values.~~ hwinfo: cleanup API doc and test Feb 19, 2020

galak reviewed Feb 19, 2020

View reviewed changes

alexanderwachter force-pushed the hwinfo_doc_retval branch from 169a316 to 670af6b Compare February 19, 2020 20:31

alexanderwachter removed the Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. label Feb 19, 2020

alexanderwachter requested a review from MaureenHelm as a code owner February 20, 2020 08:59

alexanderwachter requested review from nordic-krch and ioannisg February 20, 2020 08:59

zephyrbot added the area: Boards label Feb 20, 2020

alexanderwachter requested review from carlescufi and galak February 20, 2020 09:04

alexanderwachter force-pushed the hwinfo_doc_retval branch from d51fd8e to 2a3afae Compare February 20, 2020 12:37

alexanderwachter requested a review from erwango as a code owner February 20, 2020 12:37

alexanderwachter removed the dev-review To be discussed in dev-review meeting label Feb 21, 2020

nandojve mentioned this pull request Feb 26, 2020

Convert SAM SoC drivers to DT_INST #23107

Closed

61 tasks

erwango requested changes Feb 26, 2020

View reviewed changes

alexanderwachter force-pushed the hwinfo_doc_retval branch from 82c1b69 to f76adec Compare February 26, 2020 20:06

erwango self-requested a review February 27, 2020 09:40

alexanderwachter force-pushed the hwinfo_doc_retval branch from f76adec to 4e0ec73 Compare February 27, 2020 12:35

alexanderwachter added 6 commits February 27, 2020 13:36

tests: hwinfo: Make the tests depending on HWINFO_HAS_DRIVER

5ee8c7e

The test checks for a correct implementation if HWINFO_HAS_DRIVER is set, otherwise it checks for -ENOTSUP return value. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

drivers: hwinfo: Exclude NRF53 non-secure from hwinfo driver

b7dc7c9

Make the NRF hwinfo driver depending on !TRUSTED_EXECUTION_NONSECURE because the FICR registers are not accessible from the non-secure world. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

SoC: stm32: Include LL utilities if HWINFO is selected

3ad65e8

Include stm32XXxx_ll_utils.h in soc.h for every stm32 SoC, if CONFIG_HWINFO_STM32 is selected. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

drivers: hwinfo: Don't disable CLOE on SAM3x seies

660f628

CLOE (Code Loop Optimization) does not exist on SAM3x. Make the EEFC_FMR_CLOE disable depending on CONFIG_SOC_SERIES_SAM3X. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

SoC: SAM3x and SAM4: Add the Flash Controller to the DT

4e0ec73

This commit adds Device-Tree instances of the Flash controller to the SAM3X, SAM4E and SAM4S series. The Flash-Controller is used to get the unique device identifier. Signed-off-by: Alexander Wachter <alexander@wachter.cloud>

erwango approved these changes Feb 27, 2020

View reviewed changes

carlescufi approved these changes Feb 27, 2020

View reviewed changes

jhedberg approved these changes Feb 27, 2020

View reviewed changes

carlescufi merged commit b96477d into zephyrproject-rtos:master Feb 27, 2020

alexanderwachter deleted the hwinfo_doc_retval branch January 30, 2021 09:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hwinfo: cleanup API doc and test #22947

hwinfo: cleanup API doc and test #22947

alexanderwachter commented Feb 19, 2020 •

edited

galak left a comment

alexanderwachter commented Feb 19, 2020

galak commented Feb 19, 2020

galak commented Feb 19, 2020

galak commented Feb 19, 2020

galak left a comment

galak Feb 19, 2020

galak left a comment

alexanderwachter commented Feb 19, 2020

alexanderwachter commented Feb 20, 2020

ioannisg commented Feb 20, 2020

alexanderwachter commented Feb 20, 2020

nandojve commented Feb 20, 2020

alexanderwachter commented Feb 21, 2020

erwango left a comment

erwango Feb 26, 2020

alexanderwachter Feb 26, 2020

erwango commented Feb 27, 2020

alexanderwachter commented Feb 27, 2020

carlescufi commented Feb 27, 2020

carlescufi commented Feb 27, 2020

carlescufi commented Feb 27, 2020

jhedberg commented Feb 27, 2020

alexanderwachter commented Feb 27, 2020

jhedberg commented Feb 27, 2020

hwinfo: cleanup API doc and test #22947

hwinfo: cleanup API doc and test #22947

Conversation

alexanderwachter commented Feb 19, 2020 • edited

galak left a comment

Choose a reason for hiding this comment

alexanderwachter commented Feb 19, 2020

galak commented Feb 19, 2020

galak commented Feb 19, 2020

galak commented Feb 19, 2020

galak left a comment

Choose a reason for hiding this comment

galak Feb 19, 2020

Choose a reason for hiding this comment

galak left a comment

Choose a reason for hiding this comment

alexanderwachter commented Feb 19, 2020

alexanderwachter commented Feb 20, 2020

ioannisg commented Feb 20, 2020

alexanderwachter commented Feb 20, 2020

nandojve commented Feb 20, 2020

alexanderwachter commented Feb 21, 2020

erwango left a comment

Choose a reason for hiding this comment

erwango Feb 26, 2020

Choose a reason for hiding this comment

alexanderwachter Feb 26, 2020

Choose a reason for hiding this comment

erwango commented Feb 27, 2020

alexanderwachter commented Feb 27, 2020

carlescufi commented Feb 27, 2020

carlescufi commented Feb 27, 2020

carlescufi commented Feb 27, 2020

jhedberg commented Feb 27, 2020

alexanderwachter commented Feb 27, 2020

jhedberg commented Feb 27, 2020

alexanderwachter commented Feb 19, 2020 •

edited