tinygo-org / tinygo

Go compiler for small places. Microcontrollers, WebAssembly (WASM/WASI), and command-line tools. Based on LLVM.
https://tinygo.org
Other
15.45k stars 911 forks source link

Wio Terminal does not work well for I2C #3820

Closed sago35 closed 1 year ago

sago35 commented 1 year ago

It appears to have become no longer readable after the following changes https://github.com/tinygo-org/tinygo/pull/3492

If enableCache() is commented out, the correct values can still be read in 0.28.1.

# expected (v0.27.0)
$ tinygo flash --target wioterminal --monitor ./chap05/08-i2c/01
Connected to COM38. Press Ctrl-C to exit.
Who am I : 0x33
# actual (v0.28.1)
$ tinygo flash --target wioterminal --monitor ./chap05/08-i2c/01
Connected to COM38. Press Ctrl-C to exit.
Who am I : 0x00
package main

import (
    "fmt"
    "machine"
    "time"
)

func main() {
    i2c := machine.I2C0
    i2c.Configure(machine.I2CConfig{
        SCL: machine.SCL0_PIN,
        SDA: machine.SDA0_PIN,
    })
    time.Sleep(2 * time.Second)
    data := []byte{0}
    err := i2c.ReadRegister(0x18, 0x0F, data)
    if err != nil {
        // error
    }
    fmt.Printf("Who am I : 0x%02X\r\n", data[0])
    select {}
}
deadprogram commented 1 year ago

You could try disabling the cache during i2c reads. Please see https://github.com/adafruit/circuitpython/pull/819/files#diff-bd21491825c3dabc01a50d87e235d8a8338b5f8935705e79173a8c3a36b01e4cR168

sago35 commented 1 year ago

It was verified to work correctly with enableCache / disableCache. However, after a little more investigation, I found that the problem was with the i2cTimeout value.

The original code waited 208.276us. With Cache enabled, it waits only 75.056us.

https://github.com/tinygo-org/tinygo/blob/v0.28.1/src/machine/machine_atsamd51.go#L1247-L1253

The current i2cTimeout is set to 1000. As a proposed fix, the i2cTimeout should be 1000 * 208 / 75 (=> 2800).

sago35 commented 1 year ago

sdcard and qspi have not yet been confirmed. Maybe disableCache() is needed.

deadprogram commented 1 year ago

However, after a little more investigation, I found that the problem was with the i2cTimeout value.

That makes a lot more sense.

As a proposed fix, the i2cTimeout should be 1000 * 208 / 75 (=> 2800).

That sounds very reasonable.

deadprogram commented 1 year ago

sdcard and qspi have not yet been confirmed. Maybe disableCache() is needed.

From my reading, that is absolutely going to be needed for QSPI.

aykevl commented 1 year ago

Both 208µs and 75µs seem rather short to me, perhaps bumping it to 1ms or so would be better? Also see https://github.com/tinygo-org/tinygo/pull/3833 where I use an even larger timeout value.

deadprogram commented 1 year ago

Closing since this is part of release 0.29.0 thank you everyone!