xmos/lib_i2c

I2C peripheral library

lib_i2c's Issues

Comment references only working on xcore-200

lib_i2c/lib_i2c/api/i2c.h

Line 411 in f23a024

* supported on xCORE-200 devices.

This should probably be changed to say this will not work on xs-1 or similar rather than specifying what it will work on.

Bug in write_16

https://www.xcore.com/forum/viewtopic.php?f=26&t=4361 lists a bug and a fix on writing 16-bit values.

MSB and LSB have been swapped in write_16.

wscript version number is out of date

How to use lib_i2c when writing an application in C, as opposed to xC?

I read that new applications should favour being written in C/C++, rather than xC, going forwards.

However, lib_i2c only seems to be available for xC projects (most of the code is hidden behind an XC define).

How can I convert the following example in the docs to compile from a C file (specifically the i2c bits; the ports are straightfowards to use)?

port p_scl = XS1_PORT_1E;
port p_sda = XS1_PORT_1F;
int main(void) {
    i2c_master_if i2c[1];
    static const uint8_t target_device_addr = 0x3c;
    par {
        i2c_master(i2c, 1, p_scl, p_sda, 100);
        my_application(i2c[0], target_device_addr);
    }
    return 0;
}

Stop bit image size

stop_bit.png should be scaled to something like 30%. Currently 100%, too big, as introduced in fix 0cd00ce

Description of "other_bits_mask" not very clear

i.e.

• \param other_bits_mask The mask for the other bits of the port to use

•                       when driving it.  Note that, on occassions,
  

•                       the other bits are left to float, so external
  

•                       resistors shall be used to reinforce the default
  

•                       value
  

I had to look at source code to be sure what value to use

i2c_res_t excluded in non-xc builds

In i2c.h, the i2c_res_t enum typedef is defined inside the #ifdef block that hides non-xc stuff from the c/c++ compilers. I don't see why this typedef can't be declared outside of this #ifdef block. In fact, it is currently causing a problem for my project so I had to manually edit the header myself to do so.

Smaller Jenkins artefact

Snapshot size with the below command is currently over 300MB, as it includes build temporaries and much else:

archiveArtifacts artifacts: "${REPO}/**/*.*", fingerprint: true

Suggest reducing this to the necessary minimum (just source?)

i2c_master_async build warning

