Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: counter: add support for top value configuration on native_posix #63897

Merged
merged 1 commit into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion boards/posix/native_posix/hw_counter.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ static bool counter_running;
static uint64_t counter_value;
static uint64_t counter_target;
static uint64_t counter_period;
static uint64_t counter_wrap;

/**
* Initialize the counter with prescaler of HW
Expand All @@ -28,6 +29,7 @@ void hw_counter_init(void)
counter_value = 0;
counter_running = false;
counter_period = NEVER;
counter_wrap = NEVER;
}

void hw_counter_triggered(void)
Expand All @@ -38,7 +40,7 @@ void hw_counter_triggered(void)
}

hw_counter_timer = hwm_get_time() + counter_period;
counter_value = counter_value + 1;
counter_value = (counter_value + 1) % counter_wrap;

if (counter_value == counter_target) {
hw_irq_ctrl_set_irq(COUNTER_EVENT_IRQ);
Expand All @@ -54,6 +56,16 @@ void hw_counter_set_period(uint64_t period)
counter_period = period;
}

/*
* Set the count value at which the counter will wrap
* The counter will count up to (counter_wrap-1), i.e.:
* 0, 1, 2,.., (counter_wrap - 1), 0
*/
void hw_counter_set_wrap_value(uint64_t wrap_value)
{
counter_wrap = wrap_value;
}

/**
* Starts the counter. It must be previously configured with
* hw_counter_set_period() and hw_counter_set_target().
Expand Down Expand Up @@ -82,6 +94,11 @@ void hw_counter_stop(void)
hwm_find_next_timer();
}

bool hw_counter_is_started(void)
aescolar marked this conversation as resolved.
Show resolved Hide resolved
{
return counter_running;
}

/**
* Returns the current counter value.
*/
Expand All @@ -90,6 +107,14 @@ uint64_t hw_counter_get_value(void)
return counter_value;
}

/**
* Resets the counter value.
*/
void hw_counter_reset(void)
{
counter_value = 0;
aescolar marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Configures the counter to generate an interrupt
* when its count value reaches target.
Expand Down
3 changes: 3 additions & 0 deletions boards/posix/native_posix/hw_counter.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ void hw_counter_triggered(void);

void hw_counter_set_period(uint64_t period);
void hw_counter_set_target(uint64_t counter_target);
void hw_counter_set_wrap_value(uint64_t wrap_value);
void hw_counter_start(void);
void hw_counter_stop(void);
bool hw_counter_is_started(void);
uint64_t hw_counter_get_value(void);
void hw_counter_reset(void);

#ifdef __cplusplus
}
Expand Down
1 change: 1 addition & 0 deletions boards/posix/native_posix/native_posix.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ toolchain:
- llvm
supported:
- can
- counter
- eeprom
- netif:eth
- usb_device
Expand Down
1 change: 1 addition & 0 deletions boards/posix/native_posix/native_posix_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ toolchain:
- llvm
supported:
- can
- counter
- eeprom
- netif:eth
- usb_device
Expand Down
1 change: 1 addition & 0 deletions boards/posix/native_sim/native_sim.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ toolchain:
- llvm
supported:
- can
- counter
- eeprom
- netif:eth
- usb_device
Expand Down
1 change: 1 addition & 0 deletions boards/posix/native_sim/native_sim_64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ toolchain:
- llvm
supported:
- can
- counter
- eeprom
- netif:eth
- usb_device
Expand Down
5 changes: 5 additions & 0 deletions drivers/counter/Kconfig.native_posix
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ config COUNTER_NATIVE_POSIX_FREQUENCY
int "native_posix counter frequency in Hz"
default 1000
depends on COUNTER_NATIVE_POSIX

config COUNTER_NATIVE_POSIX_NBR_CHANNELS
int "native counter, number of channels"
default 4
depends on COUNTER_NATIVE_POSIX
145 changes: 119 additions & 26 deletions drivers/counter/counter_native_posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#define DT_DRV_COMPAT zephyr_native_posix_counter

