[Sival, i2c] chip_sw_i2c_speed test

[engdoreis]

This PR:

• Optimize the existing i2c DIF and testutils for speed.
• Refactor the pmod:fram test to run with all the i2c instances.
• Add a throughput step to the pmod:fram test.

Fix #19836

[a-will]

I'd argue that this belongs in an i2c_testutils, not the DIFs. It imposes blocking semantics, where it probably ought to allow for some sort of timeout + bytes actually read, which then might require other IPs for measuring. This is target mode, so our device here doesn't have control of timing or whether a transaction will ever complete.

[jwnrt]

These changes look correct to me, thanks @engdoreis. I haven't checked the tests in depth, but if they pass then that's good enough for me.

I agree with @a-will that those blocking difs might be better off in testutils with a timeout.

[engdoreis]

I agree with @a-will that those blocking difs might be better off in testutils with a timeout.

For reference, the dif_spi_host also contains blocking routines for feeding the FIFO and checking status registers:

opentitan/sw/device/lib/dif/dif_spi_host.c

Lines 175 to 193 in 9fe2120

	while (misalignment32_of(ptr) && len > 0) {
	tx_fifo_write8(spi_host, ptr);
	ptr += 1;
	len -= 1;
	}
	// Write complete 32-bit words to the fifo.
	while (len > 3) {
	tx_fifo_write32(spi_host, ptr);
	ptr += 4;
	len -= 4;
	}
	// Clean up any leftover bytes.
	while (len > 0) {
	tx_fifo_write8(spi_host, ptr);
	ptr += 1;
	len -= 1;
	}

[pamaury]

I don't know the i2c block really well but overall this looks good to me.

[a-will]

I agree with @a-will that those blocking difs might be better off in testutils with a timeout.

For reference, the dif_spi_host also contains blocking routines for feeding the FIFO and checking status registers:

opentitan/sw/device/lib/dif/dif_spi_host.c

Lines 175 to 193 in 9fe2120

	while (misalignment32_of(ptr) && len > 0) {
	tx_fifo_write8(spi_host, ptr);
	ptr += 1;
	len -= 1;
	}
	// Write complete 32-bit words to the fifo.
	while (len > 3) {
	tx_fifo_write32(spi_host, ptr);
	ptr += 4;
	len -= 4;
	}
	// Clean up any leftover bytes.
	while (len > 0) {
	tx_fifo_write8(spi_host, ptr);
	ptr += 1;
	len -= 1;
	}

FYI, that is merely a past mistake that should not be used as a reference. That function also should not have been in the DIFs.

However, there is a significant difference between I2C in target mode and spi_host, as I pointed out in the last line: Our I2C device here doesn't have control of timing or whether a transaction will ever complete. By contrast, spi_host is guaranteed to make forward progress to its end state. That is why the i2c functions need to be in testutils and make effective use of timeouts.

[engdoreis]

Our I2C device here doesn't have control of timing or whether a transaction will ever complete.

That makes sense, I'll move the functions to the testutils when working on i2c_dev tests.

[github-actions]

Successfully created backport PR for earlgrey_es_sival:

• Cherry-pick to earlgrey_es_sival: [Sival, i2c] chip_sw_i2c_speed test #21382