Skip to content

Commit 61b3c95

Browse files
committed
tests/drivers/ipm: support ipm test case in SMP
Current design of ipm test doesn't consider SMP, there is the possibility that offload_irq and rx_thread run at different cpu core at the same time, it will cause state(enabled and channel_disable) disorder if two different context access them concurrently. Fixes #12478. Signed-off-by: Wentong Wu <[email protected]>
1 parent cb44b7e commit 61b3c95

File tree

6 files changed

+40
-39
lines changed

6 files changed

+40
-39
lines changed

drivers/console/ipm_console_receiver.c

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,41 +15,49 @@
1515
#include <ipm.h>
1616
#include <console/ipm_console.h>
1717
#include <misc/__assert.h>
18+
#include <atomic.h>
1819

1920
static void ipm_console_thread(void *arg1, void *arg2, void *arg3)
2021
{
2122
u8_t size32;
2223
u16_t type;
23-
int ret, key;
24+
int ret;
25+
k_spinlock_key_t key;
2426
struct device *d;
2527
const struct ipm_console_receiver_config_info *config_info;
2628
struct ipm_console_receiver_runtime_data *driver_data;
27-
int pos;
29+
int pos, end;
30+
atomic_t *target;
2831

2932
d = (struct device *)arg1;
3033
driver_data = d->driver_data;
3134
config_info = d->config->config_info;
3235
ARG_UNUSED(arg2);
3336
size32 = 0U;
3437
pos = 0;
38+
target = config_info->print_num;
3539

3640
while (1) {
3741
k_sem_take(&driver_data->sem, K_FOREVER);
3842

43+
key = k_spin_lock(&driver_data->rb_spinlock);
3944
ret = ring_buf_item_get(&driver_data->rb, &type,
4045
(u8_t *)&config_info->line_buf[pos],
4146
NULL, &size32);
47+
k_spin_unlock(&driver_data->rb_spinlock, key);
4248
if (ret) {
4349
/* Shouldn't ever happen... */
4450
printk("ipm console ring buffer error: %d\n", ret);
4551
size32 = 0U;
4652
continue;
4753
}
4854

55+
end = 0;
4956
if (config_info->line_buf[pos] == '\n' ||
5057
pos == config_info->lb_size - 2) {
5158
if (pos != config_info->lb_size - 2) {
5259
config_info->line_buf[pos] = '\0';
60+
end = 1;
5361
} else {
5462
config_info->line_buf[pos + 1] = '\0';
5563
}
@@ -61,26 +69,14 @@ static void ipm_console_thread(void *arg1, void *arg2, void *arg3)
6169
printf("%s: '%s'\n", d->config->name,
6270
config_info->line_buf);
6371
}
72+
if (end == 1) {
73+
atomic_dec(target);
74+
}
6475
pos = 0;
6576
} else {
6677
++pos;
6778
}
6879

69-
/* ISR may have disabled the channel due to full buffer at
70-
* some point. If that happened and there is now room,
71-
* re-enable it.
72-
*
73-
* Lock interrupts to avoid pathological scenario where
74-
* the buffer fills up in between enabling the channel and
75-
* clearing the channel_disabled flag.
76-
*/
77-
if (driver_data->channel_disabled &&
78-
ring_buf_space_get(&driver_data->rb)) {
79-
key = irq_lock();
80-
ipm_set_enabled(driver_data->ipm_device, 1);
81-
driver_data->channel_disabled = 0;
82-
irq_unlock(key);
83-
}
8480
}
8581
}
8682

