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

i2c has incorrect timing, causing i2c communication to not function #365

Open
phsdv opened this issue Jul 3, 2024 · 1 comment
Open

Comments

@phsdv
Copy link

phsdv commented Jul 3, 2024

According to https://www.nxp.com/docs/en/user-guide/UM10204.pdf section 3.1.3: The data on the SDA line must be stable during the HIGH period of the clock. The HIGH or LOW state of the data line can only change when the clock signal on the SCL line is LOW

However in the code (tools/communication/src/i2c.cpp) both SDA and SCL are changing at the same time, which is a violation of the standard. Therefore i2c communication does not work, at least not with the devices I tested.

In writeBit for example, the SCL must be low, then SDA can change, and after a little time the SCL can go high, wait half a clock cycle then SCL can go low again.

In below image it can be seen that SCL in SDA change at the same time (using libiio 0.25 and libm2k 0.8.0)
afbeelding

[edit]this concerns the i2c in the tools / digital communications. My code that generated above image is based on https://github.com/analogdevicesinc/libm2k/blob/master/bindings/python/examples/i2c.py
[edit2] clarified which code and fixed a typo 'now' -> 'not'

proposal for improvement:
The code for writeBit in tools/communication/src/i2c.cpp should be something like shown below, as SDA should already be valid before SCL goes high. As there are now 4 changes per sample, samplesPerHalfBit might need to be changed and I renamed it to samplesPerQuartBit. Further it would be clearer if writebyte calls writebit 8 times and then only this change has to be made.

static void writeBit(struct i2c_desc *desc, std::vector<unsigned short> &buffer, bool bit)
{
	auto *m2KI2CDesc = (m2k_i2c_desc *) desc->extra;
	auto samplesPerQuartBit = (unsigned int) (m2KI2CDesc->sample_rate / desc->max_speed_hz) / 4;
	//scl LOW
	for (unsigned int i = 0; i < samplesPerQuartBit; ++i) {
		unsigned short sample = 0;
		if (bit) {
			setBit(sample, m2KI2CDesc->sda);
		}
		buffer.push_back(sample);
	}
	//scl HIGH for 2 quarts
	for (unsigned int i = 0; i < (samplesPerQuartBit*2); ++i) {
		unsigned short sample = 0;
		setBit(sample, m2KI2CDesc->scl);
		if (bit) {
			setBit(sample, m2KI2CDesc->sda);
		}
		buffer.push_back(sample);
	}
	//scl LOW
	for (unsigned int i = 0; i < samplesPerQuartBit; ++i) {
		unsigned short sample = 0;
		if (bit) {
			setBit(sample, m2KI2CDesc->sda);
		}
		buffer.push_back(sample);
	}
}
@phsdv
Copy link
Author

phsdv commented Jul 26, 2024

The following code works correctly for me:

static void writeBit(struct i2c_desc *desc, std::vector<unsigned short> &buffer, bool bit)
{
	auto *m2KI2CDesc = (m2k_i2c_desc *) desc->extra;
	auto samplesPerQuartBit = (unsigned int) (m2KI2CDesc->sample_rate / desc->max_speed_hz) / 4;

	//scl low
	for (unsigned int i = 0; i < samplesPerQuartBit; ++i) {
		unsigned short sample = 0;
		if (bit) {
			setBit(sample, m2KI2CDesc->sda);
		}
		buffer.push_back(sample);
	}

	//scl high
	for (unsigned int i = 0; i < ((samplesPerQuartBit*2) ); ++i) { 
		unsigned short sample = 0;
		setBit(sample, m2KI2CDesc->scl);
		if (bit) {
			setBit(sample, m2KI2CDesc->sda);
		}
		buffer.push_back(sample);
	}

	//scl low
	for (unsigned int i = 0; i < samplesPerQuartBit ; ++i) {
		unsigned short sample = 0;
		if (bit) {
			setBit(sample, m2KI2CDesc->sda);
		}
		buffer.push_back(sample);
	}
}

static void writeByte(struct i2c_desc *desc, std::vector<unsigned short> &buffer, uint8_t byte)
{
	for (int i = 0; i < 8; i++) {
	    writeBit(desc, buffer, getBit(byte, 7 - i));
	}
}

Adrian-Stanea added a commit that referenced this issue Aug 2, 2024
- Modified SDA to transition 1/4 Tbit earlier, ensuring data stability when SCL is high.
- Adjusted the early transition in the start condition to leverage the open drain configuration of I2C, which ensures both lines are high initially.

Resolves issue #365

Signed-off-by: Adrian Stanea <Adrian.Stanea@analog.com>
AlexandraTrifan pushed a commit that referenced this issue Aug 22, 2024
- Modified SDA to transition 1/4 Tbit earlier, ensuring data stability when SCL is high.
- Adjusted the early transition in the start condition to leverage the open drain configuration of I2C, which ensures both lines are high initially.

Resolves issue #365

Signed-off-by: Adrian Stanea <Adrian.Stanea@analog.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant