potential issues in i2c_read_blocking_internal

[fivdi]

There may be two issues in i2c_read_blocking_internal.

Issue 1 The first potential issue is these two lines of code:

            abort_reason = i2c->hw->tx_abrt_source;
            abort = (bool) i2c->hw->clr_tx_abrt;

The first line of code copies the value of the IC_TX_ABRT_SOURCE register to abort_reason. The second line clears the same register, that is, the IC_TX_ABRT_SOURCE register. If the hardware decides to set a bit in the IC_TX_ABRT_SOURCE register after the first line has been executed, but before the second line is executed, the reason for the aborted transfer will be lost. I don't have an example program that can be used to demonstrate this, it's an assumption.

Issue 2 The second potential issue is these two lines of code:

        while (!i2c_get_write_available(i2c))
            tight_loop_contents();

I'm not 100% sure but I don't think this loop serves any purpose. Can it be removed?

[lurch]

I'm not 100% sure but I don't think this loop serves any purpose.

Maybe it was supposed to be i2c_get_read_available? That's just a (very uninformed) guess though :wink: :shrug:

[fivdi]

I'm not 100% sure but I don't think this loop serves any purpose.

Maybe it was supposed to be i2c_get_read_available? That's just a (very uninformed) guess though 😉 🤷

I don't think so, i2c_get_read_available is called here.

[lurch]

I don't think so, i2c_get_read_available is called here.

On a side-note, I wonder if tight_loop_contents() ought to be being used inside that do ... while loop, if timeout_check is NULL? :shrug:

[fivdi]

On a side-note, I wonder if tight_loop_contents() ought to be being used inside that do ... while loop, if timeout_check is NULL? 🤷

This is likely to be true. I think it should be used irrespective of of whether timeout_check is NULL, like here in i2c_write_blocking_internal.