Infinite loop in i2c example

[Dicklessgreat]

In my environment, I2C example does not reach to loop .
It stucks at

rp-hal/rp2040-hal/src/i2c/controller.rs

Line 219 in 0f5bc07

while self.i2c.ic_raw_intr_stat.read().tx_empty().is_inactive() {}

that causes infinite loop.
I simply did git clone and cargo run --example i2c --features="critical-section-impl"
Am I missing some hidden settings?

I'm working on Windows10, and using Raspberry Pi Pico.

[ithinuel]

It could be that your hardware setup forces the scl pin low which would prevent the controller from sending the address byte, causing this issue.

[Dicklessgreat]

Thanks.
I'm noob in embedded development, so could you elaborate "hardware setup"?
From what settings interface, should I set the pin "high"?

[jannic]

In the i2c example, gpio19 is configured as the SCL pin. What's connected to that pin in your hardware?

[Dicklessgreat]

Ah...I got it. I'm sorry, I didn't know I2C example need destination hardware.
currently I have no hardware other than pico that has I2C interface.
Though it is a bit different from the title, at first I tried to do a "loopback" using the two I2C ports on the RasPi Pico. However, I still got the same infinite loop and it didn't work.(And afterward, I download rp-hal repo and did cargo run --example i2c --features="critical-section-impl", which cause same infinite loop. That's why I opened an issue)
Can you please tell me if loopback is possible and how to do it?
I wrote this code after the i2c example.

    // Configure two pins as being I²C, not GPIO
    let sda1_pin = pins.gpio18.into_function::<hal::gpio::FunctionI2C>();
    let scl1_pin = pins.gpio19.into_function::<hal::gpio::FunctionI2C>();
    let sda0_pin = pins.gpio20.into_function::<hal::gpio::FunctionI2C>();
    let scl0_pin = pins.gpio21.into_function::<hal::gpio::FunctionI2C>();

    // Create the I²C drive, using the two pre-configured pins. This will fail
    // at compile time if the pins are in the wrong mode, or if this I²C
    // peripheral isn't available on these pins!
    let mut i2c1 = hal::I2C::i2c1(
        pac.I2C1,
        sda1_pin,
        scl1_pin, 
        400.kHz(),
        &mut pac.RESETS,
        &clocks.system_clock,
    );

    let mut i2c0 = hal::I2C::i2c0(
        pac.I2C0,
        sda0_pin,
        scl0_pin, 
        400.kHz(),
        &mut pac.RESETS,
        &clocks.system_clock,
    );

    // Write three bytes to the I²C device with 7-bit address 0x2C
    i2c1.write(0x2c, &[1, 2, 3]).unwrap();

    let mut buffer = [0; 3];
    i2c0.read(0x2c, &mut buffer).unwrap();

    assert_eq!(buffer, [1, 2, 3]);
    // Demo finish - just loop until reset

    loop {
        cortex_m::asm::wfi();
    }

[ithinuel]

Hi, indeed you cannot loop-back I2C as you did. It is a bit more complex than UART and requires some extra things.

First of all, to avoid any confusion here's the vocabulary I'll use:

• controller: part in charge of controlling the bus and initiating transactions.
• peripheral: part listening on the bus and replying to transactions
• block: the i2c block the driver interacts with through the memory mapped registers.

In a nutshell:
The i2c bus is meant to be a multi-controllers/multi-peripherals bus. That means on the same scl/SDA pair you can find multiple chip initiating transactions and multiple chips replying to them.

To accommodate for all this but lines are expected to be pulled up, preferably externally from the rp2040.
The strength of the pull-up depends on the frequency you intend to run the bus at (weaker pull-up/stronger resistor = slower transfer speed).
On the rp2040, most pins (but not all) start with a pull-down resistors enabled by default.
You'll want to reconfigure your scl and sda lines to either PullNone or PullUp.

When a controller wants to read or write data it needs to generate a certain start sequence (bringing scl and sda low in a known sequence).
It then

1. sends over the bus the address of the target peripheral (7bits plus 1 read/write marker bit)
2. waits for an acknowledgement from that peripheral and then either:
3. Write each data byte waiting for an ack of each
4. Clock out 8 bits sending an ack after each.

The peripheral is allowed to hold the clock low when data or ack is expected from it in order to get more time to process the request.

When the controller is done talking to the peripheral, it either generates a restart sequence to send another request (to the same or to another peripheral) or a stop sequence to free the bus for any other controller.

The controller should listen to the bus to make sure there's no contention/conflict.

On the rp2040 the block can behave as a controller or a peripheral (but not both at the same time).

I cannot remember if we have an example for the peripheral mode (I'm on my phone right now), will edit this msg if I find one.

[Dicklessgreat]

@ithinuel Thank you for the detailed explanation. I was confused about the difference in meaning between controller/peripheral/block, so this is a great help!

So, Pico has two I2C executor - "I2C0" and "I2C1".
I thought they are completely separated and can be either side "controller" and the other side "peripheral". But you said they can't. Are they internally the same?

[ithinuel]

I2C0 and I2C1 are two independent instances of the same block.

You can have one be a controller while the other is acting as a peripheral.
But when one is a controller, it cannot receive transactions from another controller.

Eg if your Pico's I2C0 is configured as a controller on the bus, another chip (like another pico) on the same bus cannot send it send it commands.
A controller is not "responding" to requests.

To achieve this, you'd need to have your Pico's i2C0 run as a peripheral to listen and respond on the bus and if/when your Pico needs to talk to another chip's peripheral, then your firmware will need to reconfigure i2C0 from peripheral mode to controller, and revert back to peripheral once it's done.

TBH it's a fairly rare usecase, it might be easier for you to have i2c0 as controller and i2c1 as peripheral (or the other way around).

[Dicklessgreat]

Thanks! I think I got it:)