@@ -90,27 +86,20 @@ static void ipm_console_receive_callback(void *context, u32_t id,
9086
struct device *d;
9187
struct ipm_console_receiver_runtime_data *driver_data;
9288
int ret;
89+
k_spinlock_key_t key;
9390

9491
ARG_UNUSED(data);
9592
d = context;
9693
driver_data = d->driver_data;
9794

95+
key = k_spin_lock(&driver_data->rb_spinlock);
9896
/* Should always be at least one free buffer slot */
9997
ret = ring_buf_item_put(&driver_data->rb, 0, id, NULL, 0);
98+
k_spin_unlock(&driver_data->rb_spinlock, key);
10099
__ASSERT(ret == 0, "Failed to insert data into ring buffer");
101100
k_sem_give(&driver_data->sem);
102101

103-
/* If the buffer is now full, disable future interrupts for this channel
104-
* until the thread has a chance to consume characters.
105-
*
106-
* This works without losing data if the sending side tries to send
107-
* more characters because the sending side is making an ipm_send()
108-
* call with the wait flag enabled. It blocks until the receiver side
109-
* re-enables the channel and consumes the data.
110-
*/
111-
if (ring_buf_space_get(&driver_data->rb) == 0) {
112-
ipm_set_enabled(driver_data->ipm_device, 0);
113-
driver_data->channel_disabled = 1;
102+
while (ring_buf_space_get(&driver_data->rb) == 0) {
114103
}
115104
}
116105

include/drivers/console/ipm_console.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include <kernel.h>
1313
#include <device.h>
1414
#include <ring_buffer.h>
15+
#include <spinlock.h>
16+
#include <atomic.h>
1517

1618
#ifdef __cplusplus
1719
extern "C" {
@@ -61,6 +63,8 @@ struct ipm_console_receiver_config_info {
6163
* IPM_CONSOLE_STDOUT or IPM_CONSOLE_PRINTK
6264
*/
6365
unsigned int flags;
66+
67+
atomic_t *print_num;
6468
};
6569

6670
struct ipm_console_receiver_runtime_data {
@@ -80,6 +84,11 @@ struct ipm_console_receiver_runtime_data {
8084

8185
/** Receiver worker thread */
8286
struct k_thread rx_thread;
87+
88+
/**
89+
* Ring buffer spinlock
90+
*/
91+
struct k_spinlock rb_spinlock;
8392
};
8493

8594
struct ipm_console_sender_config_info {

include/ring_buffer.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ extern "C" {
2525
* @brief A structure to represent a ring buffer
2626
*/
2727
struct ring_buf {
28-
u32_t head; /**< Index in buf for the head element */
29-
u32_t tail; /**< Index in buf for the tail element */
28+
volatile u32_t head; /**< Index in buf for the head element */
29+
volatile u32_t tail; /**< Index in buf for the tail element */
3030
union ring_buf_misc {
3131
struct ring_buf_misc_item_mode {
3232
u32_t dropped_put_count; /**< Running tally of the
@@ -39,7 +39,7 @@ struct ring_buf {
3939
u32_t tmp_head;
4040
} byte_mode;
4141
} misc;
42-
u32_t size; /**< Size of buf in 32-bit chunks */
42+
volatile u32_t size; /**< Size of buf in 32-bit chunks */
4343

4444
union ring_buf_buffer {
4545
u32_t *buf32; /**< Memory region for stored entries */

tests/drivers/ipm/src/ipm_dummy.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,8 @@ static int ipm_dummy_send(struct device *d, int wait, u32_t id,
6969
driver_data->regs.id = id;
7070
driver_data->regs.busy = 1U;
7171

72-
irq_offload(ipm_dummy_isr, d);
72+
ipm_dummy_isr(d);
7373

74-
if (wait) {
75-
while (driver_data->regs.busy) {
76-
/* busy-wait */
77-
}
78-
}
7974
return 0;
8075
}
8176

tests/drivers/ipm/src/main.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <device.h>
1111
#include <init.h>
1212
#include <stdio.h>
13+
#include <atomic.h>
1314

1415
#include <tc_util.h>
1516
#include "ipm_dummy.h"
@@ -54,6 +55,7 @@ DEVICE_INIT(ipm_console_send0, "ipm_send0", ipm_console_sender_init,
5455
static u32_t ring_buf_data[RING_BUF_SIZE32];
5556
static K_THREAD_STACK_DEFINE(thread_stack, IPM_CONSOLE_STACK_SIZE);
5657
static char line_buf[LINE_BUF_SIZE];
58+
static atomic_t target;
5759

5860
/* Dump incoming messages to printk() */
5961
static struct ipm_console_receiver_config_info receiver_config = {
@@ -63,7 +65,8 @@ static struct ipm_console_receiver_config_info receiver_config = {
6365
.rb_size32 = RING_BUF_SIZE32,
6466
.line_buf = line_buf,
6567
.lb_size = LINE_BUF_SIZE,
66-
.flags = DEST
68+
.flags = DEST,
69+
.print_num = &target
6770
};
6871

6972
struct ipm_console_receiver_runtime_data receiver_data;
@@ -78,16 +81,19 @@ void main(void)
7881
int rv, i;
7982
struct device *ipm;
8083

84+
target = 0;
8185
TC_START("Test IPM");
8286
ipm = device_get_binding("ipm_dummy0");
8387

8488
/* Try sending a raw string to the IPM device to show that the
8589
* receiver works
8690
*/
91+
atomic_inc(&target);
8792
for (i = 0; i < strlen(thestr); i++) {
8893
ipm_send(ipm, 1, thestr[i], NULL, 0);
8994
}
9095

96+
atomic_inc(&target);
9197
/* Now do this through printf() to exercise the sender */
9298
printf("Lorem ipsum dolor sit amet, consectetur adipiscing elit, "
9399
"sed do eiusmod tempor incididunt ut labore et dolore magna "
@@ -102,6 +108,9 @@ void main(void)
102108
* automation purposes?
103109
*/
104110

111+
while (atomic_get(&target) != 0) {
112+
}
113+
105114
rv = TC_PASS;
106115
TC_END_RESULT(rv);
107116
TC_END_REPORT(rv);

tests/drivers/ipm/testcase.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,4 @@ tests:
22
peripheral.mailbox:
33
filter: not CONFIG_SOC_QUARK_SE_C1000_SS
44
arch_exclude: posix xtensa
5-
platform_exclude: qemu_x86_64 # see issue #12478
65
tags: drivers ipc

0 commit comments

Comments
 (0)