i.MX EPIT Timer Driver #11441

stanislav-poboril · 2018-11-16T16:11:12Z

This adds EPIT driver and enables it on M4 core of i.MX 6SoloX SoC and on UDOO Neo Full board.
The driver conforms to the new counter API (#8340, #8331) and should be merged into topic-counters branch.

codecov-io · 2018-11-16T16:56:41Z

Codecov Report

Merging #11441 into topic-counters will decrease coverage by 1.13%.
The diff coverage is n/a.

@@                Coverage Diff                 @@
##           topic-counters   #11441      +/-   ##
==================================================
- Coverage           55.08%   53.94%   -1.14%     
==================================================
  Files                 255      242      -13     
  Lines               28923    27654    -1269     
  Branches             6975     6717     -258     
==================================================
- Hits                15931    14917    -1014     
+ Misses              10095     9932     -163     
+ Partials             2897     2805      -92

Impacted Files	Coverage Δ
lib/os/work_q.c
lib/os/mempool.c
lib/os/json.c
lib/os/fdtable.c
lib/os/crc16_sw.c
lib/os/rb.c
lib/os/crc7_sw.c
lib/os/crc8_sw.c
lib/os/base64.c
lib/os/printk.c
... and 3 more

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 6f5bf6a...d847144. Read the comment docs.

galak · 2018-11-16T16:58:01Z

drivers/counter/Kconfig.imx_epit

+
+if TIMER_IMX_EPIT_1
+
+config TIMER_IMX_EPIT_1_PRESCALER


Can we put the prescaler in DTS?

I've seen it set in Kconfig here in #8340, so I wanted to make it the same.
If the most of counter/timer drivers use dts, I can change it.

I think we should move the others to dts as well.

Moved the prescaler setting into DTS for this pr.

dts/arm/nxp/nxp_imx6sx_m4.dtsi

galak

I think EPIT2 is at wrong address in DTS:

devices/MCIMX6X/MCIMX6X_M4.h:#define EPIT1_BASE                               (0x420D0000u)
devices/MCIMX6X/MCIMX6X_M4.h:#define EPIT2_BASE                               (0x420D4000u)

nordic-krch · 2018-11-23T07:12:55Z

drivers/counter/timer_imx_epit.c

+	EPIT_Type *base = get_epit_config(dev)->base;
+	struct imx_epit_data *driver_data = dev->driver_data;
+
+	if ((EPIT_CR_REG(base) & EPIT_CR_EN_MASK) == 0U) {


it shoulld be possible to set wrap when counter is not started.

nordic-krch · 2018-11-23T07:18:06Z

drivers/counter/timer_imx_epit.c

+		/* (Re)enable EPIT Output Compare interrupt */
+		EPIT_SetIntCmd(base, true);
+	} else {
+		/* Reset reload value without affecting the actual count */


why callback=NULL resets wrap? There is no such statement in the API. Null callback mainly makes sense when channel alarms are implemented because it is establishing period.

Removed this.

MaureenHelm

Can you also add counter to the list of supported features in the board yaml and doc, and update tests/drivers/counter/counter_basic_api to run on this board?

MaureenHelm · 2018-11-28T02:43:23Z

drivers/counter/Kconfig.imx_epit

+# SPDX-License-Identifier: Apache-2.0
+#
+
+config TIMER_IMX_EPIT


Rename this to COUNTER_IMX_EPIT

MaureenHelm · 2018-11-28T02:44:00Z

drivers/counter/timer_imx_epit.c

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


Rename the file to counter_imx_epit.c

MaureenHelm · 2018-11-28T02:50:01Z

boards/arm/udoo_neo_full_m4/Kconfig.defconfig

+	default y
+
+config TIMER_IMX_EPIT_2
+	default n


Can remove these two lines

stanislav-poboril · 2018-12-03T11:22:30Z

Can you also add counter to the list of supported features in the board yaml and doc, and update tests/drivers/counter/counter_basic_api to run on this board?

@MaureenHelm @nordic-krch I've added two commits for the tests, one fixes the compilation errors existing on the topic-counter branch at the moment and the other adds support for the UDOO Neo board.
Note that the EPIT timer has just one compare register. As this is used to fire the wrap callback, there are zero channels available. I had to modify the test to look at the number of available channels first and skip some tests if needed.

How about the functionality when only one compare register is available? We have some options:

The EPIT implementation could stay as it is now - no channel alarms, wrap callback available.
Return error if counter_set_wrap() is called with non-null callback. Support one channel alarm.
After initialisation of the driver, counter_get_num_of_channels() would return 1. If counter_set_wrap()
is called with non-null callback, counter_get_num_of_channels() would return 0 and it will not be allowed to set a channel alarm. If counter_set_wrap() is called with null callback, counter_get_num_of_channels() would still return 1 and allow to set one channel alarm. But I think the counter_get_num_of_channels() is meant to be consistent and always number the same number of channels, although it is not stated in the API documentation explicitly.

dbkinder

+1 for doc changes

nordic-krch · 2018-12-18T11:23:05Z

@stanislav-poboril could you rebase?

regarding the approach when only one channel is available. There are 2 scenarios that can be covered with that:

periodic alarm
single-shot alarm.

For periodic alarm you need set_wrap to indicate that counter must be reset on wrap. For single shot you can use set_alarm or alternatively use set_wrap and disable counter in the callback to ensure single-shot functionality. Imo, to keep it simple i would stick with set_wrap only. The cost is that user must call counter_disable to avoid getting periodic callback.

stanislav-poboril · 2019-01-09T09:45:22Z

rebased

MaureenHelm · 2019-01-15T19:46:51Z

@nordic-krch Please revisit

Add shim driver for i.MX EPIT (Enhanced Periodic Interrupt Timer) peripheral which can be used for i.MX6SoloX, i.MX7D and other i.MX socs. Origin: Original Signed-off-by: Stanislav Poboril <[email protected]>

Add EPIT (Enhanced Periodic Interrupt Timer) peripheral support for i.MX6SoloX soc. Origin: Original Signed-off-by: Stanislav Poboril <[email protected]>

Enabled EPIT (Enhanced Periodic Interrupt Timer) in Cortex-M4 core of Udoo Neo Full board so the counter API could be used there. Origin: Original Signed-off-by: Stanislav Poboril <[email protected]>

Tests i.MX EPIT timer peripheral driver. Since it supports zero number of channel alarms, some tests had to be adjusted to look at the number of channel alarms supported first. Signed-off-by: Stanislav Poboril <[email protected]>

stanislav-poboril · 2019-01-24T06:48:24Z

Rebased and updated after the merge of #11572.

stanislav-poboril requested review from MaureenHelm, nordic-krch and anangl November 16, 2018 16:11

stanislav-poboril requested a review from galak as a code owner November 16, 2018 16:11

galak reviewed Nov 16, 2018

View reviewed changes

dts/arm/nxp/nxp_imx6sx_m4.dtsi Outdated Show resolved Hide resolved

galak requested changes Nov 16, 2018

View reviewed changes

galak approved these changes Nov 20, 2018

View reviewed changes

nordic-krch requested changes Nov 27, 2018

View reviewed changes

MaureenHelm added area: Counter area: API Changes to public APIs labels Nov 28, 2018

nordic-krch mentioned this pull request Nov 28, 2018

Unify counter.h and rtc.h interfaces #8331

Closed

7 tasks

MaureenHelm requested changes Nov 28, 2018

View reviewed changes

stanislav-poboril requested a review from dbkinder as a code owner December 3, 2018 11:01

nordic-krch force-pushed the topic-counters branch from 2e57cff to 215f872 Compare December 4, 2018 10:27

dbkinder approved these changes Dec 4, 2018

View reviewed changes

nashif force-pushed the topic-counters branch from 215f872 to 6b96890 Compare January 15, 2019 13:45

MaureenHelm approved these changes Jan 15, 2019

View reviewed changes

nordic-krch force-pushed the topic-counters branch 2 times, most recently from 2f603c6 to 72f7261 Compare January 21, 2019 08:19

stanislav-poboril added 4 commits January 24, 2019 07:35

imx: Add IMX EPIT driver for i.MX socs

5c6a70c

Add shim driver for i.MX EPIT (Enhanced Periodic Interrupt Timer) peripheral which can be used for i.MX6SoloX, i.MX7D and other i.MX socs. Origin: Original Signed-off-by: Stanislav Poboril <[email protected]>

imx: Add EPIT peripheral support for i.MX6SoloX soc

4bd7e3e

Add EPIT (Enhanced Periodic Interrupt Timer) peripheral support for i.MX6SoloX soc. Origin: Original Signed-off-by: Stanislav Poboril <[email protected]>

imx: Enabled EPIT in Udoo Neo Full board

e578089

Enabled EPIT (Enhanced Periodic Interrupt Timer) in Cortex-M4 core of Udoo Neo Full board so the counter API could be used there. Origin: Original Signed-off-by: Stanislav Poboril <[email protected]>

nordic-krch approved these changes Jan 24, 2019

View reviewed changes

nordic-krch merged commit 26b5136 into zephyrproject-rtos:topic-counters Jan 24, 2019

stanislav-poboril deleted the imx6sx_epit branch January 24, 2019 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

i.MX EPIT Timer Driver #11441

i.MX EPIT Timer Driver #11441

stanislav-poboril commented Nov 16, 2018

codecov-io commented Nov 16, 2018 •

edited

Loading

galak Nov 16, 2018

stanislav-poboril Nov 19, 2018

galak Nov 19, 2018

stanislav-poboril Nov 20, 2018

galak left a comment •

edited

Loading

nordic-krch Nov 23, 2018

stanislav-poboril Dec 3, 2018

nordic-krch Nov 23, 2018

stanislav-poboril Dec 3, 2018

MaureenHelm left a comment

MaureenHelm Nov 28, 2018

stanislav-poboril Dec 3, 2018

MaureenHelm Nov 28, 2018

stanislav-poboril Dec 3, 2018

MaureenHelm Nov 28, 2018

stanislav-poboril Dec 3, 2018

stanislav-poboril commented Dec 3, 2018

dbkinder left a comment

nordic-krch commented Dec 18, 2018

stanislav-poboril commented Jan 9, 2019

MaureenHelm commented Jan 15, 2019

stanislav-poboril commented Jan 24, 2019

i.MX EPIT Timer Driver #11441

i.MX EPIT Timer Driver #11441

Conversation

stanislav-poboril commented Nov 16, 2018

codecov-io commented Nov 16, 2018 • edited Loading

Codecov Report

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

galak left a comment • 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

Choose a reason for hiding this comment

MaureenHelm 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

stanislav-poboril commented Dec 3, 2018

dbkinder left a comment

Choose a reason for hiding this comment

nordic-krch commented Dec 18, 2018

stanislav-poboril commented Jan 9, 2019

MaureenHelm commented Jan 15, 2019

stanislav-poboril commented Jan 24, 2019

codecov-io commented Nov 16, 2018 •

edited

Loading

galak left a comment •

edited

Loading