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

[Robert S. Grimes]

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;


Removing the ; at the end of "void *buffer;" makes my compiler happy.  
Is there any reason someone shouldn't make this change?

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;


Justification: Some SPI devices want their chip select control signal to 
be "frame" each word (device-specific width), while others (such as some 
serial EEPROM and Ramtron FRAM devices) use the chip select as a 
"transaction" signal; in these devices, the chip select is made active, 
a command byte is sent, followed generally by a byte or two of address 
info, and finally one or more bytes of data to either write or (dummy) 
data to stimulate read data - the end of such a transaction is signaled 
by the CS going inactive.

Additionally, some SPI controllers, such as the Virtex SPI peripheral, 
can optionally generate automatic, per-word chip selects, or manual - 
thus supporting the per-word and per-transaction modes, respectively, 
mentioned above.

However, there is currently no mechanism for specifying to the SPI 
driver which strategy to use.  It is specific to the actual device on 
the SPI bus, so devices on one bus might well use different strategies.  
It seems the current SPI driver calls the ioctl function 
RTEMS_LIBI2C_IOCTL_SET_TFRMODE, which passes a pointer to a 
rtems_libi2c_tfr_mode_t structure; furthermore, initialization of SPI 
device-specific drivers (such as c/src/libchip/i2c/spi-flash-m25p40.c) 
provide the definition of said structure.  So this seems the right place 
to put it.

Does this sound like a good idea?  Can one of the gurus sponsor (or 
reject) this proposal?  FWIW - this is the approach I've adopted, and it 
seems to work just fine...

Thanks,
-Bob