Date   

RFC: extend sanitycheck testcase filtering expressiveness

Boie, Andrew P
 

Problem statement:

Test case filtering in sanitycheck's testcase.ini files allows
sanitycheck to exclude certain test cases that are incompatible with
particular architectures, boards, or the presence or absence of Kconfig
options.

The goal of this mechanism is to allow the testcases to scale up to many
different boards without having to do constant gardening of the testcase
configuration files. I imagine a future where Zephyr supports dozens, if
not hundreds of microcontrollers and we want to ensure that we have good
test coverage on all these boards.

This is currently done in testcase.ini with the following directives:

arch_whitelist = <list of arches, such as x86, arm, arc>
Set of architectures that this test case should only be run for.

arch_exclude = <list of arches, such as x86, arm, arc>
Set of architectures that this test case should not run on.

platform_whitelist = <list of platforms>
Set of platforms that this test case should only be run for.

platform_exclude = <list of platforms>
Set of platforms that this test case should not run on.

config_whitelist = <list of config options>
Config options can either be config names like CONFIG_FOO which
match if the configuration is defined to any value, or key/value
pairs like CONFIG_FOO=bar which match if it is set to a specific
value. May prepend a '!' to invert the match.

This current scheme is less than ideal for several reasons, enumerated
below:

1. config_whitelist is very limited in expressiveness.

1a. You cannot do numerical comparisons, for example I can't currently
filter out a large footprint test by saying that CONFIG_RAM_SIZE
must be equal to or greater than some value. You can only test for
equality or if the config option is defined. This was somewhat
vexing when trying to add the new nucleo_f103rb board to sanity
checks as it has very little RAM.

1b. All the items in config_whitelist are ANDed together. You cannot
specify boolean OR relationship, do any kind of grouping with
parenthesis, or use NOT except on individual items. You can only
do very simple stuff.

1c. There is no way to tie config_whitelist in with the current arch
or platform. For example, test_tickless is currently improperly
specified. The true semantics of the test is that it works on
all x86, is unimplemented on ARC, and it supports two different
SOC types on ARM: CONFIG_SOC_ATMEL_SAM3, CONFIG_SOC_FSL_FRDM_K64F

Currently we have in its testcase.ini:

[test]
tags = core
config_whitelist = !CONFIG_SOC_TI_LM3S6965_QEMU

This currently only works for a few reasons that do not scale.

* The test is automatically excluded on ARC because of no
microkernel support (which may change in the future)
* This test runs on all x86, because currently we exclude
targets that have a particular SOC, which is a bad way to
do it as you will need to keep adding more ARM SOCs to the
list as they are introduced to the tree, such as
SOC_STM32F103RB. If I instead expressed all the CONFIG_SOC_*
on the ARM side that the test *is* compatible with, there's
no way to tell it that it's also good to run on all X86.

What we should be saying is something more like, "run on all
x86 boards, or those ARM boards which have a particular SOC
supported in the code". In the language developed for this
proposal, we would have:

filter = ARCH == "x86" or (ARCH == "arm" and
(CONFIG_SOC_FSL_FRDM_K64F or CONFIG_SOC_ATMEL_SAM3))

2. platform_whitelist or platform_exclude is the *worst* way to filter
testcases, as it simply doesn't scale. You have to keep adding new
boards to these lists as they are introduced to the Zephyr tree, which
is not fun to manage in a hypothetical optimistic future where Zephyr
takes off and we have dozens or even hundreds of supported boards in the
tree.

If possible, it's much better to filter on board *features* through
config_whitelist, as these typically do not require more changes to the
testcase.ini as more boards are added.

For example, we have a lot of bluetooth tests which have a
platform_whitelist with "arduino_101". This means the test will only run
on that board. It may be better to indicate that the test can run on any
Quark SE board with compatible Bluetooth hardware; on cursory inspection
these tests seem to be specific to the bluetooth chip in use and not
even the SOC.



Proposed solution:

We need a more expressive language for filtering test cases. I propose a
simple boolean expression language with the following grammar:

expression ::= expression "and" expression
| expression "or" expression
| "not" expression
| "(" expression ")"
| symbol "==" constant
| symbol "!=" constant
| symbol "<" number
| symbol ">" number
| symbol ">=" number
| symbol "<=" number
| symbol "in" list
| symbol

list ::= "[" list_contents "]"

list_contents ::= constant
| list_contents "," constant

constant ::= number
| string

When symbols are encountered, they are looked up in an environment
dictionary. In sanitycheck the environment will initially consist of:

{
ARCH : <architecture>,
PLATFORM : <platform>,
<all CONFIG_* key/value pairs in the test's generated defconfig>
}

We can later augment this with additional interesting metadata if we
want. For example I was thinking of ways we could integrate some
footprint information for large tests.

For the case where

expression ::= symbol

it evaluates to true if the symbol is defined to a non-empty string.

For all comparison operators, if the config symbol is undefined, it will
be treated as a 0 (for > < >= <=) or an empty string "" (for == != in).
For numerical comparisons it doesn't matter if the environment stores
the value as an integer or string, it will be cast appropriately.

Operator precedence, starting from lowest to highest:

or (left associative)
and (left associative)
not (right associative)
all comparison operators (non-associative)

In testcase.ini the 'config_whitelist' directive will be removed and
replaced with 'filter' directives containing one of these expressions.
arch_whitelist, arch_exclude, platform_whitelist, platform_exclude
are all syntactic sugar for these expressions. For instance

arch_exclude = x86 arc

Is the same as:

filter = not ARCH in ["x86", "arc"]


Implementation details:

Writing a parser by hand is a pain and prone to bugs. The best way to do
a language like this is to use a LR parser generator like lex/yacc.
There exists an open source library called PLY which does lex/yacc for
Python, it has been around for a long time and works very well:

http://www.dabeaz.com/ply/index.html

The sample implementation I have drops a copy of PLY directly in the
Zephyr source tree since its license is compatible and this will result
in the least pain for users as they won't have to do anything. If this
is unacceptable we can either find a way to stick it in the SDK, or add
it to the list of workstation dependencies (either have the user install
with pip or their distribution's package manager. Fedora appears to have
PLY installed by default).

Except for the case mentioned above with test_tickless, I haven't yet
gone through all the testcase.ini to ensure that the filtering done is
optimal. However I do have an implementation of the language which can
be looked at below:

Add PLY sources to tree
https://gerrit.zephyrproject.org/r/1075

Implement expression parser
https://gerrit.zephyrproject.org/r/1076

Integrate expression parser into sanitycheck
https://gerrit.zephyrproject.org/r/1077

Fix test_tickless filtering expression
https://gerrit.zephyrproject.org/r/1078

In addition to sanitycheck it is hoped that the expression language
could be generally useful for other scripts which need to do yes/no
filtering based on values in an environment.

--
Andrew Boie
Staff Engineer - EOS Zephyr
Intel Open Source Technology Center


Re: FRDM-K64 PWM BUS Fault

Maciek Borzecki <maciek.borzecki@...>
 

On Wed, Mar 23, 2016 at 4:01 PM, Anders Dam Kofoed <adk(a)accipio.dk> wrote:
Hi all,

I am trying to use the PWM output on the K64 board. I have taken the sample/drivers/pwm_dw/src/main.c and modified it to use the CONFIG_PWM_K64_FTM_0_DEV_NAME and removed the CONFIG_PWM_DW=y from the prj.conf. Compiles without errors or warnings. Using the latest 0.75 SDK and zephyr-project code I get this on the terminal when running it:

