rp2040: implementation of i2c target mode #3301
Conversation
|
Very cool! Also see conversation in #1904 for some of what we were previously discussing. |
| // | ||
| // The return value should be one or more bytes to return to the | ||
| // controller (when evt is I2CRequest), else it is ignored. | ||
| type I2CHandler func(evt I2CTargetEvent, r byte) []byte |
There was a problem hiding this comment.
Appreciate all the work that went into this, awesome stuff!
Just have one question regarding the I2CHandler- it receives one byte, which I presume is the last read. So is this called one time for each byte received? Maybe worth documenting that each byte in the returned byte slice is added to the buffer (which is max 16 length)?
There was a problem hiding this comment.
Yep - it's one byte at a time for read. We could aggregate into an array, but then it needs a pre-allocated buffer, so some decision on the max number of permitted bytes from the controller.
To make it device neutral, I could define a constant (MaxI2CHandlerResponseBytes?) that is set to the size of the underlying hardware buffer.
There was a problem hiding this comment.
Yeah I think I'd avoid not writing the bytes over wire immediately (based on intuition alone though). The constant sounds like a good idea to me
| } | ||
|
|
||
| type I2C struct { | ||
| Bus *rp.I2C0_Type | ||
| restartOnNext bool | ||
| targetMode bool |
There was a problem hiding this comment.
Perhaps this would be better as:
type I2CMode int
const (
I2CModeController I2CMode = iota
I2CModeTarget
)and then this can be defined as:
type I2C struct {
...
mode I2CMode
...
}There was a problem hiding this comment.
Yep - I prefer this for selecting the mode
|
Thanks for the comments. I'm working on an nrf52 implementation and thinking about re-doing the API on the basis of that. Something like I2C.WaitForEvent and I2C.Reply, so app logic would look more like this... var (
// Max read from Controller is 3 bytes (addr + 16-bit register content)
readBuf := make([]byte, 3)
// Max write to Controller is 2 bytes (16-bit register content)
writeBuf := make([]byte, 2)
// 256 16-bit registers simulated
regs := make([]uint16, 256)
ptr := 0
)
i2c.Listen(addr)
for {
evt, count, err := i2c.WaitForEvent(readBuf, timeout)
if err != nil {
// handle error
}
if evt == machine.I2CTargetWrite {
ptr := buf[0]
if count > 2 {
reg[ptr] = uint16(readBuf[1] << 8) | uint16(readBuf[2])
ptr++
}
} else if evt == machine.I2CTargetRead {
writeBuf[0] = byte(reg[ptr] >> 8)
writeBuf[1] = byte(reg[ptr] & 0xff)
ptr++
i2c.Reply(writeBuf)
} else {
panic("invalid i2c event")
}
}One problem I see is that although Alternative would be to have I think this pattern is better since it avoids app logic from having to honor interrupt handler restrictions and allows implementations in the |
|
I appreciate the thought you put in API design to get it right :)
Thanks a lot! Having more than one implementation helps to avoid tying the API to a particular implementation.
There are many peripherals that run into this. Essentially, you want to a semaphore: block inside the goroutine, and unblock from within the interrupt. We have some support for that through channels but it requires some chip specific support (which isn't implemented for the RP2040 yet) and channels are a somewhat heavy data structure for this when all we want is a simple semaphore. |
Implements i2c target mode on RP2040
Includes example where a single RP2040 can implement both controller and target on it's two I2C controllers (requires them to be physically wired together).
The API is intentionally low-level since not all devices implement a 'memory map' over I2C:
I'd envisage libraries outside Tinygo could provide simpler abstractions (e.g. an interrupt-safe 8-bit / 16-bit registers, etc)
I've updated terminology to controller/target. The RP2040 spec continues to use the terms 'master/slave', but I thought it preferable to align on the 'controller/target' terminology in TinyGo as this is the first i2c target implementation. controller/target is used in the latest 'official' spec for I2C: UM10204 Rev. 7.0 — 1 October 2021, which includes this note: Updated the terms "master/slave" to "controller/target" throughout to align with MIPI I3C specification and NXP's Inclusive Language Project