#include <string.h>
#include <zephyr/device.h>
#include <zephyr/drivers/counter.h>
#include <zephyr/irq.h>
Expand All @@ -14,38 +15,80 @@
#include <limits.h>

#define DRIVER_CONFIG_INFO_FLAGS (COUNTER_CONFIG_INFO_COUNT_UP)
#define DRIVER_CONFIG_INFO_CHANNELS 1
#define DRIVER_CONFIG_INFO_CHANNELS CONFIG_COUNTER_NATIVE_POSIX_NBR_CHANNELS
#define COUNTER_NATIVE_POSIX_IRQ_FLAGS (0)
#define COUNTER_NATIVE_POSIX_IRQ_PRIORITY (2)

#define COUNTER_PERIOD (USEC_PER_SEC / CONFIG_COUNTER_NATIVE_POSIX_FREQUENCY)
#define TOP_VALUE (UINT_MAX)

static struct counter_alarm_cfg pending_alarm;
static bool is_alarm_pending;
static struct counter_alarm_cfg pending_alarm[DRIVER_CONFIG_INFO_CHANNELS];
static bool is_alarm_pending[DRIVER_CONFIG_INFO_CHANNELS];
static struct counter_top_cfg top;
static bool is_top_set;
static const struct device *device;

static void schedule_next_isr(void)
{
int64_t current_value = hw_counter_get_value();
uint32_t next_time = top.ticks; /* top.ticks is TOP_VALUE if is_top_set == false */

if (current_value == top.ticks) {
current_value = -1;
}

for (int i = 0; i < DRIVER_CONFIG_INFO_CHANNELS; i++) {
if (is_alarm_pending[i]) {
if (pending_alarm[i].ticks > current_value) {
/* If the alarm is not after a wrap */
next_time = MIN(pending_alarm[i].ticks, next_time);
}
}
}

/* We will at least get an interrupt at top.ticks even if is_top_set == false,
* which is fine. We may use that to set the next alarm if needed
*/
hw_counter_set_target(next_time);
}

static void counter_isr(const void *arg)
{
ARG_UNUSED(arg);
uint32_t current_value = hw_counter_get_value();

if (is_alarm_pending) {
is_alarm_pending = false;
pending_alarm.callback(device, 0, current_value,
pending_alarm.user_data);
for (int i = 0; i < DRIVER_CONFIG_INFO_CHANNELS; i++) {
if (is_alarm_pending[i] && (current_value == pending_alarm[i].ticks)) {
is_alarm_pending[i] = false;
if (pending_alarm[i].callback) {
pending_alarm[i].callback(device, i, current_value,
pending_alarm[i].user_data);
}
}
}

if (is_top_set && (current_value == top.ticks)) {
if (top.callback) {
aescolar marked this conversation as resolved.
Show resolved Hide resolved
top.callback(device, top.user_data);
}
}
aescolar marked this conversation as resolved.
Show resolved Hide resolved

schedule_next_isr();
}

static int ctr_init(const struct device *dev)
{
device = dev;
is_alarm_pending = false;
memset(is_alarm_pending, 0, sizeof(is_alarm_pending));
is_top_set = false;
top.ticks = TOP_VALUE;

IRQ_CONNECT(COUNTER_EVENT_IRQ, COUNTER_NATIVE_POSIX_IRQ_PRIORITY,
counter_isr, NULL, COUNTER_NATIVE_POSIX_IRQ_FLAGS);
irq_enable(COUNTER_EVENT_IRQ);
hw_counter_set_period(COUNTER_PERIOD);
hw_counter_set_target(TOP_VALUE);
hw_counter_set_wrap_value((uint64_t)top.ticks + 1);
hw_counter_reset();

return 0;
}
Expand All @@ -54,6 +97,7 @@ static int ctr_start(const struct device *dev)
{
ARG_UNUSED(dev);

schedule_next_isr();
hw_counter_start();
return 0;
}
Expand All @@ -80,41 +124,88 @@ static uint32_t ctr_get_pending_int(const struct device *dev)
return 0;
}

