zephyr/i2c: Add support for hardware i2c

[MaureenHelm]

Adds support for hardware i2c to the zephyr port. Similar to other ports
such as stm32 and nrf, we only implement the i2c protocol functions
(readfrom and writeto) and defer memory operations (readfrom_mem,
readfrom_mem_into, and writeto_mem) to the software i2c implementation.
This may need to change in the future because zephyr is considering
deprecating its i2c_transfer function in favor of i2c_write_read; in
this case we would probably want to implement the memory operations
directly using i2c_write_read.

Tested with the accelerometer on frdm_k64f and bbc_microbit boards.

[MaureenHelm]

cc: @pfalcon

[pfalcon]

@stinos, I believe appveyor CI support was added by you, right? Can you please look what's wrong with it here - this patch doesn't touch anything outside Zephyr port, while appveyor fails, seemingly producing zero log output. I suggested to force-push to retrigger the build, and appveyor failed in the same way as before.

@MaureenHelm: Perhaps, try to repush again now, maybe that was a bad day for appveyor? Recently posted PRs doesn't show failures.

[stinos]

Seems there was an Appveyor outage about 3 days ago (see https://help.appveyor.com/discussions/problems/22315-builds-are-failing-most-of-the-time-fallback-clouds-stack-is-too-big), should be up again now.

[MaureenHelm]

@pfalcon Thanks for the review. I pushed a new version

[dpgeorge]

Thanks very much @MaureenHelm , a good addition! And thanks @pfalcon for the review. Merged in 2befcb8

[pfalcon]

@MaureenHelm, Thanks for this patch. Sorry, I only now get to look into it at more detail and test. There're also a couple of problem spots (I'm going to fix them in my repo).

From commit message:

This may need to change in the future because zephyr is considering
deprecating its i2c_transfer function in favor of i2c_write_read; in
this case we would probably want to implement the memory operations
directly using i2c_write_read.

I hope you would use the experience in developing this module to try to protect i2c_transfer() availability in Zephyr TSC meetings ;-). There're really too much shufflings and deprecations happening in Zephyr. If it works and offers more flexibility, why deprecate it?

Otherwise, I tested this class on FRDM-K64F with its FXOS8700 sensor (I actually posted on this yesterday, but don't see my comment today, weird).

Here're some commands from my testing session, for future reference:

from machine import I2C
i2c = I2C("I2C_0")
# ID reg
i2c.writeto(29, b"\x0d", False)
i2c.readfrom(29, 1)
b'\xc7'

i2c.readfrom_mem(29, 0x0d, 1)
b'\xc7'

# Magnetometer readings
ubinascii.hexlify(i2c.readfrom_mem(29, 0x33, 6), " ")

[pfalcon]

Postreview: This new snippet is somewhat misplaced, given that the next paragraph reads: "The above code uses an LED location for a FRDM-K64F board (port B, pin 21;
following Zephyr conventions port are identified by "GPIO_x", where x
starts from 0)."

[MaureenHelm]

Where should it go instead?

[pfalcon]

Where should it go instead?

In #4668, I've reordered it so it go after the text related to GPIO.

[pfalcon]

Postreview: this is actually not a good change. Support for I2C (or not) is still a board-specific matter. This change broke CI build for qemu_cortex_m3, which doesn't have I2C support. (Or course, we might argue that Zephyr shouldn't error the build in this case, but we have what we have anyway.)

[MaureenHelm]

Should support for the I2C micropython module be conditional then?

[pfalcon]

Should support for the I2C micropython module be conditional then?

I'm not sure, just switching back to enable it per-board fixed the build, I didn't look further so far. That's what included in #4668.

[MaureenHelm]

This may need to change in the future because zephyr is considering
deprecating its i2c_transfer function in favor of i2c_write_read; in
this case we would probably want to implement the memory operations
directly using i2c_write_read.

I hope you would use the experience in developing this module to try to protect i2c_transfer() availability in Zephyr TSC meetings ;-). There're really too much shufflings and deprecations happening in Zephyr. If it works and offers more flexibility, why deprecate it?

i2c_transfer() isn't well defined and behaves differently on different platforms, particularly around restart conditions. I considered not using it at all in favor of i2c_write_read() instead, but that would make zephyr the only hard i2c implementation in micropython that doesn't defer to soft i2c for some functions. I wasn't sure if that would be acceptable, so I went with the simpler approach for my first PR. I can send another PR if it is.

[dpgeorge]

I considered not using it at all in favor of i2c_write_read() instead, but that would make zephyr the only hard i2c implementation in micropython that doesn't defer to soft i2c for some functions

What's important is that a port does it's best to implement the defined I2C methods. Whether it does that by providing readfrom/writeto C-level functions and using the existing Python wrappers, or by implementing everything from scratch, is less important (except that it's much simpler to implement readfrom/witeto C functions as was done).