To confirm I'm in right path, let me ask one more.
When I initialize i2c1, like

let sda1_pin = pins.gpio18.into_function::<hal::gpio::FunctionI2C>();
let scl1_pin = pins.gpio19.into_function::<hal::gpio::FunctionI2C>();
let mut i2c1 = hal::I2C::i2c1(
    pac.I2C1,
    sda1_pin,
    scl1_pin, 
    400.kHz(),
    &mut pac.RESETS,
    &clocks.system_clock,
);

the i2c1 is now "controller", right? If so, I didn't know even that;)
And I should find how to run pico's I2Cx as "peripheral", right?

[ithinuel]

That is correct.

Peripheral mode starts with: https://docs.rs/rp2040-hal/latest/rp2040_hal/i2c/struct.I2C.html#method.new_peripheral_event_iterator

[Dicklessgreat]

Thanks.
Now my program is

// Configure two pins as being I²C, not GPIO
let sda1_pin = pins.gpio18.into_function::<hal::gpio::FunctionI2C>();
let scl1_pin = pins.gpio19.into_function::<hal::gpio::FunctionI2C>();
let sda0_pin = pins.gpio20.into_function::<hal::gpio::FunctionI2C>();
let scl0_pin = pins.gpio21.into_function::<hal::gpio::FunctionI2C>();

let mut i2c0 = hal::I2C::new_peripheral_event_iterator(
    pac.I2C0, 
    sda0_pin, 
    scl0_pin, 
    &mut pac.RESETS, 
    0x2c
);

// Create the I²C drive, using the two pre-configured pins. This will fail
// at compile time if the pins are in the wrong mode, or if this I²C
// peripheral isn't available on these pins!
let mut i2c1 = hal::I2C::i2c1(
    pac.I2C1,
    sda1_pin,
    scl1_pin, 
    400.kHz(),
    &mut pac.RESETS,
    &clocks.system_clock,
);

i2c0.next();//I don't know this is necessary or not.

// Write three bytes to the I²C device with 7-bit address 0x2C
i2c1.write(0x2c, &[1, 2, 3]).unwrap();

let mut buffer = [0; 3];
i2c0.read(&mut buffer);

assert_eq!(buffer, [1, 2, 3]);
// Demo finish - just loop until reset

loop {
    cortex_m::asm::wfi();
}

But it still stuck at the same point(write_internal() while self.i2c.ic_raw_intr_stat.read().tx_empty().is_inactive() {}).
How can I start session?

[jannic]

Could you perhaps try if this change to the example helps?

diff --git a/rp2040-hal/examples/i2c.rs b/rp2040-hal/examples/i2c.rs
index 54b815f..f149a77 100644
--- a/rp2040-hal/examples/i2c.rs
+++ b/rp2040-hal/examples/i2c.rs
@@ -75,8 +75,8 @@ fn main() -> ! {
     );
 
     // Configure two pins as being I²C, not GPIO
-    let sda_pin = pins.gpio18.into_function::<hal::gpio::FunctionI2C>();
-    let scl_pin = pins.gpio19.into_function::<hal::gpio::FunctionI2C>();
+    let sda_pin = pins.gpio18.into_function::<hal::gpio::FunctionI2C>().into_pull_type::<hal::gpio::PullUp>();
+    let scl_pin = pins.gpio19.into_function::<hal::gpio::FunctionI2C>().into_pull_type::<hal::gpio::PullUp>();
     // let not_an_scl_pin = pins.gpio20.into_function::<hal::gpio::FunctionI2C>();
 
     // Create the I²C drive, using the two pre-configured pins. This will fail

It looks like since commit b087c0c ("Rework GPIO API and fix a few inconsistencies (#585)"), the internal pull-ups are no longer automatically activated when I2C is selected.
I don't know if this was intentional. The datasheet strongly suggests that external pull-ups should be used. @ithinuel, that was your commit. Do you remember how it should be used?

At least, we should fix the example.

[Dicklessgreat]

Thanks @jannic ! It works!!!
In my "loopback" case I set both(controller and peripheral) side of sdax_pin and sclx_pin as .into_pull_type::<hal::gpio::PullUp>().
Then it works.

Though my problem has soleved, it seems like tasks remain to fix the example.
So I leave this issue open, but please feel free to close when you(authors) feel it should be to.

[jannic]

Yes, definitely keep the ticket open. I'm not sure how the solution will look like, but at least we need to document the changed behavior and update the example.

In most (if not all) cases, internal pull-ups should be enabled on i2c pins. Even though stronger external pull-ups are recommended, and with them, the internal ones are no longer needed, they wouldn't hurt either.

However I'd like to have some way to enable the i2c interface without the internal pull-ups. Not because I have a particular use case, but just because I think in the embedded space in general, there are many unusual approaches where it makes sense to do something differently than usual.