[Bug 1001397] I2C driver for Kinetis microcontrollers

[bugzilla-daemon@bugs.ecos.sourceware.org]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001397

--- Comment #58 from Ilija Kocho <ilijak@siva.com.mk> ---
(In reply to comment #57)
> I get the same conservative frequency no matter how
> CYGNUM_DEVS_FREESCALE_I2C_CLOCK_AGR is set.

That means that it is the closest frequency to the set-point (i.e. the closest
higher frequency would give larger error). The frequency selection is rather
coarse so there's little help, given the system clock frequency. By
experimenting I have noticed that if system clock is 100MHz (rather than
current 96MHz) we get better match for 100Khz (and 400Khz). Also with inf that
case the aggressive lookup gives closer (upper) match than conservative.

> 
> I have given the code below, which I know ignores the delay specified in the
> CDL. Is there a different way to write the code so that the delay and AGR of
> the CDL get applied?

Just set i2c_delay (i.e. i2c_bus_time) to 0. Instead of infinite frequency it
will give you the default one.

For example:
     cyg_i2c_device device = {
             .i2c_bus        = &cyg_i2c0_bus,
             .i2c_address    = address,
             .i2c_flags      = 0,
             .i2c_delay      = 0
         };

See other examples for enforcing FIT and aggressive lookup in comment 54.

Some [off topic] remarks to the code below:
device is automatic variable that means it's being initialized every time the
function is called. It's very inefficient. Setting i2c_delay =  0 shall make
initialization far much efficient since the  clock lookup once executed during
initialization of the I2C bus, in subsequent calls will use cached F register
setting. However the code is still less efficient than what's possible, because
the device structure ans device itself shall be initialized on every function
call. The better way is to declare device as static variable (spend little-bit
memory but save little-bit stack and little-bit more CPU time). However, I can
imagine that you use the same (hardware) device from other functions. If it is
true, than device should be a global variable.

> 
> uint16_t smbus_read_word(uint8_t address, uint8_t command)
> {
>     cyg_i2c_device device = {                        \
>             .i2c_bus        = &cyg_i2c0_bus,	     \
>             .i2c_address    = address,               \
>             .i2c_flags      = 0,                     \
>             .i2c_delay      = i2c_bus_time           \
>         };
>     cyg_uint8 buffer[1];
>     cyg_uint8 input[2];
>     buffer[0] = command;
> 
>     cyg_i2c_transaction_begin(&device);
> 	if(!cyg_i2c_transaction_tx(&device, true, &buffer[0], 1, false)) {
>         diag_printf("Read Word: fail TX.\n");
> 	} else if(!cyg_i2c_transaction_rx(&device, true, &input[0], 2, true, true))
> {
>         diag_printf("Read Word: fail RX.\n");
>     }
>     cyg_i2c_transaction_end(&device);
> 
>     return input[1] << 8 | input[0];
> }

-- 
You are receiving this mail because:
You are on the CC list for the bug.