I'm using the I2C module for the slave component on an explorerkit. It's connected to a raspberry pi 3 which is master running at 400KHZ max. Boot order is raspberry pi, and then xmos. At the end of the boot process for the pi 3 a program starts running to get data from the explorerkit. The problem is the xmos i2c slave holds the clock low indefinitely. After I press the button to reset the xmos device they both run just fine at 166-250KHZ, usually closer to 250KHZ. If I reset the pi which resets both it goes right back to holding the clock low.
So has anybody else had an issue with the xmos i2c slave component holding the clock low indefinitely? If I have to I'm sure I can monitor the bus on the pi and if I find the clock is held low reset the xmos board but that's not ideal.
The PI seems to be unaware of the issue also as I can see on my oscilloscope that the clock is still running when the clock is held low, but instead of peak to peak being around 3.6V, it closer to 200mV.
I2C holding clock low Topic is solved
-
Gothmag
- XCore Addict
- Posts: 129
- Joined: Wed May 11, 2016 3:50 pm
-
Gothmag
- XCore Addict
- Posts: 129
- Joined: Wed May 11, 2016 3:50 pm
Apparently the raspberry PI has an I2C hardware bug so I'll probably just blame it on that since the same code and library work fine when talking to other micros over i2c.
After some testing the changes seem to be working. The specific bug is it only supports stretching after an ack, on a read. Also only if it stretches for >.5 clock periods. To get it working I removed all of the ensure_setup_time() calls to help keep speed up. I also removed all instances of clock stretching. Added a monitor on writes to look for specific values and stored it in a thread variable. That isn't needed but I didn't want to stretch on all reads, just the 2 specific ones that make calls elsewhere, you could do it always if you didn't mind the speed slowing down. This all works because I have copies of all the data locally in the i2c slave callback thread(user code). Luckily if needed you could set a specific register as your do stuff register if you know it takes too long to be able to do it without stretching.
So I added an unsigned for stretch time, begin, and end and a timer at the beginning. This is the pre fix code, only in ack_addr of slave component
This is after the changes. All my data is setup in start_read_request. So the ack needs to happen asap, and then comes down if my id'd values are there then it gets start time, runs the user code, and gets stretch end time. Calculates total stretch and makes sure it stretches at least 1.75us which is a very safe value. For general use you just need to remove if (trans_type != 0) along with the stretch time condition block, leaving the rest since it's not needed.
I still run 166-250khz clock but it does seem to be slightly more time at 166khz than 250khz than before but works at startup. If anybody else decides to communicate with a pi using the hardware i2c ports this is essentially what needs to be done and you need to work around the limitations.
Next is just the complete slave source file I've edited.
After some testing the changes seem to be working. The specific bug is it only supports stretching after an ack, on a read. Also only if it stretches for >.5 clock periods. To get it working I removed all of the ensure_setup_time() calls to help keep speed up. I also removed all instances of clock stretching. Added a monitor on writes to look for specific values and stored it in a thread variable. That isn't needed but I didn't want to stretch on all reads, just the 2 specific ones that make calls elsewhere, you could do it always if you didn't mind the speed slowing down. This all works because I have copies of all the data locally in the i2c slave callback thread(user code). Luckily if needed you could set a specific register as your do stuff register if you know it takes too long to be able to do it without stretching.
So I added an unsigned for stretch time, begin, and end and a timer at the beginning. This is the pre fix code, only in ack_addr of slave component
Code: Select all
case ACK_ADDR:
// Stretch clock (hold low) while application code is called
p_scl <: 0;
// Callback to the application to determine whether to ACK
// or NACK the address.
int ack;
if (rw) {
i.start_read_request();
ack = i.ack_read_request();
} else {
i.start_write_request();
ack = i.ack_write_request();
}
ignore_stop_bit = 0;
if (ack == I2C_SLAVE_NACK) {
// Release the data line so that it is pulled high
p_sda :> void;
next_state = WAITING_FOR_START_OR_STOP;
} else {
// Drive the ACK low
p_sda <: 0;
if (rw) {
next_state = MASTER_READ;
{ else {
next_state = MASTER_WRITE;
}
}
scl_val = 1;
state = ACK_WAIT_HIGH;
ensure_setup_time();
// Release the clock
p_scl :> void;
break;Code: Select all
case ACK_ADDR:
// Callback to the application to determine whether to ACK
// or NACK the address.
int ack;
if (rw) {
ack = i.ack_read_request();
} else {
i.start_write_request();
ack = i.ack_write_request();
}
ignore_stop_bit = 0;
if (ack == I2C_SLAVE_NACK) {
// Release the data line so that it is pulled high
p_sda :> void;
next_state = WAITING_FOR_START_OR_STOP;
} else {
// Drive the ACK low
p_sda <: 0;
if (rw) {
next_state = MASTER_READ;
} else {
next_state = MASTER_WRITE;
}
}
scl_val = 1;
state = ACK_WAIT_HIGH;
if (rw) {
if (trans_type != 0) {
p_scl <: 0;
stretch_timer :> stretch_start;
trans_type = 0;
stretch_time = 1;
} else stretch_time = 0;
i.start_read_request();
if (stretch_time) {
stretch_timer :> stretch_end;
if (stretch_start > stretch_end)
stretch_time = (unsigned)((unsigned)(4294967295U - stretch_start) + stretch_end);
else stretch_time = stretch_end - stretch_start;
if (stretch_time < 175) delay_ticks(175 - stretch_time);
}
}
// Release the clock
p_scl :> void;
break;I still run 166-250khz clock but it does seem to be slightly more time at 166khz than 250khz than before but works at startup. If anybody else decides to communicate with a pi using the hardware i2c ports this is essentially what needs to be done and you need to work around the limitations.
Next is just the complete slave source file I've edited.
Code: Select all
// Copyright (c) 2016, XMOS Ltd, All rights reserved
#include <i2c.h>
#include <xs1.h>
#include <xclib.h>
enum i2c_slave_state {
WAITING_FOR_START_OR_STOP,
READING_ADDR,
ACK_ADDR,
ACK_WAIT_HIGH,
ACK_WAIT_LOW,
IGNORE_ACK,
MASTER_WRITE,
MASTER_READ
};
[[combinable]]
void i2c_slave(client i2c_slave_callback_if i,
port p_scl, port p_sda,
uint8_t device_addr)
{
set_core_fast_mode_on();
set_core_high_priority_on();
enum i2c_slave_state state = WAITING_FOR_START_OR_STOP;
enum i2c_slave_state next_state = WAITING_FOR_START_OR_STOP;
timer stretch_timer;
unsigned trans_type = 0, stretch_start = 0, stretch_end = 0, stretch_time = 0;
int sda_val = 0;
int scl_val;
int bitnum = 0;
int data;
int rw = 0;
int stop_bit_check = 0;
int ignore_stop_bit = 1;
p_sda when pinseq(1) :> void;
while (1) {
select {
case i.shutdown():
return;
case state != WAITING_FOR_START_OR_STOP => p_scl when pinseq(scl_val) :> void:
switch (state) {
case READING_ADDR:
// If clock has gone low, wait for it to go high before doing anything
if (scl_val == 0) {
scl_val = 1;
break;
}
int bit;
p_sda :> bit;
if (bitnum < 7) {
data = (data << 1) | bit;
bitnum++;
scl_val = 0;
break;
}
// We have gathered the whole device address sent by the master
if (data != device_addr) {
state = IGNORE_ACK;
} else {
state = ACK_ADDR;
rw = bit;
}
scl_val = 0;
break;
case IGNORE_ACK:
// This request is not for us, ignore the ACK
next_state = WAITING_FOR_START_OR_STOP;
scl_val = 1;
state = ACK_WAIT_HIGH;
break;
case ACK_ADDR:
// Callback to the application to determine whether to ACK
// or NACK the address.
int ack;
if (rw) {
ack = i.ack_read_request();
} else {
i.start_write_request();
ack = i.ack_write_request();
}
ignore_stop_bit = 0;
if (ack == I2C_SLAVE_NACK) {
// Release the data line so that it is pulled high
p_sda :> void;
next_state = WAITING_FOR_START_OR_STOP;
} else {
// Drive the ACK low
p_sda <: 0;
if (rw) {
next_state = MASTER_READ;
} else {
next_state = MASTER_WRITE;
}
}
scl_val = 1;
state = ACK_WAIT_HIGH;
if (rw) {
if (trans_type != 0) {
p_scl <: 0;
stretch_timer :> stretch_start;
trans_type = 0;
stretch_time = 1;
} else stretch_time = 0;
i.start_read_request();
if (stretch_time) {
stretch_timer :> stretch_end;
if (stretch_start > stretch_end) stretch_time = (unsigned)((unsigned)(4294967295U - stretch_start) + stretch_end);
else stretch_time = stretch_end - stretch_start;
if (stretch_time < 175) delay_ticks(175 - stretch_time);
}
}
// Release the clock
p_scl :> void;
break;
case ACK_WAIT_HIGH:
// Rising edge of clock, hold ack to the falling edge
state = ACK_WAIT_LOW;
scl_val = 0;
break;
case ACK_WAIT_LOW:
// ACK done, release the data line
p_sda :> void;
if (next_state == MASTER_READ) {
scl_val = 0;
} else if (next_state == MASTER_WRITE) {
data = 0;
scl_val = 1;
} else { // WAITING_FOR_START_OR_STOP
sda_val = 0;
}
state = next_state;
bitnum = 0;
break;
case MASTER_READ:
if (scl_val == 1) {
// Rising edge
if (bitnum == 8) {
// Sample ack from master
int bit;
p_sda :> bit;
if (bit) {
// Master has NACKed so the transaction is finished
state = WAITING_FOR_START_OR_STOP;
sda_val = 0;
} else {
bitnum = 0;
scl_val = 0;
}
} else {
// Wait for next falling edge
scl_val = 0;
bitnum++;
}
} else {
// Falling edge, drive data
if (bitnum < 8) {
if (bitnum == 0) {
data = i.master_requires_data();
// Data is transmitted MSB first
data = bitrev(data) >> 24;
// Send first bit of data
p_sda <: data & 0x1;
} else {
p_sda <: data & 0x1;
}
data >>= 1;
} else {
// Release the bus for the master to be able to ACK/NACK
p_sda :> void;
}
scl_val = 1;
}
break;
case MASTER_WRITE:
if (scl_val == 1) {
// Rising edge
int bit;
p_sda :> bit;
data = (data << 1) | (bit & 0x1);
if (bitnum == 0) {
if (bit) {
sda_val = 0;
} else {
sda_val = 1;
}
// First bit could be a start or stop bit
stop_bit_check = 1;
}
scl_val = 0;
bitnum++;
} else {
// Falling edge
// Not a start or stop bit
stop_bit_check = 0;
if (bitnum == 8) {
i.start_master_write();
trans_type = ((data == 0x01) || (data == 0x41)) ? data : 0;
int ack = i.master_sent_data(data);
if (ack == I2C_SLAVE_NACK) {
// Release the data bus so it is pulled high to signal NACK
p_sda :> void;
} else {
// Drive data bus low to signal ACK
p_sda <: 0;
}
state = ACK_WAIT_HIGH;
}
scl_val = 1;
}
break;
}
break;
case (state == WAITING_FOR_START_OR_STOP) || stop_bit_check =>
p_sda when pinseq(sda_val) :> void:
if (sda_val == 1) {
// SDA has transitioned from low to high, if SCL is high
// then it is a stop bit.
int val;
p_scl :> val;
if (val) {
if (!ignore_stop_bit) {
i.stop_bit();
}
state = WAITING_FOR_START_OR_STOP;
ignore_stop_bit = 1;
stop_bit_check = 0;
}
sda_val = 0;
} else {
// SDA has transitioned from high to low, if SCL is high
// then it is a start bit.
int val;
p_scl :> val;
if (val == 1) {
state = READING_ADDR;
bitnum = 0;
data = 0;
scl_val = 0;
stop_bit_check = 0;
} else {
sda_val = 1;
}
}
break;
}
}
}