PWM demo app
***** BUS FAULT *****
Executing thread ID (thread): 0x20001b28
Faulting instruction address: 0x00000b78
Imprecise data bus error
Fatal fault in task ! Aborting task.
Full assembly listing in written to outdir/zephyr.lst. Having the
address of instruction that trigger the fault, try looking it up in
the listing and see if there's something obviously wrong. The assembly
will be mixed with C, you can try a debug build to get an unoptimized
version which should be a bit easier to look at . If nothing in
particular stands out, you can always try gdb.

---
prj.conf:
---
CONFIG_STDOUT_CONSOLE=y
CONFIG_PRINTK=y
CONFIG_NANO_TIMERS=y
CONFIG_NANO_TIMEOUTS=y
CONFIG_GPIO=y
CONFIG_PWM=y
---

CODE:
---
#include <zephyr.h>

#if defined(CONFIG_STDOUT_CONSOLE)
#include <stdio.h>
#define PRINT printf
#else
#include <misc/printk.h>
#define PRINT printk
#endif

#include <device.h>
#include <pwm.h>
#include <sys_clock.h>

/* about 1 ms */
#define MIN_PERIOD 32000

/* about 1 second */
#define MAX_PERIOD 32000000

#define SLEEPTICKS SECONDS(4)

void main(void)
{
struct nano_timer timer;
void *timer_data[1];
struct device *pwm_dev;
uint32_t period;
uint8_t dir;

nano_timer_init(&timer, timer_data);

PRINT("PWM demo app\n");

pwm_dev = device_get_binding(CONFIG_PWM_K64_FTM_0_DEV_NAME);
if (!pwm_dev) {
PRINT("Cannot find %s!\n", CONFIG_PWM_K64_FTM_0_DEV_NAME);
}

period = MAX_PERIOD;
dir = 0;

while (1) {
pwm_pin_set_values(pwm_dev, 0, period, period);
//pwm_pin_set_duty_cycle(pwm_dev, 0, 40);

if (dir) {
period *= 2;

if (period > MAX_PERIOD) {
dir = 0;
period = MAX_PERIOD;
}
} else {
period /= 2;

if (period < MIN_PERIOD) {
dir = 1;
period = MIN_PERIOD;
}
}

nano_timer_start(&timer, SLEEPTICKS);
nano_timer_test(&timer, TICKS_UNLIMITED);
}
}

I have found another way to do what I want so no rush. Just wanted to state that it's not working.

Kind regards
Anders Dam Kofoed


--
Maciek Borzecki


Re: FRDM-K64 PWM BUS Fault

Kalowsky, Daniel <daniel.kalowsky@...>
 

I'd encourage you to file a JIRA/bug so that we can track this and get someone looking at it.

-----Original Message-----
From: Anders Dam Kofoed [mailto:adk(a)accipio.dk]
Sent: Wednesday, March 23, 2016 8:01 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] FRDM-K64 PWM BUS Fault

Hi all,

I am trying to use the PWM output on the K64 board. I have taken the
sample/drivers/pwm_dw/src/main.c and modified it to use the
CONFIG_PWM_K64_FTM_0_DEV_NAME and removed the
CONFIG_PWM_DW=y from the prj.conf. Compiles without errors or
warnings. Using the latest 0.75 SDK and zephyr-project code I get this on the
terminal when running it:

PWM demo app
***** BUS FAULT *****
Executing thread ID (thread): 0x20001b28
Faulting instruction address: 0x00000b78
Imprecise data bus error
Fatal fault in task ! Aborting task.

---
prj.conf:
---
CONFIG_STDOUT_CONSOLE=y
CONFIG_PRINTK=y
CONFIG_NANO_TIMERS=y
CONFIG_NANO_TIMEOUTS=y
CONFIG_GPIO=y
CONFIG_PWM=y
---

CODE:
---
#include <zephyr.h>

#if defined(CONFIG_STDOUT_CONSOLE)
#include <stdio.h>
#define PRINT printf
#else
#include <misc/printk.h>
#define PRINT printk
#endif

#include <device.h>
#include <pwm.h>
#include <sys_clock.h>

/* about 1 ms */
#define MIN_PERIOD 32000

/* about 1 second */
#define MAX_PERIOD 32000000

#define SLEEPTICKS SECONDS(4)

void main(void)
{
struct nano_timer timer;
void *timer_data[1];
struct device *pwm_dev;
uint32_t period;
uint8_t dir;

nano_timer_init(&timer, timer_data);

PRINT("PWM demo app\n");

pwm_dev =
device_get_binding(CONFIG_PWM_K64_FTM_0_DEV_NAME);
if (!pwm_dev) {
PRINT("Cannot find %s!\n",
CONFIG_PWM_K64_FTM_0_DEV_NAME);
}

period = MAX_PERIOD;
dir = 0;

while (1) {
pwm_pin_set_values(pwm_dev, 0, period, period);
//pwm_pin_set_duty_cycle(pwm_dev, 0, 40);

if (dir) {
period *= 2;

if (period > MAX_PERIOD) {
dir = 0;
period = MAX_PERIOD;
}
} else {
period /= 2;

if (period < MIN_PERIOD) {
dir = 1;
period = MIN_PERIOD;
}
}

nano_timer_start(&timer, SLEEPTICKS);
nano_timer_test(&timer, TICKS_UNLIMITED);
}
}

I have found another way to do what I want so no rush. Just wanted to state
that it's not working.

Kind regards
Anders Dam Kofoed


FRDM-K64 PWM BUS Fault

Anders Dam Kofoed <adk@...>
 

Hi all,

I am trying to use the PWM output on the K64 board. I have taken the sample/drivers/pwm_dw/src/main.c and modified it to use the CONFIG_PWM_K64_FTM_0_DEV_NAME and removed the CONFIG_PWM_DW=y from the prj.conf. Compiles without errors or warnings. Using the latest 0.75 SDK and zephyr-project code I get this on the terminal when running it:

PWM demo app
***** BUS FAULT *****
Executing thread ID (thread): 0x20001b28
Faulting instruction address: 0x00000b78
Imprecise data bus error
Fatal fault in task ! Aborting task.

---
prj.conf:
---
CONFIG_STDOUT_CONSOLE=y
CONFIG_PRINTK=y
CONFIG_NANO_TIMERS=y
CONFIG_NANO_TIMEOUTS=y
CONFIG_GPIO=y
CONFIG_PWM=y
---

CODE:
---
#include <zephyr.h>

#if defined(CONFIG_STDOUT_CONSOLE)
#include <stdio.h>
#define PRINT printf
#else
#include <misc/printk.h>
#define PRINT printk
#endif

#include <device.h>
#include <pwm.h>
#include <sys_clock.h>

/* about 1 ms */
#define MIN_PERIOD 32000

/* about 1 second */
#define MAX_PERIOD 32000000

#define SLEEPTICKS SECONDS(4)

