Skip to content

i2c_sam0 LOWTOUT is not functional #21232

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.

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

Closed
sslupsky opened this issue Dec 6, 2019 · 1 comment
Closed

i2c_sam0 LOWTOUT is not functional #21232

sslupsky opened this issue Dec 6, 2019 · 1 comment
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale

Comments

@sslupsky
Copy link
Contributor

sslupsky commented Dec 6, 2019

Describe the bug
When using the i2c_sam0 driver, SCL remains low indefinitely after a bus transaction. SCL should be released when a bus transaction completes. LOWTOUT is intended to guard against situations where SCL is held low too long.

The i2c_sam0 driver enables LOWTOUT (SCL low timeout). However, the driver does not initialize GCLK_SERCOM_SLOW which is required to enable the timeout (see 28.5.3 and 28.6.3.1).

To Reproduce
Steps to reproduce the behavior:

  1. use blinky example
  2. Add opt3001 i2c peripheral (actually, any i2c peripheral)
  3. make
  4. observe SCL
  5. See error

Expected behavior
SCL will be released after the LOWTOUT timeout.

I applied the following patch to i2c_sam0_initialize() to test the expected behaviour:

    GCLK->CLKCTRL.reg = GCLK_CLKCTRL_ID_SERCOMX_SLOW | GCLK_CLKCTRL_GEN_GCLK2 |
                GCLK_CLKCTRL_CLKEN;

Impact
When SCL is held low, the pull up resistors on the bus consume power on the order of 500uA or more (depending on the value of the pull up resistor). This bug rules out any low power applications that require the i2c driver.

@sslupsky sslupsky added the bug The issue is a bug, or the PR is fixing a bug label Dec 6, 2019
@jhedberg jhedberg added the priority: medium Medium impact/importance bug label Dec 10, 2019
@pabigot pabigot added the has-pr label May 11, 2020
@carlescufi carlescufi added priority: low Low impact/importance bug and removed priority: medium Medium impact/importance bug labels May 26, 2020
sslupsky added a commit to sslupsky/zephyr that referenced this issue Jul 10, 2020
Optimizations:
When using system power management, the bus transactions
can be interrupted by low power modes which causes excessive
power consumption and can cause the transactions to terminate
before completed. Further, other interrupts can cause bus transaction
anomalies.

When using the RESTART flag, the MB and SB bits must be cleared.

This commit prevents interruption of the bus during low power modes and
correctly clears the MB or SB flags when a bus transaction is restarted.
Also fixes an initialization problem with the INACTOUT timer and ensures
the LOWTOUT is properly configured when using i2c_configure().

The GCLK_SERCOM_SLOW clock is used to time the i2c
time-outs and must be configured to use a 32KHz oscillator.
See 28.6.3.1

This commit configures GCLK4 to 32768 Hz

Fixes zephyrproject-rtos#21711, zephyrproject-rtos#21549, zephyrproject-rtos#21233, zephyrproject-rtos#21232, zephyrproject-rtos#21114, zephyrproject-rtos#21092

A problem can arise when back to back transactions occur before
the STOP transaction is complete.

This commit also introduces a busy wait for the STOP to complete
with an timeout in the event there is a bus failure.

Signed-off-by: Steven Slupsky <[email protected]>
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants