Add FPGA Timer feature and example test updates #11147

mprse · 2019-08-01T10:45:13Z

Description

The following PR adds Verilog FPGA Timer implementation: ARMmbed/fpga-ci-test-shield#1 (must be merged before this one).

The intention of FPGA Timer is to have a very accurate timer to measure elapsed time or generate delays. The FPGA timer might be very useful in the timing tests, which are currently quite problematic.

Possible usage of the FPGA Timer:

Use FPGA Timer for timing tests which currently uses Time Drift model and communicate with the Host PC. These tests are not very accurate and were causing problems on CI (at the moment all time drift tests are disabled on CI). We can update these tests to use FPGA timer instead Time Drift model and reenable them on CI (e98819b, dd8d8f4).
Use FPGA Timer as a reference in Ticker/Timer/Timeout tests. As far as I remember in these tests lp ticker is used to verify us ticker and vice versa. We could improve these tests to use FPGA Timer as a reference.
Watchdog tests. With FPGA Timer we can probably check when the watchdog fired.
All other timing tests: task scheduling, system tick, event queue, etc.

Example code:

#include "mbed.h"
#include "MbedTester.h"

const PinList *form_factor = pinmap_ff_default_pins();
const PinList *restricted = pinmap_restricted_pins();
MbedTester tester(form_factor, restricted);

void test()
{
    tester.reset();

    tester.timer_init();

    tester.peripherals_reset();

    tester.select_peripheral(MbedTester::PeripheralTimer);

    // FPGA Timer Example - count elapsed time
    tester.timer_set_mode(MbedTester::TimerModeTimer);

    tester.timer_reset();

    tester.timer_start();

    wait_ns(5000000); // 5 ms

    tester.timer_stop();

    printf("FPGA Timer [ns]: %llu \r\n", tester.timer_read_ns());
    printf("FPGA Timer [us]: %lu \r\n", tester.timer_read_us());
    printf("FPGA Timer [ms]: %lu \r\n", tester.timer_read_ms());

    // FPGA Count-Down-Timer Example - perform delay
    tester.timer_set_mode(MbedTester::TimerModeCountDownTimer);

    tester.timer_set_delay_ms(10);

    tester.timer_reset();

    Timer timer;

    timer.start();

    tester.timer_delay();

    timer.stop();

    printf("Mbed Timer [us] : %i \r\n", timer.read_us());
    printf("Mbed Timer [ms] : %i \r\n", timer.read_ms());

    tester.timer_free();
}

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@c1728p9 @jamesbeyond @fkjagodzinski @maciejbocianski

Example usage: ``` const PinList *form_factor = pinmap_ff_default_pins(); const PinList *restricted = pinmap_restricted_pins(); MbedTester tester(form_factor, restricted); void test() { tester.reset(); tester.timer_init(); tester.peripherals_reset(); tester.select_peripheral(MbedTester::PeripheralTimer); // FPGA Timer Example- count elapsed time tester.timer_set_mode(MbedTester::TimerModeTimer); tester.timer_reset(); tester.timer_start(); wait_ns(5000000); // 5 ms tester.timer_stop(); printf("FPGA Timer [ns]: %llu \r\n", tester.timer_read_ns()); printf("FPGA Timer [us]: %lu \r\n", tester.timer_read_us()); printf("FPGA Timer [ms]: %lu \r\n", tester.timer_read_ms()); // FPGA Count-Down-Timer Example - perform delay tester.timer_set_mode(MbedTester::TimerModeCountDownTimer); tester.timer_set_delay_ms(10); tester.timer_reset(); Timer timer; timer.start(); tester.timer_delay(); timer.stop(); printf("Mbed Timer [us] : %i \r\n", timer.read_us()); printf("Mbed Timer [ms] : %i \r\n", timer.read_ms()); tester.timer_free(); } ```

…er instead Time Drift model.

…instead Time Drift model.

ciarmcom · 2019-08-01T11:00:35Z

@mprse, thank you for your changes.
@fkjagodzinski @maciejbocianski @jamesbeyond @c1728p9 @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

fkjagodzinski

Looks good. 👍 Just a couple of minor flaws to resolve.

fkjagodzinski · 2019-09-02T13:13:43Z

components/testing/COMPONENT_FPGA_CI_TEST_SHIELD/MbedTester.h

+    /*
+     * These are the peripherals internal to the FPGA. A peripheral can be
+     * selected by calling MbedTester::select_peripheral.
+     */


This comment seems out of place.

fkjagodzinski · 2019-09-06T14:42:12Z

TESTS/mbed_hal_fpga_ci_test_shield/basic/main.cpp