void main(void)
{
struct nano_timer timer;
void *timer_data[1];
struct device *pwm_dev;
uint32_t period;
uint8_t dir;

nano_timer_init(&timer, timer_data);

PRINT("PWM demo app\n");

pwm_dev = device_get_binding(CONFIG_PWM_K64_FTM_0_DEV_NAME);
if (!pwm_dev) {
PRINT("Cannot find %s!\n", CONFIG_PWM_K64_FTM_0_DEV_NAME);
}

period = MAX_PERIOD;
dir = 0;

while (1) {
pwm_pin_set_values(pwm_dev, 0, period, period);
//pwm_pin_set_duty_cycle(pwm_dev, 0, 40);

if (dir) {
period *= 2;

if (period > MAX_PERIOD) {
dir = 0;
period = MAX_PERIOD;
}
} else {
period /= 2;

if (period < MIN_PERIOD) {
dir = 1;
period = MIN_PERIOD;
}
}

nano_timer_start(&timer, SLEEPTICKS);
nano_timer_test(&timer, TICKS_UNLIMITED);
}
}

I have found another way to do what I want so no rush. Just wanted to state that it's not working.

Kind regards
Anders Dam Kofoed


Re: RFC: Use error codes from errno.h

Andre Guedes <andre.guedes@...>
 

Hi Daniel, thanks for your feedback!

See some comments inline:

Quoting Dan Kalowsky (2016-03-21 14:10:43)
Hi, trying the webmail reply for a first time. Be gentle.

Hi all,

Quoting Andre Guedes (2016-03-03 16:43:24)

Do we have a consensus about this?

After the "errno-drivers" patchset, DEV_* codes will be used only at
'board'
and 'arch' layers. Actually, all occurrences of DEV_* codes in board/ are
from pinmux drivers. These drivers are going to be landed in drivers/ soon
(patches are under review on gerrit) so they will already be replaced by -E*
codes. This means that only a few files from arch/ will be using DEV_* codes.

If we have a consensus, the next steps are:
1) Replace DEV_* occurrences in arch/ (and boards/ if applicable);
+1
Patch sent to gerrit.


2) Deprecate DEV_* (just add a comment in device.h saying: DEV_* are deprecate,
use errno.h codes instead.
+1 I would argue this should step 1. This can be and should be merged in before the close of the release window this week. So get this done ASAP.
Fine by me. Patch sent to gerrit. There is no dependency so it can be merged
before any other patch.


3) Remove DEV_* codes.

Since 3) might break external applications, we should apply it during a major
release I guess.
I'd wait until the start of the next release cycle to make this change. It will give a long window of opportunity to update.
Fine by me too. I also sent a patch removing the DEV_* codes to gerrit, but
we can wait until the next merge window is open to apply it.

BTW, while we don't remove DEV_* codes, we have to be extra careful to avoid
new code is merged using DEV_* instead of -E*. I just noticed this occurred
with aon counters and gpio_stm32 drivers. Patches fixing this on gerrit.

Regards,

Andre


RFC: 2/5 System Device Driver Modifications (Revised v1.2)

Thomas, Ramesh
 

[Rev 1.2 - Added a parameter to .suspend() and .resume() functions
indicating the power policy used by the PMA. This parameter will help
driver take policy based optimized actions. Described in detail at the
end]

[Revised v1.1 incorporating feedbacks. The changes are shown at the
end]

Problem Statement:
Not all Zephyr kernel drivers provide the same interfaces.

Why this is a problem:
-----------------------------
The Zephyr kernel currently has devices that are essential for core OS
functions (such as APICs and timers), but provide no public API means
for accessing them. Without any consistent method to access device
drivers on the platform, it becomes very difficult to achieve power
targets.

What should be done:
-----------------------------
1) Each native Zephyr kernel driver and shim driver (such as QMSI) shall
support:

a) uint32_t suspend() - routine that will move any required device state
data
to RAM* with the following valid return values:
- DEV_OK
- DEV_BUSY
- DEV_FAIL

b) uint32_t resume() - routine that will retrieve any device state data
from
RAM* with the following valid return values:

- DEV_OK
- DEV_FAIL

2) Provide a name recognized by the device_get_binding() function, this
includes what are currently thought to be drivers such as timers,
IOAPIC, and LOAPIC.

*The power management process is not expected to power down system RAM
(it will most likely stay in selective suspend).

The size of the device data is dependent upon an individual device, and
therefore the system integrator must be wary of the number of devices
being utilized.

Device suspend flow:
Device Drivers at suspend shall:
- Device state shall be saved to memory and maintained across a PM event
- Turning off the device clock
- Return appropriate error code

Device resume flow:
Device Drivers at resume shall:
- Restore to state saved in memory
- Turn on the device clock
- Return appropriate error code

The drivers may skip the saving/restoring and device re-initialization
steps, if the power policy used by the PMA is not expected to cause the
devices to lose state.

Device domain experts will be needed to implement the proper methods for
suspending and resuming each device.

----New additions-------
Following 2 macros would be added to allow device drivers to implement
the .suspend() and .resume() hooks described in #1 above.

A device ops structure holds the .suspend() and .resume() which is
included inside each device's device structure:

struct device_ops example_dev_ops = {
.suspend = example_suspend,
.resume = example_resume
};

Two new macros would be created that drivers use instead of DEVICE_INIT
and SYS_INIT if they implement the .suspend and .resume functions. For
devices using non-PM macros, a default dev_ops will be assigned that
does nothing (so app need not check if dev_ops != NULL).

a) DEVICE_INIT_PM(dev_name, drv_name, init_fn, &example_dev_ops, data,
cfg_info, level, prio);

b) SYS_INIT_PM(name, init_fn, &example_dev_ops, level, prio);

SYS_INIT_PM also added the "name" argument so PMA can call
the .suspend/.resume for devices like APICs that need to do
suspend/resume operations. The methods that retrieve the pointer to the
device structure of any device requires the device to be assigned a name
unlike the SYS_INIT macro which assigns a "" as the name for devices.

All this will be inside DEVICE_POWER_MANAGEMENT Kconfig flag.

----Rev 1.2 changes-------
The .suspend and .resume functions described in #1 would be passed a
parameter indicating the policy used by the PMA. Depending on the
policy, the driver may take different actions during .suspend
and .resume calls. This is because, unless the policy causes the
devices to lose state, the device driver need not save/restore state.

This helps reduce wake latency in states that do not need save/restore
and reinitialization of devices.

The policies described in RFC 5/5 in detail are listed below:
1. PM_SOC_LPS - Devices do not lose state
2. PM_SOC_DS - Devices may lose state
3. PM_SOC_DEVICE_SUSPEND_ONLY - Devices do not lose state


Re: RFC: Use error codes from errno.h

Kalowsky, Daniel <daniel.kalowsky@...>
 

Hi, trying the webmail reply for a first time. Be gentle.

Hi all,

Quoting Andre Guedes (2016-03-03 16:43:24)

Do we have a consensus about this?

After the "errno-drivers" patchset, DEV_* codes will be used only at
'board'
and 'arch' layers. Actually, all occurrences of DEV_* codes in board/ are
from pinmux drivers. These drivers are going to be landed in drivers/ soon
(patches are under review on gerrit) so they will already be replaced by -E*
codes. This means that only a few files from arch/ will be using DEV_* codes.

If we have a consensus, the next steps are:
1) Replace DEV_* occurrences in arch/ (and boards/ if applicable);
+1

2) Deprecate DEV_* (just add a comment in device.h saying: DEV_* are deprecate,
use errno.h codes instead.
+1 I would argue this should step 1. This can be and should be merged in before the close of the release window this week. So get this done ASAP.

3) Remove DEV_* codes.

Since 3) might break external applications, we should apply it during a major
release I guess.
I'd wait until the start of the next release cycle to make this change. It will give a long window of opportunity to update.


