Why not alloc/free with 'static' resource too?
clock c = clock_enable(XS1_CLKBLK_1);
port p = port_enable(XS1_PORT_1A);
Using enable/disable along with start/stop is confusing!
Especially as we are 'claiming' the specified resourse.
Do we want to allow multiple accessors viz calls to clock_enable()?
What happens if two threads call clock_enable(XS1_CLKBLK_1)? How should we handle it?
Could track 2ndary alloc & free with a global bit flag and error appropriately.
Use same pattern for all allocs viz:
timer timer_alloc(void);
lock lock_alloc(void);
chanend chanend_alloc(void);
Or:
void timer_alloc(timer t);
void lock_alloc(lock l);
void chanend_alloc(chanend c);
We need to be able to predict (and return an error) when a resource (dynamic or static) is not availble.
Thus we get:
ErrorCode chanend_alloc(chanend c);
ErrorCode timer_alloc(timer t);
ErrorCode clock_alloc(clock c, XS1_CLKBLK_1);
Do we want to offer a pair of chanends to pass around? Will this be confusing with XC 'chan'?
struct channel {
chanend left;
chanend right;
}
inline ErrorCode channel_alloc(channel *c) {
if (chanend_alloc(&c->left) ||
chanend_alloc(&c->right) ||
chanend_set_dest(c->left, c->right) ||
chanend_set_dest(c->right, c->left)) return ErrorChannelAlloc;
return ErrorNone;
}
void channel_free(channel c); // 64bit so efficient on xs2a
Is this the best way to partition the port api?
xcore_c_port.h
xcore_c_port_configure.h
Do we want to separate out:
basic usage
lowlevel config
highlevel protocols
Why byte and not word? Why not both?
void s_chan_output_block(streaming_chanend c, char buf[], int n);
void chan_output_block(chanend c, char buf[], int n);
void t_chan_output_block(REFERENCE_PARAM(transacting_chanend, tc), char buf[], int n)
Is this a bug?
inline void s_chan_free(streaming_chanend c1, streaming_chanend c2) {
s_chan_output_ct_end(c1);
s_chan_output_ct_end(c2);
s_chan_check_ct_end(c1);
s_chan_check_ct_end(c1); // is this a bug?
asm volatile("freer res[%0]" :: "r" (c1));
asm volatile("freer res[%0]" :: "r" (c2));
}
change (timer & time not tightly coupled):
inline void event_change_timer_time(timer t, int time);
inline void event_setup_timer(timer t, unsigned value, int time);
to (follows other event layouts - assuming timer-time coupling):
inline void event_change_timer_time(timer t, int time);
inline void event_setup_timer(timer t, int time, unsigned value);
if !defined(XS2A) // should test for xs1a/b/c devices (assume xs2b will be same as xs2a)
if ((value >> 16) != 0x1) {
assert(__FILE, LINE,
"On XS1 bit 16 will always be set in the value returned from an event");
}
endif
Why not write the code explicitly dual-issue (safe single-isue) for xs2a to be sure it is safe either way?
Do we always come out of a waiteu in single issue mode?
if defined(XS2A)
.issue_mode single
.align 4 // does this need to be align 8?
event_select_no_wait:
Why the trailing _s?
The idiom is 'struct transacting_chanend' and alias 'transacting_chanend' (as per c+)
typedef struct transacting_chanend_s {
chanend c;
int last_out;
} transacting_chanend;
comment in doc/rst/xcore_c.rst
chan_output_word(c, new_c); // send my new chanend to other.
chanend new_dest = chan_input_word(c); // recieve other's new chanend.
chanend_set_dest(new_c, new_dest); // connect my new chanend to other's new chanend.
Change 'EVENT_DEFAULT' to 'EVENT_NONE' - thus 'default' is not confuesed with 'EVENT_NONE'
typedef enum {
EVENT_CHAN_C = EVENT_ENUM_BASE,
EVENT_CHAN_D,
EVENT_NONE
} event_choice_t;
...
case EVENT_NONE: {
// Run background task
...
break;
default:
assert(0 && "some other event has triggered");
}
ghost
commented
8 years ago
Why not alloc/free with 'static' resource too? clock c = clock_enable(XS1_CLKBLK_1); port p = port_enable(XS1_PORT_1A); Using enable/disable along with start/stop is confusing! Especially as we are 'claiming' the specified resourse. Do we want to allow multiple accessors viz calls to clock_enable()? What happens if two threads call clock_enable(XS1_CLKBLK_1)? How should we handle it? Could track 2ndary alloc & free with a global bit flag and error appropriately.
Use same pattern for all allocs viz: timer timer_alloc(void); lock lock_alloc(void); chanend chanend_alloc(void); Or: void timer_alloc(timer t); void lock_alloc(lock l); void chanend_alloc(chanend c); We need to be able to predict (and return an error) when a resource (dynamic or static) is not availble. Thus we get: ErrorCode chanend_alloc(chanend c); ErrorCode timer_alloc(timer t); ErrorCode clock_alloc(clock c, XS1_CLKBLK_1);
Do we want to offer a pair of chanends to pass around? Will this be confusing with XC 'chan'? struct channel { chanend left; chanend right; } inline ErrorCode channel_alloc(channel *c) { if (chanend_alloc(&c->left) || chanend_alloc(&c->right) || chanend_set_dest(c->left, c->right) || chanend_set_dest(c->right, c->left)) return ErrorChannelAlloc; return ErrorNone; } void channel_free(channel c); // 64bit so efficient on xs2a
Is this the best way to partition the port api? xcore_c_port.h xcore_c_port_configure.h Do we want to separate out: basic usage lowlevel config highlevel protocols
Why byte and not word? Why not both? void s_chan_output_block(streaming_chanend c, char buf[], int n); void chan_output_block(chanend c, char buf[], int n); void t_chan_output_block(REFERENCE_PARAM(transacting_chanend, tc), char buf[], int n)
Is this a bug? inline void s_chan_free(streaming_chanend c1, streaming_chanend c2) { s_chan_output_ct_end(c1); s_chan_output_ct_end(c2); s_chan_check_ct_end(c1); s_chan_check_ct_end(c1); // is this a bug? asm volatile("freer res[%0]" :: "r" (c1)); asm volatile("freer res[%0]" :: "r" (c2)); }
change (timer & time not tightly coupled): inline void event_change_timer_time(timer t, int time); inline void event_setup_timer(timer t, unsigned value, int time); to (follows other event layouts - assuming timer-time coupling): inline void event_change_timer_time(timer t, int time); inline void event_setup_timer(timer t, int time, unsigned value);
if !defined(XS2A) // should test for xs1a/b/c devices (assume xs2b will be same as xs2a)
if ((value >> 16) != 0x1) { assert(__FILE, LINE, "On XS1 bit 16 will always be set in the value returned from an event"); }
endif
Why not write the code explicitly dual-issue (safe single-isue) for xs2a to be sure it is safe either way? Do we always come out of a waiteu in single issue mode?
if defined(XS2A)
inline void s_chan_free(streaming_chanend c1, streaming_chanend c2) { s_chan_output_ct_end(c1); s_chan_output_ct_end(c2); s_chan_check_ct_end(c1); s_chan_check_ct_end(c1); chan_free(c1, c2); // maintain symmetry with s_chan_alloc() }
Why the trailing _s? The idiom is 'struct transacting_chanend' and alias 'transacting_chanend' (as per c+) typedef struct transacting_chanend_s { chanend c; int last_out; } transacting_chanend;
comment in doc/rst/xcore_c.rst chan_output_word(c, new_c); // send my new chanend to other. chanend new_dest = chan_input_word(c); // recieve other's new chanend. chanend_set_dest(new_c, new_dest); // connect my new chanend to other's new chanend.
Change 'EVENT_DEFAULT' to 'EVENT_NONE' - thus 'default' is not confuesed with 'EVENT_NONE' typedef enum { EVENT_CHAN_C = EVENT_ENUM_BASE, EVENT_CHAN_D, EVENT_NONE } event_choice_t; ... case EVENT_NONE: { // Run background task ... break; default: assert(0 && "some other event has triggered"); }