warning: argument 1 of `i2c_master_async_aux' slices interface preventing analysis of its parallel usage
i2c_master_async_aux(i, n, i2c_dist[0], max_transaction_size);

Version number on .xproject file does not match released version

The .xproject file states version 5.0.0dc6fb354094

This has not been maintained, this is likely a general issue with lib_repos.

doxygen build warning

/buildbot_worker/DailyVocalFusion_xmos_docker/build/sb/lib_i2c/lib_i2c/doc/rst/_build/.doxygen/api/i2c.h:662: Warning: argument 'max_transaction_size' of command @param is not found in the argument list of i2c_slave(client i2c_slave_callback_if i, port p_scl, port p_sda, uint8_t device_addr)

SDA can be driven high (violates i2c specification)

lib_i2c/lib_i2c/src/i2c_master.xc

Line 181 in 648351b

p_sda <: data & 0x1;

Both I2C bus lines should only ever be driven in open drain mode i.e. either drive low or drive high-z (input). This is the case for SCL but not the case for SDA when driving data on to the bus - it is driven high if the data is high. This is contrary to the specification and could lead to multiple hardware issues. (driving current into pullup supply voltage, multi voltage compatibility issues etc.)

Instead of:
p_sda <: data & 0x1;
Correct function here would be:
if (data & 1)
p_sda :> void;
else
p_sda <: 0;

I have made this change and done quick testing with success.

A similar change would be needed to the other i2c sources e.g. i2c_slave.xc and i2c_master_async.xc etc.

Use case that is not supported

I am trying to write a piece of code, and somewhere in the middle of a long call tree, I need to use I2C. This is the only place I need I2C, I don't need it anywhere else.

Ideally, I would just call two functions 'READ' and 'WRITE' to interact with I2C. However, to do that I need to create an interface to an I2C master, but when I do that I end up with a thread that won't die.

  f() {
      ...
      par {
        i2c_single_port_master(...);
        code_that_uses_said_master;
      }
      ...
  }

How do I write that, without having to change my whole call tree to pass an i2c interface around? I am never concerned about sharing, as this is my final application. f() is a call back from other libraries, and hence I do not want to change it.

Ugly solutions include starting i2c_single_port_master at top level and somehow storing the i2c interface in a global.

Clean solutions would be to just use i2c_read and i2c_write functions; I am missing a trick as to how to use those functions encapsulated in the interface without actually creating the interface as an ever-running task.

Repeated start condition setup time violated if a long delay occurs between completion of a non-terminated transmission and start of next transmission

The I2C specification details specific timing requirements that must be adhered to.
Two of these timing requirements relate to the "setup" and "hold" times for a start/repeated start condition, where a repeated start condition is a start condition without a preceding stop condition from a previous transmission; the definition of these two times is illustrated in Figure 1. The required setup time for a repeated start condition is 4.7/0.6 μs for busses running at 100/400 KHz respectively, while the required hold time for a start condition is 4/0.6 μs at 100/400 KHz.

Figure 1: I2C start condition timing constraint illustration (not to scale)

Testing for the fix detailed in PR #79 has highlighted that under specific conditions we continue to violate the setup time constraint for a repeated start, even when the previous fix has been applied.
This occurs when a long delay is seen between the conclusion of a non-terminated transmission and the start of the next, and is highlighted in test_bus_lock. A timing compliant repeated start can be seen in Figure 2a below, while Figure 2b shows the non-compliant timing seen when running this specific test.

Figure 2: a) timing-compliant repeated start condition. b) I2C timing error seen when running test_bus_lock.

Example makefiles need tidying and mix architecture and product naming

This issue covers a few point points on the example makefiles - as commented on #68 (review)

• Makefiles are mixing architecture naming (xs2/xs3) with product naming (xcore200/xcore.ai)
• IF statement is non intuitive (if condition not true then do something... rather than if condition do something)
• Extra XCOREAI var used
• Flags should be outside of the IF

The last two break the CONFIG=xxx option to xmake/xcommon. i.e. I should be able to build only for xcore200 with:

xmake CONFIG=xcore200

I provide an example below for app_simple_single_port_master. I believe this style should be replicated over all examples (and indeed examples in other libs as applicable)

# The APP_NAME variable determines the name of the final .xe file. It should
# not include the .xe postfix. If left blank the name will default to
# the project name

APP_NAME =

# The flags passed to xcc when building the application
# You can also set the following to override flags for a particular language:
#
#    XCC_XC_FLAGS, XCC_C_FLAGS, XCC_ASM_FLAGS, XCC_CPP_FLAGS
#
# If the variable XCC_MAP_FLAGS is set it overrides the flags passed to
# xcc for the final link (mapping) stage.

BUILD_FLAGS = -O2 -g -DDEBUG_PRINT_ENABLE=1 -report

XCC_FLAGS_xcore200 = $(BUILD_FLAGS)
XCC_FLAGS_xcoreai = $(BUILD_FLAGS)

# The TARGET variable determines what target system the application is
# compiled for. It either refers to an XN file in the source directories
# or a valid argument for the --target option when compiling.

ifeq ($(CONFIG),xcoreai)
TARGET = XCORE-AI-EXPLORER
else
TARGET = XCORE-200-EXPLORER
endif

# The USED_MODULES variable lists other module used by the application.

USED_MODULES = lib_i2c(>=6.0.0) lib_logging(>=2.1.0)

#=============================================================================
# The following part of the Makefile includes the common build infrastructure
# for compiling XMOS applications. You should not need to edit below here.

XMOS_MAKE_PATH ?= ../..
include $(XMOS_MAKE_PATH)/xcommon/module_xcommon/build/Makefile.common

Tests should be ported to use pytest and test_support repo

xmostest now deprecated.

Note, xmostest py2, test_support py3

AN00157: Use workspace_loc in .cproject

It has an absolute path /Users/peter

Comments in i2c.h are wrong

The comments for the function i2c_master_single_port() say that p_i2c is configured using i2c_conf.h. This is not the case, it is configured using the scl_bit_position and sda_bit_position arguments. Comments need fixing.