[PATCH 3/3] arc_timer: assert that counter always lower than limit

Desfarges, Simon <simon.desfarges@...>
 

From: Simon Desfarges <simon.desfarges(a)intel.com>

ASSERT are put each time the timer0 limit register or the timer0 count register
is modified.

Change-Id: I38684d57803de285f4e26c68b449c71396e4c750
Tracked-On: https://jira.ndg.intel.com/browse/FIRE-4213
Signed-off-by: Simon Desfarges <simon.desfarges(a)intel.com>
---
drivers/timer/arcv2_timer0.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/timer/arcv2_timer0.c b/drivers/timer/arcv2_timer0.c
index ca25fea..db353de 100644
--- a/drivers/timer/arcv2_timer0.c
+++ b/drivers/timer/arcv2_timer0.c
@@ -60,6 +60,7 @@
#include <sys_clock.h>
#include <drivers/system_timer.h>
#include <stdbool.h>
+#include <misc/__assert.h>

/*
* A board support package's board.h header must provide definitions for the
@@ -173,6 +174,10 @@ void _timer_int_handler(void *unused)

#if defined(CONFIG_TICKLESS_IDLE)
timer0_limit_register_set(cycles_per_tick - 1);
+ __ASSERT_EVAL({},
+ uint32_t timer_count = timer0_count_register_get(),
+ timer_count <= (cycles_per_tick - 1),
+ "timer_count: %d, limit %d\n", timer_count, cycles_per_tick - 1);

_sys_idle_elapsed_ticks = 1;
#endif
@@ -233,6 +238,10 @@ void _timer_idle_enter(int32_t ticks)
if (status & _ARC_V2_TMR_CTRL_IP) {
straddled_tick_on_idle_enter = true;
}
+ __ASSERT_EVAL({},
+ uint32_t timer_count = timer0_count_register_get(),
+ timer_count <= programmed_limit,
+ "timer_count: %d, limit %d\n", timer_count, programmed_limit);
}

/*
@@ -250,6 +259,10 @@ void _timer_idle_exit(void)
if (straddled_tick_on_idle_enter) {
/* Aborting the tickless idle due to a straddled tick. */
straddled_tick_on_idle_enter = false;
+ __ASSERT_EVAL({},
+ uint32_t timer_count = timer0_count_register_get(),
+ timer_count <= programmed_limit,
+ "timer_count: %d, limit %d\n", timer_count, programmed_limit);
return;
}

@@ -269,6 +282,10 @@ void _timer_idle_exit(void)
update_accumulated_count();
_sys_clock_tick_announce();

+ __ASSERT_EVAL({},
+ uint32_t timer_count = timer0_count_register_get(),
+ timer_count <= programmed_limit,
+ "timer_count: %d, limit %d\n", timer_count, programmed_limit);
return;
}

@@ -288,6 +305,11 @@ void _timer_idle_exit(void)
*/
timer0_limit_register_set(cycles_per_tick - 1);
timer0_count_register_set(current_count % cycles_per_tick);
+
+ __ASSERT_EVAL({},
+ uint32_t timer_count = timer0_count_register_get(),
+ timer_count <= (cycles_per_tick - 1),
+ "timer_count: %d, limit %d\n", timer_count, cycles_per_tick-1);
}
#else
static void tickless_idle_init(void) {}
--
1.9.1


[PATCH 2/3] arc_timer: fix tickless idle

Desfarges, Simon <simon.desfarges@...>
 

From: Simon Desfarges <simon.desfarges(a)intel.com>

When exiting from tickless idle uppon an external IRQ, the TICK timer
is set to fire at next TICK boundary. The current algorithm can lead
to a point that timer0_count register is higher than the timer0_limit
register.

In this situation the next TICK will fire after the counter has
wrapped and performed another cycle (~133 seconds).

This condition appears when the counter reaches the limit after the
Interrupt Pending flag is checked. At this point the counter is
automatically wrapped to 0, but is set just next to the limit to fire
at next TICK boundary by SW. At exit of the _timer_idle_exit function,
the timer handler is called, and sets the limit to 1 TICK. At this
point the situation is:
- limit register == 1 TICK
- count register is just below the old limit register and higher than
1 TICK

To fix this issue, at _timer_idle_exit, the limit register is always
set to 1 TICK and the count register set such as the next TICK fires
on time.

Change-Id: Ifa002809d426aa04109592e53d2b02a224f51101
Tracked-On: https://jira.ndg.intel.com/browse/FIRE-4213
Signed-off-by: Simon Desfarges <simon.desfarges(a)intel.com>
---
drivers/timer/arcv2_timer0.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/timer/arcv2_timer0.c b/drivers/timer/arcv2_timer0.c
index 46c4612..ca25fea 100644
--- a/drivers/timer/arcv2_timer0.c
+++ b/drivers/timer/arcv2_timer0.c
@@ -276,9 +276,6 @@ void _timer_idle_exit(void)
* A non-timer interrupt occurred. Announce any
* ticks that have elapsed during the tickless idle.
*/
-
- uint32_t remaining_cycles = programmed_limit - current_count;
-
_sys_idle_elapsed_ticks = current_count / cycles_per_tick;
if (_sys_idle_elapsed_ticks > 0) {
update_accumulated_count();
@@ -289,11 +286,8 @@ void _timer_idle_exit(void)
* Ensure the timer will expire at the end of the next tick in case
* the ISR makes any tasks and/or fibers ready to run.
*/
-
- if (remaining_cycles >= cycles_per_tick) {
- timer0_count_register_set(programmed_limit -
- ((remaining_cycles - 1) % cycles_per_tick) - 1);
- }
+ timer0_limit_register_set(cycles_per_tick - 1);
+ timer0_count_register_set(current_count % cycles_per_tick);
}
#else
static void tickless_idle_init(void) {}
--
1.9.1


[PATCH 1/3] arc_timer: fix wrong programmed limit when entering idle

Desfarges, Simon <simon.desfarges@...>
 

From: Simon Desfarges <simon.desfarges(a)intel.com>

The timer counts from 0 to programmed_limit included.

Change-Id: Ifc8585210c319f5452fafc911d4f6d72c4b91eaa
Tracked-On: https://jira.ndg.intel.com/browse/FIRE-4213
Signed-off-by: Simon Desfarges <simon.desfarges(a)intel.com>
---
drivers/timer/arcv2_timer0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/timer/arcv2_timer0.c b/drivers/timer/arcv2_timer0.c
index 932ceac..46c4612 100644
--- a/drivers/timer/arcv2_timer0.c
+++ b/drivers/timer/arcv2_timer0.c
@@ -220,7 +220,7 @@ void _timer_idle_enter(int32_t ticks)
}

programmed_ticks = ticks;
- programmed_limit = (programmed_ticks * cycles_per_tick);
+ programmed_limit = (programmed_ticks * cycles_per_tick) - 1;

timer0_limit_register_set(programmed_limit);

--
1.9.1


Re: FRDM-K64 GPIO driver name

Idupsle <idupsle@...>
 

diff --git a/drivers/gpio/gpio_k64.c b/drivers/gpio/gpio_k64.c
index 2d6a355..011eb52 100644
--- a/drivers/gpio/gpio_k64.c
+++ b/drivers/gpio/gpio_k64.c
@@ -111,6 +111,9 @@ static int gpio_k64_config(struct device *dev, int
access_op,
}
}