@@ -0,0 +1,129 @@
+/*


The current location of this file (TESTS/mbed_hal_fpga_ci_test_shield/basic/main.cpp) is a bit misleading. I think this directory should be renamed to something like rtos-basic.

fkjagodzinski · 2019-09-06T14:42:48Z

TESTS/mbed_hal_fpga_ci_test_shield/basic/main.cpp

@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2013-2017, ARM Limited, All Rights Reserved


Suggested change

* Copyright (c) 2013-2017, ARM Limited, All Rights Reserved

* Copyright (c) 2013-2019, ARM Limited, All Rights Reserved

Update the date if modified. This also applies for the following files.

fkjagodzinski · 2019-09-06T14:45:37Z

TESTS/mbed_hal_fpga_ci_test_shield/basic/main.cpp

+    uint32_t start, stop;
+
+    tester.reset();
+


A lot of empty lines here.

fkjagodzinski · 2019-09-06T14:49:01Z

TESTS/mbed_hal_fpga_ci_test_shield/common_tickers_freq/main.cpp

+    tester.timer_init();
+
+    tester.peripherals_reset();
+


Empty lines.

fkjagodzinski · 2019-09-06T14:53:13Z

components/testing/COMPONENT_FPGA_CI_TEST_SHIELD/MbedTester.h

+
+    /**
+     * Execute programed FPGA Count Down Timer delay
+     *


I think we should explicitly state this is a blocking call.

fkjagodzinski · 2019-09-06T15:02:13Z

BTW, the example code is missing main(). 🤓

adbridge · 2019-09-27T13:58:38Z

@mprse what is the status of this PR ?

mprse · 2019-09-30T07:05:26Z

@mprse what is the status of this PR ?

All review comments have been addressed. @fkjagodzinski Thanks for the review.
PR is ready for further review/CI.

fkjagodzinski

This looks OK now. But still, I don't see the need to have that many empty lines here and here

jamesbeyond · 2019-10-02T10:30:19Z

Hi @mprse are these test will require we having a new release of FPGA images? currently we are using
https://github.com/ARMmbed/fpga-ci-test-shield/releases/tag/v0001

mprse · 2019-10-02T10:51:28Z

Yes. The FPGA test shield firmware will need to be updated to run these test.

0xc0170 · 2019-10-18T11:48:23Z

The following PR adds Verilog FPGA Timer implementation: ARMmbed/fpga-ci-test-shield#1 (must be merged before this one).

We should make sure that one is in first. I've left a comment there to get it moving again.

0xc0170 · 2019-10-18T11:50:02Z

TESTS/mbed_timing_fpga_ci_test_shield/common_tickers_freq/ticker_api_test_freq.h

@@ -0,0 +1,47 @@
+/* mbed Microcontroller Library
+ * Copyright (c) 2017-2017 ARM Limited
+ *


please add SPDX identifiers to new files

0xc0170 · 2019-10-23T13:42:01Z

Two things to do: fix one style error and SPDX identifier for new files

mprse · 2019-10-25T06:51:55Z

Fixed style and added SPDX header. It looks like its ready.
@0xc0170 I will close this one since it requires updated firmware for the FPGA-test-shield. Next year we are planning to add I2C Slave tester and FPGA timer support for the test shield. When both features are ready and new firmware is available I will reopen this PR.

0xc0170 · 2019-10-25T11:54:38Z

Closing as requested

mprse added 3 commits August 1, 2019 09:05

Make a copy of tests-mbed_hal-common_tickers_freq which uses FPGA tim…

e98819b

…er instead Time Drift model.

Make a copy of tests-mbedmicro-rtos-mbed-basic which uses FPGA timer …

dd8d8f4

…instead Time Drift model.

ciarmcom requested review from a team, c1728p9, fkjagodzinski, jamesbeyond and maciejbocianski August 1, 2019 11:00

ciarmcom added the needs: review label Aug 1, 2019

fkjagodzinski suggested changes Sep 6, 2019

View reviewed changes

Addressed all review comments

88d5555

fkjagodzinski approved these changes Oct 1, 2019

View reviewed changes

0xc0170 added the needs: preceding PR label Oct 18, 2019

0xc0170 approved these changes Oct 18, 2019

View reviewed changes

mprse mentioned this pull request Oct 21, 2019

Add FPGA Timer/CountDownTimer feature ARMmbed/fpga-ci-test-shield#1

Merged

FPGA Timer: Add setting to measure only single pulse

1307bcc

0xc0170 removed the needs: review label Oct 23, 2019

Fix style and add missing SPDX header

26e230c

0xc0170 approved these changes Oct 25, 2019

View reviewed changes

0xc0170 closed this Oct 25, 2019

		@@ -0,0 +1,129 @@
		/*
		* Copyright (c) 2013-2017, ARM Limited, All Rights Reserved

	* Copyright (c) 2013-2017, ARM Limited, All Rights Reserved
	* Copyright (c) 2013-2019, ARM Limited, All Rights Reserved

Add FPGA Timer feature and example test updates #11147

Add FPGA Timer feature and example test updates #11147

Uh oh!

Conversation

mprse commented Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Uh oh!

ciarmcom commented Aug 1, 2019

Uh oh!

fkjagodzinski left a comment

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski Sep 2, 2019

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski Sep 6, 2019

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski Sep 6, 2019

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski Sep 6, 2019

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski Sep 6, 2019

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski Sep 6, 2019

Choose a reason for hiding this comment

Uh oh!

fkjagodzinski commented Sep 6, 2019

Uh oh!

adbridge commented Sep 27, 2019

Uh oh!

mprse commented Sep 30, 2019

Uh oh!

fkjagodzinski left a comment

Choose a reason for hiding this comment

Uh oh!

jamesbeyond commented Oct 2, 2019

Uh oh!

mprse commented Oct 2, 2019

Uh oh!

0xc0170 commented Oct 18, 2019

Uh oh!

0xc0170 Oct 18, 2019

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Oct 23, 2019

Uh oh!

mprse commented Oct 25, 2019

Uh oh!

0xc0170 commented Oct 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mprse commented Aug 1, 2019 •

edited

Loading