static bool is_any_alarm_pending(void)
{
for (int i = 0; i < DRIVER_CONFIG_INFO_CHANNELS; i++) {
if (is_alarm_pending[i]) {
return true;
}
}
return false;
}

static int ctr_set_top_value(const struct device *dev,
const struct counter_top_cfg *cfg)
{
ARG_UNUSED(dev);
ARG_UNUSED(cfg);

posix_print_warning("%s not supported\n", __func__);
return -ENOTSUP;
if (is_any_alarm_pending()) {
posix_print_warning("Can't set top value while alarm is active\n");
return -EBUSY;
}

uint32_t current_value = hw_counter_get_value();

if (cfg->flags & COUNTER_TOP_CFG_DONT_RESET) {
if (current_value >= cfg->ticks) {
if (cfg->flags & COUNTER_TOP_CFG_RESET_WHEN_LATE) {
hw_counter_reset();
}
return -ETIME;
}
} else {
hw_counter_reset();
}

top = *cfg;
hw_counter_set_wrap_value((uint64_t)top.ticks + 1);

if ((cfg->ticks == TOP_VALUE) && !cfg->callback) {
is_top_set = false;
} else {
is_top_set = true;
}

schedule_next_isr();

return 0;
}

static uint32_t ctr_get_top_value(const struct device *dev)
{
return TOP_VALUE;
return top.ticks;
}

static int ctr_set_alarm(const struct device *dev, uint8_t chan_id,
const struct counter_alarm_cfg *alarm_cfg)
{
ARG_UNUSED(dev);

if (chan_id >= DRIVER_CONFIG_INFO_CHANNELS) {
posix_print_warning("channel %u is not supported\n", chan_id);
return -ENOTSUP;
}
if (is_alarm_pending[chan_id])
return -EBUSY;

pending_alarm = *alarm_cfg;
is_alarm_pending = true;
uint32_t ticks = alarm_cfg->ticks;

if (ticks > top.ticks) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check not happen after handling COUNTER_ALARM_CFG_ABSOLUTE? If ticks is relative to current_value, the alarm will never be reached.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check not happen after handling COUNTER_ALARM_CFG_ABSOLUTE?

I think not. The API description is not clear about this, but from how the test behaves here:
https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/counter/counter_basic_api/src/test_counter.c#L396-L400
the test expects the driver to reject a relative alarm with a tick offset > top.ticks

posix_print_warning("Alarm ticks %u exceed top ticks %u\n", ticks,
top.ticks);
return -EINVAL;
}

if (!(alarm_cfg->flags & COUNTER_ALARM_CFG_ABSOLUTE)) {
pending_alarm.ticks =
hw_counter_get_value() + pending_alarm.ticks;
uint32_t current_value = hw_counter_get_value();

ticks += current_value;
if (ticks > top.ticks) { /* Handle wrap arounds */
ticks -= (top.ticks + 1); /* The count period is top.ticks + 1 */
}
}

hw_counter_set_target(pending_alarm.ticks);
irq_enable(COUNTER_EVENT_IRQ);
pending_alarm[chan_id] = *alarm_cfg;
pending_alarm[chan_id].ticks = ticks;
is_alarm_pending[chan_id] = true;

schedule_next_isr();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to call hw_counter_set_wrap_value here? Previous top configuration may have set it less than the alarm ticks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow. The wrap is configured at init, or every time ctr_set_top_value() is called successfully.
And the hw wrap remains in effect after that for ever.

Apart from that, we are checking in 189 and 199 that the alarm target does not go beyond the top.tick / wrap_value.


return 0;
}
Expand All @@ -123,12 +214,14 @@ static int ctr_cancel_alarm(const struct device *dev, uint8_t chan_id)
{
ARG_UNUSED(dev);

if (chan_id >= DRIVER_CONFIG_INFO_CHANNELS) {
posix_print_warning("channel %u is not supported\n", chan_id);
if (!hw_counter_is_started()) {
posix_print_warning("Counter not started\n");
return -ENOTSUP;
}

is_alarm_pending = false;
is_alarm_pending[chan_id] = false;

schedule_next_isr();

return 0;
}
Expand Down
Loading