+ /* Ensure pin is configured as GPIO */
+ setting |= K64_PINMUX_FUNC_GPIO;
+
/* write pull-up/-down and, if set, interrupt configuration
settings */

if (access_op == GPIO_ACCESS_BY_PIN) {
Hi, quite well, with your patch, I got the output! Now I'm wondering how I can
manually set the pins to FUNC_GPIO = K64_PINMUX_ALT_1 = (0x1 << 8).
If I write something like

#include <device.h>
#include <gpio.h>
#include <sys_io.h>
#include <pinmux.h>
#include <zephyr.h>

#define GPIO_1 CONFIG_PINMUX_K64_GPIO_B_NAME
void main(void)
{
struct device *gpio_dev_1;
gpio_dev_1 = device_get_binding(GPIO_1);
uint32_t set = (0x1<<8);
pinmux_pin_set(gpio_dev_1,22, set);

int set_res = 0;
int* set_res_ptr = 0;
pinmux_pin_get(gpio_dev_1, 22, set_res_ptr);
printk("Pin 22 is set to: %u \n",set_res);

while(1){}
}

Nothing happens. Debugging tells me, that the command
pinmux_pin_set(gpio_dev_1,22, set)
is not executed at all. It should call:
static int _fsl_k64_set_pin(struct device *dev, uint32_t pin_id,
uint32_t func)
but doesn't.
pinmux_pin_get() Executes as excepted.
Any clue, why?


Re: FRDM-K64 PRINT not working

Anders Dam Kofoed <adk@...>
 

Got it working using minicom and no flowcontrol...


Re: FRDM-K64 GPIO driver name

Anders Dam Kofoed <adk@...>
 

Hi Maureen and Idups

I got UART output (printf) working using minicom. Don't know what the difference between screen and minicom is but using minicom I set it to not use flowcontrol 115200 8N1 and it worked...

## GPIO
Also, GPIO interrupt is working just fine. However, as @Maureen writes above, the PCR has not setup PTB22 as default. The pins are configured in an arduino style configuration as default:
zephyr-project/arch/arm/soc/fsl_frdm_k64f/soc_config.c

So using these pins as input or output GPIO works just fine.

Thanks for your inputs.
Kind regards
Anders


Re: RFC: 5/5 Provide interfaces for Power Management Applications Policies (Revised)

Thomas, Ramesh
 

(Revised and incorporated feedbacks. The term "Tickless Idle" was found
confusing to be used to refer to a PM policy. That is a specific term
used for an optimized kernel idling mechanism saving power and is
independent of the infrastructure described in this RFC. The particular
power policy that this RFC is referring to is renamed to
DEVICE_SUSPEND_ONLY. In this policy, the PMA only suspends devices and
does not do any CPU or SOC PM operations and lets kernel do its normal
idle processing)

Problem Statement:
Add OS infrastructure to enable application-based power management
policies, which is architecture independent, supports microkernel and
nanokernel and the interface clearly identifies the policy based action
taken.

Why is this a problem:
-----------------------------
Currently the kernel has hooks for a power management application to be
notified of a suspend (_sys_soc_suspend) and resume (_sys_soc_resume)
operation that only operate within the microkernel and only on X86
architectures.

This creates an inconsistent interface for all architectures to expand
and easily implement a power management application across multiple
architectures. This also creates a requirement for using the
microkernel only, while some of our supported platforms work with only a
nanokernel (i.e. Quark D2000).

During Deep Sleep, devices lose power and state with no consistent
methods to allow a PMA to enforce devices to save and restore states.
This stops core functions of the Application that depend on devices
being powered on, configured, and ready to operate.

What should be done:
-----------------------------
The Zephyr kernel is not looking to implement power management policies.
Instead, the kernel shall provide interfaces for a system integrator to
utilize and create their own Power Management Application (PMA) that can
enforce any policies needed.

This will be accomplished by providing an OS power-management
infrastructure that has an architecture independent interface. The
Zephyr kernel will provide notification methods in both the Microkernel
and Nanokernel for the PMA when the OS is about to enter and exit system
idle. Currently the Zephyr kernel has support for this notification
only on X86 through the _sys_soc_suspend() and _sys_soc_resume()
functions. Expanding the scope of these functions to include the ARM
and ARC architectures, support both the Nanokernel and Microkernel, and
provide a more detailed return codes would be the first steps. The
kernel will continue to provide the determination on how much time to
spend idle (number of ticks), passing this information along to the PMA.

When kernel is about to go idle, the kernel itself will disable
interrupts. The kernel will then call the _sys_soc_suspend() function,
notifying the PMA of the operation. Included in the notification are
the maximum number of ticks that the system can be set idle for. The
PMA will then determine what policies can be executed within the
allotted time frame.

Currently the kernel expects the _sys_soc_suspend() to return one of
the following values:
- PM_NOT_HANDLED - The PMA did not put the CPU to LPS or the SOC to
Deep Sleep.

- PM_HANDLED - The PMA was able to put the CPU to LPS or the SOC to Deep
Sleep.

The proposal is to replace _sys_soc_suspend() return values to provide a
clear action based indicator of the policies the PMA implements:
- PM_SOC_LPS - Indicating that the PMA was successful at pushing the CPU
into a low power state.

- PM_SOC_DS - Indicating that the PMA was successful at pushing the SOC
into the Deep Sleep state

- PM_SOC_DEVICE_SUSPEND_ONLY - Indicating that the PMA has accomplished
any device suspend operations. This does not include any CPU or SOC
power operations.

- PM_SOC_NOT_HANDLED - Indicating the PMA was not able to accomplish any
action in the time allotted by the kernel.

As policy decisions are the realm of the PMA, the kernel will now
provide a method for the PMA to get the current list of devices enabled
for the application. This will allow the PMA to walk the device list
and determine any policy decisions based upon the available tick count,
call the device’s suspend() routine, and deal with any possible failures
in the process.

All of these operations have any expressed latency requirement of NONE.

Writing a PMA:
---------------------
Writing a Power Management Application to enforce policies will
implement the API described below.

Upon startup, the PMA will already be registered as the handler for
_sys_soc_suspend() and _sys_soc_resume() through compile time linking.
The first act of the PMA will be to retrieve the known list of devices
through the device_list_get() function. Because the PMA is part of the
application, it is expected to start after all system devices have been
initialized. Thus the list of devices is not expected to change once
the application has begun.

The device_list_get() function will return the start and end pointers to
the current enabled devices. It is up to the PMA to walk this list and
how to determine the best mechanism to store/process this list. It is
up to the system integrator to verify the amount of time each device
requires for a power cycle, and ensure this time fits within the
allotted time provided by the kernel. This time value is highly
dependent upon each specific device used in the final platform and SOC.

When entering the PMA through the _sys_soc_suspend() function, the PMA
can select between multiple scenarios.

Case 1:
*The time allotted is too short for any power management.*

In this case, the PMA will leave interrupts disabled, and return the
code
PM_SOC_NOT_HANDLED. This will allow the Zephyr kernel to continue on
the idle loop selected at compile time.

Case 2:
*The time allotted is enough for some devices to be suspended.*

a) If everything suspends correctly, the PMA will:
1) Scan through the devices that meet the criteria
2) Call each device’s suspend() function
3) If the time allotted is enough to put the CPU into a LPS the PMA
will:
i) Push the CPU to the LPS re-enabling interrupts at the same
time.
ii) Return PM_SOC_LPS

