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

[WIP] Adds delay command to firmware #217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions common/board.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ typedef struct TesselPort {
u32 spi_dipo;
u32 uart_dopo;
u32 uart_dipo;
TimerId delay_id;
} TesselPort;

#define SERCOM_PORT_A_SPI 5
Expand Down Expand Up @@ -113,6 +114,8 @@ const static TesselPort PORT_A = {
.spi_dopo = 3,
.uart_dipo = 3,
.uart_dopo = 1,
// TODO: figure out how to link to #define in firmware.h
.delay_id = 5,
};

const static TesselPort PORT_B = {
Expand All @@ -136,4 +139,6 @@ const static TesselPort PORT_B = {
.spi_dopo = 0,
.uart_dipo = 3,
.uart_dopo = 1,
// TODO: figure out how to link to #define in firmware.h
.delay_id = 1,
};
10 changes: 10 additions & 0 deletions firmware/firmware.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,20 @@
#define TC_TERMINAL_TIMEOUT 3
#define TC_BOOT 4

// Used for CMD_WAIT command on port A
#define TC_DELAY_PORT_A 5

// TCC allocation
// muxed with i2c. also used for uart read timers
#define TCC_PORT_A 2 // PA12, PA13
#define TCC_PORT_B 0 // PA08, PA09

// TCC 1 is first used for the boot led
// but once the MediaTek is booted, we
// use it as the delay TCC for port B
#define PWR_LED_TCC_CHAN 1
#define TCC_DELAY_PORT_B 1

// GCLK channel allocation
#define GCLK_SYSTEM 0
#define GCLK_32K 2
Expand Down Expand Up @@ -188,6 +197,7 @@ void port_dma_rx_completion(PortData* p);
void port_dma_tx_completion(PortData* p);
void port_handle_sercom_uart_i2c(PortData* p);
void port_handle_extint(PortData *p, u32 flags);
void port_handle_delay_complete(PortData *p);
void port_disable(PortData *p);
void uart_send_data(PortData *p);

Expand Down
69 changes: 49 additions & 20 deletions firmware/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ USB_ALIGN u8 flash_buffer[FLASH_BUFFER_SIZE];

// Indicates whether the SPI Daemon is listening for USB traffic
volatile bool booted = false;
// LED Chan: TCC1/WO[0]
#define PWR_LED_TCC_CHAN 1
// Indicates whether we are using TCC 1 for the boot led
// Same value as `booted` except for timer interrupt when booting has just
// completed
volatile bool tcc_for_boot = true;
// CC channel 0 on TCC instance 1
#define PWR_LED_CC_CHAN 0
// The maximum counter value of both the TCC and the pattern counter
Expand Down Expand Up @@ -74,24 +76,6 @@ uint32_t interpolate(uint32_t position) {
return ((MAX_COUNTER - between) * sin_wave_points[index] + between * sin_wave_points[next_index]) / MAX_COUNTER;
}

/*
Handler for the POWER LED breathing animation
*/
void TCC1_Handler() {
tcc(PWR_LED_TCC_CHAN)->INTFLAG.reg = TCC_INTFLAG_OVF;

// booted is true when the coprocess first gets
// a status packet from the spi daemon
if (booted == true) {
// Stop this breathing animation and cancel interrupts
cancel_breathing_animation();
}

counter += MAX_COUNTER / PATTERN_PERIOD_MS * MAX_COUNTER / TICKS_PER_MS;

// Take that proportion and extract a point along the sudo sine wave
tcc(PWR_LED_TCC_CHAN)->CCB[PWR_LED_CC_CHAN].bit.CCB = interpolate(counter);
}

/*
Sets up the TCC module to send PWM output to the PWR LED
Expand Down Expand Up @@ -365,3 +349,48 @@ void TCC_HANDLER(TCC_PORT_B) {
// clear irq
tcc(TCC_PORT_B)->INTFLAG.reg = TCC_INTENSET_OVF;
}

void TCC_HANDLER(TC_DELAY_PORT_A) {

// clear irq
tcc(TC_DELAY_PORT_A)->INTFLAG.reg = TCC_INTENSET_OVF;

// Have the state machine continue
port_handle_delay_complete(&port_a);
}

/*
Handler for the POWER LED breathing animation
*/
void TCC1_Handler() {

// If the TCC is being used for the boot led sequence
if (tcc_for_boot) {

tcc(PWR_LED_TCC_CHAN)->INTFLAG.reg = TCC_INTFLAG_OVF;

// booted is true when the coprocess first gets
// a status packet from the spi daemon
if (booted == true) {
// Stop using this TCC channel for the boot led
tcc_for_boot = false;
// Stop this breathing animation and cancel interrupts
cancel_breathing_animation();
// Do not update the counter
return;
}

counter += MAX_COUNTER / PATTERN_PERIOD_MS * MAX_COUNTER / TICKS_PER_MS;

// Take that proportion and extract a point along the sudo sine wave
tcc(PWR_LED_TCC_CHAN)->CCB[PWR_LED_CC_CHAN].bit.CCB = interpolate(counter);
}
// If the TCC is being used for Port B delays
else {
// clear irq
tcc(TCC_DELAY_PORT_B)->INTFLAG.reg = TCC_INTENSET_OVF;

// Have the state machine continue
port_handle_delay_complete(&port_b);
}
}
42 changes: 39 additions & 3 deletions firmware/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ typedef enum PortCmd {
CMD_NOP = 0,
CMD_FLUSH = 1,
CMD_ECHO = 2,
CMD_WAIT = 7,
CMD_GPIO_IN = 3, // switches pin to input mode AND reads value
CMD_GPIO_HIGH = 4, // switch to output and write high
CMD_GPIO_LOW = 5, // switch to output and write low
CMD_GPIO_TOGGLE = 21, // switch to output and toggle low/high
CMD_GPIO_PULL = 26, // Set the pin state to a specific pull value
CMD_GPIO_CFG = 6,
CMD_GPIO_WAIT = 7,
CMD_GPIO_INT = 8, // set interrupt on pin
CMD_GPIO_INPUT = 22, // switches pin to input, does not read value
CMD_GPIO_RAW_READ = 23, // reads pin state, does not switch between input/output
Expand Down Expand Up @@ -194,7 +194,6 @@ int port_cmd_args(PortCmd cmd) {
case CMD_GPIO_HIGH:
case CMD_GPIO_LOW:
case CMD_GPIO_TOGGLE:
case CMD_GPIO_WAIT:
case CMD_GPIO_INT:
case CMD_GPIO_CFG:
case CMD_GPIO_INPUT:
Expand All @@ -221,6 +220,10 @@ int port_cmd_args(PortCmd cmd) {
return 3; // 1 byte for pin, 2 bytes for duty cycle
case CMD_PWM_PERIOD:
return 3; // 1 byte for tcc id & prescalar, 2 bytes for period

case CMD_WAIT:
// 32-bit microseconds arg
return 4;
}
invalid();
return 0;
Expand Down Expand Up @@ -321,6 +324,30 @@ ExecStatus port_begin_cmd(PortData *p) {
port_send_status(p, REPLY_DATA);
return EXEC_CONTINUE;

case CMD_WAIT: {
// Parse the microsecond delay arguments
u32 us_delay = (p->arg[3]) |
(p->arg[2] << 8) |
(p->arg[1] << 16) |
(p->arg[0] << 24);

// 1s/ 48e6 ticks * 1e6 us / s = 48 ticks/us
// TODO: I think we also have to incorporate the 256 div prescalar
u32 ticks = 48 * us_delay;

// If the delay is 0, just return now
if (ticks == 0) {
return EXEC_DONE;
}

// Enable delays on this TCC channel
tcc_delay_enable(p->port->delay_id);
Copy link
Member

Choose a reason for hiding this comment

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

are the TC and TCC compatible enough to use the same function and register struct for both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good question. I'm not sure why I made that assumption. I'll look into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevinmehall is there any reason we can't re-use the TC_BOOT TC for port B? Then they could both use TCs instead of having to implement switching logic for TCs and TCCs.

// Start the delay
tcc_delay_start(p->port->delay_id, ticks);

return EXEC_ASYNC;
}

case CMD_TX:
return EXEC_CONTINUE;

Expand Down Expand Up @@ -408,7 +435,6 @@ ExecStatus port_begin_cmd(PortData *p) {
return EXEC_DONE;
}

case CMD_GPIO_WAIT:
case CMD_GPIO_CFG:
return EXEC_DONE;

Expand Down Expand Up @@ -830,3 +856,13 @@ void port_handle_extint(PortData *p, u32 flags) {

port_step(p);
}

void port_handle_delay_complete(PortData *p) {
if (p->state == PORT_EXEC_ASYNC) {
p->state = EXEC_DONE;
port_step(p);
}
else {
port_error(p);
}
}
32 changes: 27 additions & 5 deletions node/tessel-export.js
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,15 @@ Tessel.Port.prototype._txrx = function(buf, cb) {
this.uncork();
};

Tessel.Port.prototype.delay = function(delayUs) {
// Create a four byte buffer for the delay time
var microseconds = new Buffer(4);
// Pack the buffer with delay bytes
microseconds.writeUInt32BE(delayUs);
// Send off the delay command
this.sock.write(Buffer.concat([new Buffer([CMD.WAIT]), microseconds]));
Copy link
Contributor

@rwaldron rwaldron Sep 16, 2016

Choose a reason for hiding this comment

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

Instead of making two buffers, create a single 5 byte buffer and write the bytes into it:

var frame = new Buffer(5);
frame.writeUInt8(0xFF);
frame.writeUInt32BE(delayUs, 1);
 this.sock.write(frame);

Here's the difference when running on a Tessel 2 (1 execution each time):

...
INFO Running measure-buffer-allocation.js...
title                              result
---------------------------------  ---------
Two Buffers, one write and concat  0s 20.7ms
One Buffer, two writes             0s 2.86ms
...
INFO Running measure-buffer-allocation.js...
title                              result
---------------------------------  ----------
Two Buffers, one write and concat  0s 20.66ms
One Buffer, two writes             0s 2.8ms
...
INFO Running measure-buffer-allocation.js...
title                              result
---------------------------------  ----------
Two Buffers, one write and concat  0s 20.82ms
One Buffer, two writes             0s 3.04ms

(https://gist.github.com/rwaldron/23a0ea37641138f7bf0d04cc9bda6d34)

More importantly, this code must not be reachable if the delay is zero.

}

Tessel.Port.PATH = {
'A': '/var/run/tessel/port_a',
'B': '/var/run/tessel/port_b'
Expand Down Expand Up @@ -954,6 +963,8 @@ Tessel.SPI = function(params, port) {

this.chipSelectActive = params.chipSelectActive === 'high' || params.chipSelectActive === 1 ? 1 : 0;

this.chipSelectDelayUs = params.chipSelectDelayUs || 0;

if (this.chipSelectActive) {
// active high, pull low for now
this.chipSelect.low();
Expand Down Expand Up @@ -1006,11 +1017,15 @@ Tessel.SPI = function(params, port) {
};

Tessel.SPI.prototype.send = function(data, callback) {
this._port.cork();

this.chipSelect.low();
this._port.delay(this.chipSelectDelayUs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrap both of these in a condition, because if the delay is zero, we definitely do not want to waste the 3ms sending a message that effectively means "don't do anything"

this._port.cork();
Copy link
Member

Choose a reason for hiding this comment

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

no need to cork and uncork around the delay -- submit it all as one batch

Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at this gist: https://gist.github.com/rwaldron/4609336bb152388260c753d8e176fa8a

The average execution of _port.cork(), over 10 measurements is about 2.74ms, which means that an SPI object created with a chipSelectDelayUs that has any value less than 2740μs is always unnecessary (and indeed detrimental), because the call to cork has already taken that amount of time. I add that it's detrimental, because it's adding 2.5-3ms to the execution—even when the delay is zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

This might also be why your logic analyzer results are coming up inconsistent

this._port._tx(data, callback);
this._port.delay(this.chipSelectDelayUs);
this.chipSelect.high();
this._port.uncork();

};

Tessel.SPI.prototype.disable = function() {
Expand All @@ -1021,17 +1036,24 @@ Tessel.SPI.prototype.disable = function() {
};

Tessel.SPI.prototype.receive = function(data_len, callback) {
this._port.cork();

this.chipSelect.low();
this._port.delay(this.chipSelectDelayUs);
this._port.cork();
this._port._rx(data_len, callback);
this._port.delay(this.chipSelectDelayUs);
this.chipSelect.high();

this._port.uncork();
};

Tessel.SPI.prototype.transfer = function(data, callback) {
this._port.cork();

this.chipSelect.low();
this._port.delay(this.chipSelectDelayUs);
this._port.cork();
this._port._txrx(data, callback);
this._port.delay(this.chipSelectDelayUs);
this.chipSelect.high();
this._port.uncork();
};
Expand Down Expand Up @@ -1095,12 +1117,12 @@ var CMD = {
NOP: 0,
FLUSH: 1,
ECHO: 2,
WAIT: 7,
GPIO_IN: 3,
GPIO_HIGH: 4,
GPIO_LOW: 5,
GPIO_TOGGLE: 21,
GPIO_CFG: 6,
GPIO_WAIT: 7,
GPIO_INT: 8,
GPIO_INPUT: 22,
GPIO_RAW_READ: 23,
Expand Down Expand Up @@ -1546,7 +1568,7 @@ function getWifiInfo() {
if (bcastMatches === null) {
recursiveWifi(network);
} else {
// Successful matches will have a result that looks like:
// Successful matches will have a result that looks like:
// ["Bcast:0.0.0.0", "Bcast", "0.0.0.0"]
if (bcastMatches.length === 3) {
network.ip = bcastMatches[2];
Expand Down
58 changes: 57 additions & 1 deletion node/test/unit/tessel.js
Original file line number Diff line number Diff line change
Expand Up @@ -2397,7 +2397,10 @@ exports['Tessel.SPI'] = {
return this.socket;
}.bind(this));

this.tessel = new Tessel();
// We want to disable other ports so that we can pass around mock data.
// Otherwise, port A and B get 'readable' events without having any
// queued callbacks (because they are queued on the port we create below).
this.tessel = new Tessel({ ports: {A: false, B: false} });

this.cork = sandbox.stub(Tessel.Port.prototype, 'cork');
this.uncork = sandbox.stub(Tessel.Port.prototype, 'uncork');
Expand Down Expand Up @@ -2475,6 +2478,59 @@ exports['Tessel.SPI'] = {
test.ok(this.spiDisable.calledOnce, true);

test.done();
},
chipSelectDelayOptions: function(test) {
test.expect(2);

var delayUs = 10000;
var usToMs = 1000;

var s1 = new this.port.SPI();

var s2 = new this.port.SPI({chipSelectDelayUs: delayUs});

test.equal(s1.chipSelectDelay, 0);

test.equal(s2.chipSelectDelay, delayUs/usToMs);

test.done();
},
chipSelectDelayFunctionality: function(test) {
test.expect(2);

var delayUs = 10000;
var usToMs = 1000;

var s1 = new this.port.SPI({chipSelectDelayUs: delayUs});

var timeoutSpy = sandbox.spy(global, 'setTimeout');

s1.transfer(new Buffer(1), () => {
// One timeout called within transfer and one in the test below
test.equal(timeoutSpy.callCount, 2);
// The first call (chip select delay) waited an appropriate amount of time)
test.equal(timeoutSpy.getCall(0).args[1], delayUs/usToMs);
// Restore setTimeout
timeoutSpy.restore();
test.done();
});

// Wait until after the chip select delay has passed
// and the callback was enqueued. Only return data on the first request
setTimeout(() => {
var called = false;
this.socket.read = () => {
if (called) {
return new Buffer([]);
}
called = true;

return new Buffer([0x84, 0x1]);
};

// Prod the socket to read our buffer
s1._port.sock.emit('readable');
}, (delayUs / 10) * 2 );
}
};

Expand Down