Documentation Mismatch in I2CMaster Regarding Output Enable Behavior in bitbang.py

[Haron123]

The issue is found in the documentation of the I2CMaster within the litex/soc/cores/bitbang.py file, at
line 44.

description="Output Enable - if 0, both the SCL and SDA output drivers are disconnected."),

However, when I set OE to 0, it appears that only the SDA line is disconnected, while the SCL line remains connected. This doesn't match with the documentation

Suggestion:

It seems more intuitive that only the SDA line would be disconnected when OE is 0. Whatever the intetion was though the documentation should match the functionality.

If anyone would like to check it themselves id appreciate it, i doubt theres much to do wrong with setting the OE register to 0 and toggling SCL so i hope it isn't a mistake from my side

Further Information/My Code

Pin Functionality Code:

inline void I2C::setSDA(PinState state)
{
	switch(state)
	{
		case PinState::OFF:
			*CSR_I2C0_W_ADDR_REG &= ~(1 << CSR_I2C0_W_SDA_OFFSET);
			break;

		case PinState::ON:
			*CSR_I2C0_W_ADDR_REG |= (1 << CSR_I2C0_W_SDA_OFFSET);
			break;

		default:
			//Ignore
			break;
	}
}

inline void I2C::setSCL(PinState state)
{
	switch(state)
	{
		case PinState::OFF:
			*CSR_I2C0_W_ADDR_REG &= ~(1 << CSR_I2C0_W_SCL_OFFSET);
			break;

		case PinState::ON:
			*CSR_I2C0_W_ADDR_REG |= (1 << CSR_I2C0_W_SCL_OFFSET);
			break;

		default:
			//Ignore
			break;
	}
}

inline void I2C::setOE(PinState state)
{
	switch(state)
	{
		case PinState::OFF:
			*CSR_I2C0_W_ADDR_REG &= ~(1 << CSR_I2C0_W_OE_OFFSET);
			break;

		case PinState::ON:
			*CSR_I2C0_W_ADDR_REG |= (1 << CSR_I2C0_W_OE_OFFSET);
			break;

		default:
			//Ignore
			break;
	}
}

Test Code:

setOE(PinState::OFF);

while(1)
{
  setSCL(PinState::OFF);
  setSDA(PinState::OFF);
  delayUS(1000);
  
  setSCL(PinState::ON);
  setSDA(PinState::ON);
  delayUS(1000);
}

Result:

Only the SCL Line Toggles

[AndrewD]

Yes, looking at the code the documentation doesn't match. Maybe it was this way but the core was changed to fix a specific case?

[Haron123]

Might be, it wouldn't make much sense to me the way it is documented, since being able to use SCL while SDA is High Impedance is a common usecase. Either way it confused me that it didnt match while writing code for it.

[AndrewD]

Not that sda is driven open drain, so it's high impedance when sda = 1.
I don't actually see the need for oe at all.

Also note that there is now a separate ic2 core that does all this for you and provides a slightly higher level interface. See i2c.py: there is a python demonstration of this in a recent PR for me.

[Haron123]

Are you sure SDA will be Open drain on any FPGA? So far ive only seen Tristate output support.

Also thanks for the pointer to the seperate i2c core, ive seen it in cores but i had a few problems with it(only seeing EV registers) ill check out the Demonstration incase i used it wrong.

[AndrewD]

See #2022 for the python usage of the simple core.
Also #2026 has a very interesting i2c core that looks very promising.

[AndrewD]

Are you sure SDA will be Open drain on any FPGA? So far ive only seen Tristate output support.

It implements open drain-like "emulation" with tristate gpio.

The other core is memory mapped: EV registers are just for the interrupt (if used). Usage should be more obvious from that python driver, our nuttx driver isn't upstream (yet).

[Haron123]

Im quite new with Litex so i am a bit confused. How exactly would i integrate the I2C Core you mentioned to use it with my software?

[AndrewD]

In the target you need to use add_slave() to connect the i2c memory mapped interface to the soc. I'll aim to post an example tomorrow.

[AndrewD]

Something like the below will be close:

self.bus.add_slave(name="i2c", 
slave=self.i2c.bus, region=SoCRegion(
             origin = 0xe000_0000,
             size   = 0x1000,
         ))

[AndrewD]

This is a complete example integration:

from litex.soc.cores.i2c import I2CMaster

self.i2c = I2CMaster(platform.request("i2c", 0))
self.bus.add_slave(
    name="i2c",
    slave=self.i2c.bus,
    region=SoCRegion(
          origin=0x8000_0000,
          size=8,
          cached=False,
    ),
)
self.irq.add(name, use_loc_if_exists=True)

Now in csr.csv you wiil see a i2c memory region and an i2c csr for irq.

1. Read/Write at I2C_0_BASE+4 to get/set the i2c clock
2. read/write I2C_0_BASE+0 to interact with the interface. litex/tools: Add RemoteI2C #2022 shows the interactions.

[Haron123]

Thanks a lot, ill try it out whenever i find the time for it