4) If the time allotted is not enough for CPU or SOC operations, the
PMA
will:
i) Return PM_SOC_DEVICE_SUSPEND_ONLY

b) If a device fails to suspend, the PMA will:
1) If the device is not essential to the suspend process, as
determined by
the system integrator, the PMA can choose to ignore the failure
2) If the device is essential to the suspend process, as determined
by the
system integrator, the PMA shall return PM_SOC_NOT_HANDLED.

Case 3:
*The time allotted is enough for all devices to be suspended.*

a) If everything suspends correctly, the PMA will:
1) Call each device’s suspend() function
2) If the time allotted is enough to put the CPU into a LPS the PMA
will:
i) Push the CPU to the LPS re-enabling interrupts at the same
time.
ii) Return PM_SOC_LPS
3) If the time allotted is enough to put the SOC into Deep Sleep, the
PMA
will:
i) Push the SOC to Deep Sleep
ii) Return PM_SOC_DS
iii) Re-enable interrupts
4) If the time allotted is not enough for CPU or SOC operations, the
PMA
will:
i) Return PM_SOC_DEVICE_SUSPEND_ONLY

b) If a device fails to suspend, the PMA will:
1) If the device is not essential to the suspend process, as
determined by
the system integrator, the PMA can choose to ignore the failure
2) If the device is essential to the suspend process, as determined
by the
system integrator, the PMA shall return PM_SOC_NOT_HANDLED.



PMA: Power Manager Application
ISR: Interrupt Service Routine
DSO: Device Suspend Only

Proposed PMA DSO Entry:
+---------+ +-----+ +---------+ +-----+
| Events | | ISR | | Kernel | | PMA |
+---------+ +-----+ +---------+ +-----+
| | | ----------\ |
| | |-| Compute | |
| | | | idle | |
| | | | ticks | |
| | | |---------| |
| | | -----------\ |
| | |-| Schedule | |
| | | | next | |
| | | | event | |
| | | |----------| |
| | | |
| | | _sys_soc_suspend(ticks) |
| | |--------------------------->|
| | | -----------\ |
| | | | Select |-|
| | | | policy | |
| | | | based on | |
| | | | ticks | |
| | | |----------| |
| | | -----------\ |
| | | | Execute |-|
| | | | DSO | |
| | | | policy | |
| | | |----------| |
| | | |
| | | PM_SOC_DEVICE_SUSPEND_ONLY |
| | |<---------------------------|
| | | |
| | | CPU |
| | | idle wait |
| | |---------- |
| | | | |
| | |<--------- |
| | | |


Proposed PMA DSO Exit:
+---------+ +-----+ +---------+ +-----+
| Events | | ISR | | Kernel | | PMA |
+---------+ +-----+ +---------+ +-----+
| | | |
| intr | | |
|-------->| | |
| | | |
| | process interrupt | |
| |----------------------->| |
| | | |
| | | _sys_soc_resume() |
| | |--------------------->|
| | | -----------\ |
| | | | Execute |-|
| | | | DSO | |
| | | | exit | |
| | | | policy | |
| | | |----------| |
| | | |
| | | return to kernel |
| | |<---------------------|
| | | |
| | return to ISR | |
| |<-----------------------| |
| | -------------\ | |
| |-| re-compute | | |
| | | Tickless | | |
| | | timeouts | | |
| | |------------| | |
| | -----------\ | |
| |-| schedule | | |
| | | next | | |
| | | task | | |
| | |----------| | |
| | | |


Re: RFC: 2/5 System Device Driver Modifications (Revised)

Thomas, Ramesh
 

[Revised incorporating feedbacks. The changes are shown at the end]

Problem Statement:
Not all Zephyr kernel drivers provide the same interfaces.

Why this is a problem:
-----------------------------
The Zephyr kernel currently has devices that are essential for core OS
functions (such as APICs and timers), but provide no public API means
for accessing them. Without any consistent method to access device
drivers on the platform, it becomes very difficult to achieve power
targets.

What should be done:
-----------------------------
1) Each native Zephyr kernel driver and shim driver (such as QMSI) shall
support:

a) uint32_t suspend() - routine that will move any required device state
data
to RAM* with the following valid return values:
- DEV_OK
- DEV_BUSY
- DEV_FAIL

b) uint32_t resume() - routine that will retrieve any device state data
from
RAM* with the following valid return values:

- DEV_OK
- DEV_FAIL

2) Provide a name recognized by the device_get_binding() function, this
includes what are currently thought to be drivers such as timers,
IOAPIC, and LOAPIC.

*The power management process is not expected to power down system RAM
(it will most likely stay in selective suspend).

The size of the device data is dependent upon an individual device, and
therefore the system integrator must be wary of the number of devices
being utilized.

Device suspend flow:
Device Drivers at suspend shall:
- Device state shall be saved to memory and maintained across a PM event
- Turning off the device clock
- Return appropriate error code

Device resume flow:
Device Drivers at resume shall:
- Restore to state saved in memory
- Turn on the device clock
- Return appropriate error code

Device domain experts will be needed to implement the proper methods for
suspending and resuming each device.

----New additions-------
Following 2 macros would be added to allow device drivers to implement
the .suspend() and .resume() hooks described in #1 above.

A device ops structure holds the .suspend() and .resume() which is
included inside each device's device structure:

struct device_ops example_dev_ops = {
.suspend = example_suspend,
.resume = example_resume
};

Two new macros would be created that drivers use instead of DEVICE_INIT
and SYS_INIT if they implement the .suspend and .resume functions. For
devices using non-PM macros, a default dev_ops will be assigned that
does nothing (so app need not check if dev_ops != NULL).

a) DEVICE_INIT_PM(dev_name, drv_name, init_fn, &example_dev_ops, data,
cfg_info, level, prio);

b) SYS_INIT_PM(name, init_fn, &example_dev_ops, level, prio);

SYS_INIT_PM also added the "name" argument so PMA can call
the .suspend/.resume for devices like APICs that need to do
suspend/resume operations. The methods that retrieve the pointer to the
device structure of any device requires the device to be assigned a name
unlike the SYS_INIT macro which assigns a "" as the name for devices.

All this will be inside DEVICE_POWER_MANAGEMENT Kconfig flag.


Re: RFC: 1/5 Consistent naming of PM Kconfig flags (Revised)

Thomas, Ramesh
 

[Revised incorporating feedbacks so far]

Problem Statement:
Power management Kconfig flags are not consistent and hierarchy is not
clear

Why this is a problem:
-----------------------------
The names include terms like “ADVANCED” which are not meaningful to
current implementation. Names do not specifically identify features that
are enabled by the flag. There are redundancies and overlaps in flags
and the hierarchy is not clear.

What should be done:
------------------------------
Change as follows :

ADVANCED_POWER_MANAGEMENT -> SYS_POWER_MANAGEMENT (turn on the basics
of power management)

ADVANCED_IDLE -> SYS_POWER_LPS (enables the kernel
for Low power state)
-> SYS_POWER_DEEP_SLEEP (enables the kernel for
Deep Sleep)

New addition -> DEVICE_POWER_MANAGEMENT (enables device power
management infrastructure)

ADVANCED_IDLE_SUPPORTED -> SYS_POWER_LPS_SUPPORTED (enables
SYS_POWER_LPS option)
-> SYS_POWER_DEEP_SLEEP_SUPPORTED (enables
SYS_POWER_DEEP_SLEEP option)

