Trivial bug in(?) and proposed change to libi2c.h

[Thomas Doerfler]

Hi Robert,

Robert S. Grimes schrieb:
> Hi,
> 
> Two small issues in cpukit/libi2clib/libi2c.h:
> 
> 1. On or about line 178, there is a small bug that causes my compiles to 
> fail.  The relevant code is this:
> 
>     typedef struct rtems_libi2c_bus_ops_
>     {
>       /* Initialize the bus; might be called again to reset the bus
>     driver */
>       rtems_status_code (*init) (rtems_libi2c_bus_t * bushdl);
>       /* Send start condition */
>       rtems_status_code (*send_start) (rtems_libi2c_bus_t * bushdl);
>       /* Send stop  condition */
>       rtems_status_code (*send_stop) (rtems_libi2c_bus_t * bushdl);
>       /* initiate transfer from (rw!=0) or to a device */
>       rtems_status_code (*send_addr) (rtems_libi2c_bus_t * bushdl,
>                                       uint32_t addr, int rw);
>       /* read a number of bytes */
>       int (*read_bytes) (rtems_libi2c_bus_t * bushdl, unsigned char *bytes,
>                          int nbytes);
>       /* write a number of bytes */
>       int (*write_bytes) (rtems_libi2c_bus_t * bushdl, unsigned char *bytes,
>                           int nbytes);
>       /* ioctl misc functions */
>       int (*ioctl) (rtems_libi2c_bus_t * bushdl,
>             int   cmd,
>     bug?==> void *buffer;
>             );
>     } rtems_libi2c_bus_ops_t;

Agreed, this should never have come there :-(

> 2.  Near the bottom of the same file, there is the typedef for 
> rtems_libi2c_tfr_mode_t.  I'd like to add a new field to this structure 
> as illustrated below (the new field is spi_sel_mode):
> 
>     typedef struct {
>       uint32_t baudrate;       /* maximum bits per second               */
>                                /* only valid for SPI drivers:           */
>       uint8_t  bits_per_char;  /* how many bits per byte/word/longword? */
>       boolean  lsb_first;      /* TRUE: send LSB first                  */
>       boolean  clock_inv;      /* TRUE: inverted clock (high active)    */
>       boolean  clock_phs;      /* TRUE: clock starts toggling at start
>     of data tfr */
>       boolean  spi_sel_mode;   /* SPI only - TRUE: SS set per
>     transaction; */
>                                /* FALSE: SS set per word */
>     } rtems_libi2c_tfr_mode_t;

Hm, in that case the transaction should also contain the currently 
selected slave address. And with this functionality, the timing for the 
negation of SS is not yet defined. Between two transactions, do you want 
it to be negated it for 1ms, 1us, 1ns... ?

I know that the current implementation also has a lot of gaps, because 
actually SPI hardware can have a fair amount of different requirements 
concerning the timing. Maybe we should add this to the tfr_mode aswell. 
And define the interaction between your change and the "send_addr" call 
differently.

The additional timing requirements IMHO would be:
- time (in nanoseconds?) between active edge of select signal and first 
active clock edge

- time (in nanoseconds?) between last active clock edge and inactive 
edge of select signal

- time (in nanoseconds?) between last inactive edge of select signal and 
active edge of (possibly different) select signal

Any comments?

wkr,

Thomas Doerfler.
-- 
--------------------------------------------
embedded brains GmbH
Thomas Doerfler           Obere Lagerstr. 30
D-82178 Puchheim          Germany
Tel. : +49-89-18 90 80 79-2
Fax  : +49-89-18 90 80 79-9
email: Thomas.Doerfler at embedded-brains.de
PGP public key available on request

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.