Skip to content

RP2040 i2c: fix crash when encountering abort while waiting for stop condition #3770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deadprogram merged 1 commit into from
Jun 4, 2023
Merged

RP2040 i2c: fix crash when encountering abort while waiting for stop condition #3770

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 81 additions & 16 deletions src/machine/machine_rp2040_i2c.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,9 @@ type I2CConfig struct {
}

type I2C struct {
Bus *rp.I2C0_Type
restartOnNext bool
mode I2CMode
txInProgress bool
Bus *rp.I2C0_Type
mode I2CMode
txInProgress bool
}

var (
Expand Down Expand Up @@ -236,7 +235,6 @@ func (i2c *I2C) init(config I2CConfig) error {
if err := i2c.disable(); err != nil {
return err
}
i2c.restartOnNext = false

i2c.mode = config.Mode

Expand Down Expand Up @@ -306,16 +304,17 @@ func (i2c *I2C) tx(addr uint8, tx, rx []byte, timeout_us uint64) (err error) {
i2c.Bus.IC_TAR.Set(uint32(addr))
i2c.enable()
abort := false
var abortReason uint32
var abortReason i2cAbortError
txStop := rxlen == 0
for txCtr := 0; txCtr < txlen; txCtr++ {
if abort {
break
}
first := txCtr == 0
last := txCtr == txlen-1 && rxlen == 0
i2c.Bus.IC_DATA_CMD.Set(
(boolToBit(first && i2c.restartOnNext) << rp.I2C0_IC_DATA_CMD_RESTART_Pos) |
(boolToBit(last) << rp.I2C0_IC_DATA_CMD_STOP_Pos) |
(boolToBit(first) << rp.I2C0_IC_DATA_CMD_RESTART_Pos) |
(boolToBit(last && txStop) << rp.I2C0_IC_DATA_CMD_STOP_Pos) |
uint32(tx[txCtr]))

// Wait until the transmission of the address/data from the internal
Expand Down Expand Up @@ -356,6 +355,9 @@ func (i2c *I2C) tx(addr uint8, tx, rx []byte, timeout_us uint64) (err error) {
// to take care of the abort.
for !i2c.interrupted(rp.I2C0_IC_RAW_INTR_STAT_STOP_DET) {
if ticks() > deadline {
if abort {
return abortReason
}
return errI2CWriteTimeout
}

Expand All @@ -365,6 +367,16 @@ func (i2c *I2C) tx(addr uint8, tx, rx []byte, timeout_us uint64) (err error) {
}
}

// Midway check for abort. Related issue https://github.com/tinygo-org/tinygo/issues/3671.
// The root cause for an abort after writing registers was "tx data no ack" (abort code=8).
// If the abort code was not registered then the whole peripheral would remain in disabled state forever.
abortReason = i2c.getAbortReason()
if abortReason != 0 {
i2c.clearAbortReason()
abort = true
}

rxStart := txlen == 0
if rxlen > 0 && !abort {
for rxCtr := 0; rxCtr < rxlen; rxCtr++ {
first := rxCtr == 0
Expand All @@ -373,14 +385,14 @@ func (i2c *I2C) tx(addr uint8, tx, rx []byte, timeout_us uint64) (err error) {
gosched()
}
i2c.Bus.IC_DATA_CMD.Set(
boolToBit(first && i2c.restartOnNext)<<rp.I2C0_IC_DATA_CMD_RESTART_Pos |
boolToBit(first && rxStart)<<rp.I2C0_IC_DATA_CMD_RESTART_Pos |
boolToBit(last)<<rp.I2C0_IC_DATA_CMD_STOP_Pos |
rp.I2C0_IC_DATA_CMD_CMD) // -> 1 for read

for !abort && i2c.readAvailable() == 0 {
abortReason = i2c.getAbortReason()
i2c.clearAbortReason()
if abortReason != 0 {
i2c.clearAbortReason()
abort = true
}
if ticks() > deadline {
Expand All @@ -407,7 +419,7 @@ func (i2c *I2C) tx(addr uint8, tx, rx []byte, timeout_us uint64) (err error) {
// Address acknowledged, some data not acknowledged
fallthrough
default:
err = makeI2CAbortError(abortReason)
err = abortReason
}
}
return err
Expand Down Expand Up @@ -534,8 +546,8 @@ func (i2c *I2C) clearAbortReason() {
// getAbortReason reads IC_TX_ABRT_SOURCE register.
//
//go:inline
func (i2c *I2C) getAbortReason() uint32 {
return i2c.Bus.IC_TX_ABRT_SOURCE.Get()
func (i2c *I2C) getAbortReason() i2cAbortError {
return i2cAbortError(i2c.Bus.IC_TX_ABRT_SOURCE.Get())
}

// returns true if RAW_INTR_STAT bits in mask are all set. performs:
Expand All @@ -554,9 +566,62 @@ func (b i2cAbortError) Error() string {
return "i2c abort, reason " + itoa.Uitoa(uint(b))
}

//go:inline
func makeI2CAbortError(reason uint32) error {
return i2cAbortError(reason)
func (b i2cAbortError) Reasons() (reasons []string) {
if b == 0 {
return nil
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_7B_ADDR_NOACK != 0 {
reasons = append(reasons, "7-bit address no ack")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_10ADDR1_NOACK != 0 {
reasons = append(reasons, "10-bit address first byte no ack")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_10ADDR2_NOACK != 0 {
reasons = append(reasons, "10-bit address second byte no ack")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_TXDATA_NOACK != 0 {
reasons = append(reasons, "tx data no ack")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_GCALL_NOACK != 0 {
reasons = append(reasons, "general call no ack")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_GCALL_READ != 0 {
reasons = append(reasons, "general call read")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_HS_ACKDET != 0 {
reasons = append(reasons, "high speed ack detect")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_SBYTE_ACKDET != 0 {
reasons = append(reasons, "start byte ack detect")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_HS_NORSTRT != 0 {
reasons = append(reasons, "high speed no restart")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_SBYTE_NORSTRT != 0 {
reasons = append(reasons, "start byte no restart")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_10B_RD_NORSTRT != 0 {
reasons = append(reasons, "10-bit read no restart")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_MASTER_DIS != 0 {
reasons = append(reasons, "master disabled")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ARB_LOST != 0 {
reasons = append(reasons, "arbitration lost")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_SLVFLUSH_TXFIFO != 0 {
reasons = append(reasons, "slave flush tx fifo")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_SLV_ARBLOST != 0 {
reasons = append(reasons, "slave arbitration lost")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_SLVRD_INTX != 0 {
reasons = append(reasons, "slave read while inactive")
}
if b&rp.I2C0_IC_TX_ABRT_SOURCE_ABRT_USER_ABRT != 0 {
reasons = append(reasons, "user abort")
}
return reasons
}

//go:inline
Expand Down