The new flags with the dependency hierarchy will be as follows:

SYS_POWER_MANAGEMENT
|
|___SYS_POWER_LPS
|
|___SYS_POWER_DEEP_SLEEP
|
|___DEVICE_POWER_MANAGEMENT


STM32/STM32F1 patchset v13

Maciek Borzecki <maciek.borzecki@...>
 

Hi,

I have posted version 13 of the patchset. As usual the series is
avaialable in my github repo https://github.com/bboozzoo/zephyr/ in
branch bboozzoo/stm32f10x-for-upstream-v13.

Changelog
=========

- addressed cosmetic comments

- updated pinmux driver to use struct pin_config instead of a custom
wrapper

- added support for HSE (high speed external oscillator) as clock
input for PLL or directly for SYSCLK, this allows for using much
higher SYSCLK values, up to 72MHz

- added register mapping for embedded flash controller; this change is
required by HSE and higher SYSCLK support, as flash access latency
needs to be configured for higher values of system clock

- svc_handler unaligned access fix; I would like to ask ARM experts to
look at this change, by no means I consider myself such expert. The
problem was identified when running latency benchmarks. The
benchmarks enable IRQ offloading, what enables previously unused
code paths in svc_handler. When the code attempts to access svc
parameter (which is not 4 byte aligned), unaligned access exception
occurs.


New Changes:
https://gerrit.zephyrproject.org/r/966 soc/stm32f1: add embedded
flash registers mapping
https://gerrit.zephyrproject.org/r/967 clock_control/stm32f1: HSE
support and PLL configuration cleanup
https://gerrit.zephyrproject.org/r/968 arm: access svc instruction
using halfword load in svc_handler


Updated Changes:
https://gerrit.zephyrproject.org/r/649 pinmux/stm32: add common
driver for STM32 pinmux
https://gerrit.zephyrproject.org/r/650 serial/stm32: add driver for
STM32 UART
https://gerrit.zephyrproject.org/r/651 gpio/stm32: add common driver
for STM32 GPIO
https://gerrit.zephyrproject.org/r/652 boards/stm32_mini_a15: add
new board
https://gerrit.zephyrproject.org/r/653 samples/drivers/disco: add
'disco' sample program
https://gerrit.zephyrproject.org/r/713 soc/stm32f1/gpio: implement
GPIO support
https://gerrit.zephyrproject.org/r/714 soc/stm32f1/pinmux: implement
STM32 pinmux integration
https://gerrit.zephyrproject.org/r/715 boards/nucleo_f103rb: add new
board
https://gerrit.zephyrproject.org/r/915 soc/stm32f1: add IRQ numbers
listing
https://gerrit.zephyrproject.org/r/916 serial/stm32: add support for
IRQ APIs
https://gerrit.zephyrproject.org/r/917
interupt_controller/stm32_exti: driver for STM32 EXTI controller
https://gerrit.zephyrproject.org/r/918 gpio/stm32: GPIO input with
interrupts
https://gerrit.zephyrproject.org/r/919 soc/stm32f1: AFIO registers
mapping
https://gerrit.zephyrproject.org/r/920 soc/stm32f1/gpio: implement
MCU specific GPIO input interrupt integration
https://gerrit.zephyrproject.org/r/921 watchdog/iwdg_stm32: add
driver for STM32 Independent Watchdog (IWDG)
https://gerrit.zephyrproject.org/r/922 samples/button: button input
example

Cheers,
--
Maciek Borzecki


Re: [PATCH 1/2] arc_timer: fix tickless idle

Desfarges, Simon <simon.desfarges@...>
 

Hello,
Thanks for the answer, I will review my copy.

BTW, the same test bench shows no jitter in the Quark TICK. So I would like to have the same behavior for ARC.
Do you think it is achievable ? I have doubts because of the fact that loapic decrements its counter (logic is reversed)
and looks to be safer.

See my [Simon] below.

-----Original Message-----
From: Mitsis, Peter [mailto:Peter.Mitsis(a)windriver.com]
Sent: Thursday, March 17, 2016 9:19 PM
To: Desfarges, Simon <simon.desfarges(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Desfarges, Simon <simon.desfarges(a)intel.com>
Subject: RE: [devel] [PATCH 1/2] arc_timer: fix tickless idle

See [Peter]

-----Original Message-----
From: simon.desfarges(a)intel.com [mailto:simon.desfarges(a)intel.com]
Sent: March-17-16 2:42 PM
To: devel(a)lists.zephyrproject.org
Cc: DESFARGES, SIMON
Subject: [devel] [PATCH 1/2] arc_timer: fix tickless idle

From: Simon Desfarges <simon.desfarges(a)intel.com>

When exiting from tickless idle uppon an external IRQ, the TICK timer is
set to fire at next TICK boundary. The current algorithm can lead to a
point that timer0_count register is higher than the timer0_limit register.

In this situation the next TICK will fire after a full wrapp of the
counter
(~133 seconds).

This condition appears when the counter reaches the limit after the
Interrupt Pending flag is checked. At this point the counter is
automatically wrapped to 0, but is set just next the limit to fire at next
TICK boundary by SW.
At exit of the _timer_idle_exit function, the timer handler is called, and
sets the limit to 1 TICK. At this point the situation is:
- limit register == 1 TICK
- count register is just below the old limit register and higher than 1
TICK

To fix this issue, at _timer_idle_exit, the limit register is always set
to 1 TICK and the count register set such as the next TICK fires on time.

Change-Id: Ifa002809d426aa04109592e53d2b02a224f51101
Tracked-On: https://jira.ndg.intel.com/browse/FIRE-4213
Signed-off-by: Simon Desfarges <simon.desfarges(a)intel.com>
---
drivers/timer/arcv2_timer0.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/timer/arcv2_timer0.c b/drivers/timer/arcv2_timer0.c
index 932ceac..5fec988 100644
--- a/drivers/timer/arcv2_timer0.c
+++ b/drivers/timer/arcv2_timer0.c
@@ -220,7 +220,7 @@ void _timer_idle_enter(int32_t ticks)
}

programmed_ticks = ticks;
- programmed_limit = (programmed_ticks * cycles_per_tick);
+ programmed_limit = (programmed_ticks * cycles_per_tick) - 1;
[Peter]: Good catch on this off by one error.
[Simon] I will separate in another patch and try to make it merged asap so we can forget this point.

timer0_limit_register_set(programmed_limit);

@@ -276,9 +276,6 @@ void _timer_idle_exit(void)
* A non-timer interrupt occurred. Announce any
* ticks that have elapsed during the tickless idle.
*/
-
- uint32_t remaining_cycles = programmed_limit - current_count;
-
_sys_idle_elapsed_ticks = current_count / cycles_per_tick;
if (_sys_idle_elapsed_ticks > 0) {
update_accumulated_count();
@@ -289,11 +286,8 @@ void _timer_idle_exit(void)
* Ensure the timer will expire at the end of the next tick in case
* the ISR makes any tasks and/or fibers ready to run.
*/
-
- if (remaining_cycles >= cycles_per_tick) {
- timer0_count_register_set(programmed_limit -
- ((remaining_cycles - 1) % cycles_per_tick) - 1);
- }
+ timer0_limit_register_set(cycles_per_tick - 1);
+ timer0_count_register_set(timer0_count_register_get() %
+cycles_per_tick);
[Peter]:

First an historical note. The reason the original code only touched the count
register was that it was not known if lowering the limit register to a value
below that of the count register would trigger an interrupt. (I did not have
access to a board with the ARCv2 timer and so was unable to confirm or deny
that behavior.) That being said, if changing the limit value from a value higher
than the count to a value lower than the count does not generate an
interrupt, then I have no objections on that front.
[Simon] This is the case I experimented. The ISR is generated only if count == limit.


Second, Using timer0_count_register_get() at this point is a no-no as it
introduces a race condition that can cause us to lose track of a whole tick.
Remember, the timer is always counting. If the elapsed ticks were
announced just prior to a "tick boundary", the call to
timer0_count_register_set() could then occur just after the "tick boundary".
The result of this scenario is that nearly one whole tick may be lost when the
modulus operation is performed. This is why we use the <current_count>
retrieved from the start of _timer_idle_exit(). Yes, it does introduce some
gradual drift, but this can be ameliorated somewhat by only modifying the
count if there is more than one tick left in the tickles idle.
[Simon]
I especially call this function to limit this gradual drift. As far as I understand, in every point of this function,
it is possible to have a race condition or an overlap of the counter. The case where the counter overlaps
after checking the 'IP' bit and before setting the count register is handled, because the ISR will fire anyway
and 1 tick will be announced.

I see in the loapic timer there is a 'timer_known_to_have_expired' variable, handling this exact case,
I will take a look at it (I guess you already did it...).


Hope this helps.

}
#else
static void tickless_idle_init(void) {}
--
1.9.1


Re: [PATCH 1/2] arc_timer: fix tickless idle

Mitsis, Peter <Peter.Mitsis@...>
 

See [Peter]

-----Original Message-----
From: simon.desfarges(a)intel.com [mailto:simon.desfarges(a)intel.com]
Sent: March-17-16 2:42 PM
To: devel(a)lists.zephyrproject.org
Cc: DESFARGES, SIMON
Subject: [devel] [PATCH 1/2] arc_timer: fix tickless idle

From: Simon Desfarges <simon.desfarges(a)intel.com>

When exiting from tickless idle uppon an external IRQ, the TICK timer is
set to fire at next TICK boundary. The current algorithm can lead to a
point that timer0_count register is higher than the timer0_limit register.

In this situation the next TICK will fire after a full wrapp of the
counter
(~133 seconds).

This condition appears when the counter reaches the limit after the
Interrupt Pending flag is checked. At this point the counter is
automatically wrapped to 0, but is set just next the limit to fire at next
TICK boundary by SW.
At exit of the _timer_idle_exit function, the timer handler is called, and
sets the limit to 1 TICK. At this point the situation is:
- limit register == 1 TICK
- count register is just below the old limit register and higher than 1
TICK

To fix this issue, at _timer_idle_exit, the limit register is always set
to 1 TICK and the count register set such as the next TICK fires on time.

Change-Id: Ifa002809d426aa04109592e53d2b02a224f51101
Tracked-On: https://jira.ndg.intel.com/browse/FIRE-4213
Signed-off-by: Simon Desfarges <simon.desfarges(a)intel.com>
---
drivers/timer/arcv2_timer0.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/timer/arcv2_timer0.c b/drivers/timer/arcv2_timer0.c
index 932ceac..5fec988 100644
--- a/drivers/timer/arcv2_timer0.c
+++ b/drivers/timer/arcv2_timer0.c
@@ -220,7 +220,7 @@ void _timer_idle_enter(int32_t ticks)
}

programmed_ticks = ticks;
- programmed_limit = (programmed_ticks * cycles_per_tick);
+ programmed_limit = (programmed_ticks * cycles_per_tick) - 1;
[Peter]: Good catch on this off by one error.

timer0_limit_register_set(programmed_limit);

@@ -276,9 +276,6 @@ void _timer_idle_exit(void)
* A non-timer interrupt occurred. Announce any
* ticks that have elapsed during the tickless idle.
*/
-
- uint32_t remaining_cycles = programmed_limit - current_count;
-
_sys_idle_elapsed_ticks = current_count / cycles_per_tick;
if (_sys_idle_elapsed_ticks > 0) {
update_accumulated_count();
@@ -289,11 +286,8 @@ void _timer_idle_exit(void)
* Ensure the timer will expire at the end of the next tick in case
* the ISR makes any tasks and/or fibers ready to run.
*/
-
- if (remaining_cycles >= cycles_per_tick) {
- timer0_count_register_set(programmed_limit -
- ((remaining_cycles - 1) % cycles_per_tick) - 1);
- }
+ timer0_limit_register_set(cycles_per_tick - 1);
+ timer0_count_register_set(timer0_count_register_get() %
+cycles_per_tick);
[Peter]:

First an historical note. The reason the original code only touched the count register was that it was not known if lowering the limit register to a value below that of the count register would trigger an interrupt. (I did not have access to a board with the ARCv2 timer and so was unable to confirm or deny that behavior.) That being said, if changing the limit value from a value higher than the count to a value lower than the count does not generate an interrupt, then I have no objections on that front.

Second, Using timer0_count_register_get() at this point is a no-no as it introduces a race condition that can cause us to lose track of a whole tick. Remember, the timer is always counting. If the elapsed ticks were announced just prior to a "tick boundary", the call to timer0_count_register_set() could then occur just after the "tick boundary". The result of this scenario is that nearly one whole tick may be lost when the modulus operation is performed. This is why we use the <current_count> retrieved from the start of _timer_idle_exit(). Yes, it does introduce some gradual drift, but this can be ameliorated somewhat by only modifying the count if there is more than one tick left in the tickles idle.

Hope this helps.

}
#else
static void tickless_idle_init(void) {}
--
1.9.1


Re: [PATCH 0/2] Arc tickless idle issue

Rodriguez, Sergio SF <sergio.sf.rodriguez@...>
 

Hi Simon,

The macro _sys_clock_tick_announce() call _nano_sys_clock_tick_announce(_sys_idle_elapsed_ticks), so _sys_idle_elapsed_ticks might off, can you check the value of _sys_idle_elapsed_ticks before every _sys_clock_tick_announce on the timer driver?, I think this line might be suspect on

drivers/timer/arcv2_timer0.c on the function _timer_idle_exit, look for the lines

uint32_t remaining_cycles = programmed_limit - current_count;

_sys_idle_elapsed_ticks = current_count / cycles_per_tick;
if (_sys_idle_elapsed_ticks > 0) {
update_accumulated_count();
_sys_clock_tick_announce();
}

The problem here is maybe _sys_idle_elapsed_ticks is going off,

also take a look at _timer_idle_exit when is coming off a timer interrupt,

if (control & _ARC_V2_TMR_CTRL_IP) {
/*
* The timer has expired. The handler _timer_int_handler() is
* guaranteed to execute. Track the number of elapsed ticks. The
* handler _timer_int_handler() will account for the final tick.
*/

_sys_idle_elapsed_ticks = programmed_ticks - 1;
update_accumulated_count();
_sys_clock_tick_announce();

return;
}

Is announcing the ticks minus one to all the timers (if any), so if no other interrupt happens (just the timer) the count will be fullfilled at the next tick, but if other interrupt happens and add more that one tick to _sys_idle_elapsed_ticks, that might set the timers with wrong count, so we have to revisit the non timer interrupt tick anounce

7801 - 7820 of 8194