Page 1 of 1

Pondering the best way around this...

Posted: Sat Sep 25, 2010 12:54 pm
by SecretSquirrel
I am in the middle of re-writing a low level SPI library for a project. The SPI interface is made available via a set of registers in the PCI address space for the card in question. Being a general purpose library, its job is to hide the technical details from the developer. The program calls a "map" function that is passed in the base address of the PCI registers and which flash chip on the card you are accessing. The oga_spi_reg structure is filled in appropriately with the address of each register. The oga_spi_func structure is also populated. Because multiple flash chips are supported, different ways of reading and writing the chip have to be supported. All this is hidden from the end developer, so all he/she needs to do is for example:
spi->func.read(spi, 0, &data, 1)
This would read one byte from address 0 and place it in data.

struct oga_spi_reg {
  unsigned int *ce_;
  unsigned int *si;
  unsigned int *sck;
  unsigned int *sio;
  unsigned int *sel;
};

struct oga_spi_func {
  void (*read)(struct oga_spi *,
          unsigned int,
          unsigned char *,
          unsigned int);
  void (*chip_erase)(struct oga_spi *);
  void (*write)(struct oga_spi *,
      unsigned int,
      unsigned char *,
      unsigned int);
};
   
struct oga_spi {
  enum oga_spi_devices device;
  unsigned char man_id;
  unsigned char dev_id;
  struct oga_spi_func func; 
  struct oga_spi_reg reg;
};


So, here is the problem. The above structure definitions are not conforming C code and the compiler lets you know.

In file included from oga_w25x_spi.c:2:
oga_spi.h:21: warning: ‘struct oga_spi’ declared inside parameter list
oga_spi.h:21: warning: its scope is only this definition or declaration, which is probably not what you want
oga_spi.h:22: warning: ‘struct oga_spi’ declared inside parameter list
oga_spi.h:26: warning: ‘struct oga_spi’ declared inside parameter list
In file included from oga_spi.c:2:
oga_spi.h:21: warning: ‘struct oga_spi’ declared inside parameter list
oga_spi.h:21: warning: its scope is only this definition or declaration, which is probably not what you want
oga_spi.h:22: warning: ‘struct oga_spi’ declared inside parameter list
oga_spi.h:26: warning: ‘struct oga_spi’ declared inside parameter list
oga_spi.c: In function ‘oga_spi_map’:
oga_spi.c:93: warning: assignment from incompatible pointer type
oga_spi.c:94: warning: assignment from incompatible pointer type
oga_spi.c:95: warning: assignment from incompatible pointer type


So, I'm looking for suggestions on how to address this. The read, write, and chip_erase functions don't actually need the whole oga_spi structure. All they need is the oga_spi_reg structure, so I could change the function signature for the three functions. That would make the above read example:
spi->func.read(&(spi->reg), 0, &data, 1)
As an end developer, that bugs me as the call is more complex than it needs to be and I start having to deal with the internals of the oga_spi structure even more than I already am.

I curious what people's suggestions are for this coding conundrum.

--SS

Re: Pondering the best way around this...

Posted: Sat Sep 25, 2010 2:31 pm
by SNM
Maybe I'm missing something or a piece got lost in transcription, but the errors are complaining because you're declaring struct oga_spi inside the struct oga_spi_func. Declare or define struct oga_spi before defining struct oga_spi_func and you'll be fine.

C/C++ isn't Java, ordering matters.

Re: Pondering the best way around this...

Posted: Sat Sep 25, 2010 7:08 pm
by SecretSquirrel
You can't declare ogi_spi first as it has references to oga_spi_func. If you try that, you end up with "inclomplete type" error messages. The messages now are simply warnings and in fact, the code will work as written. However, I dislike warnings and there are enough different ways to get rid of them, each with different implications, that I though I would toss it out for discussion.

--SS

Re: Pondering the best way around this...

Posted: Sat Sep 25, 2010 7:26 pm
by SNM
You can declare without defining, then declare pointers to the declared class, then define the class with references to a previously-defined class.

Re: Pondering the best way around this...

Posted: Sat Sep 25, 2010 7:35 pm
by SecretSquirrel
I could have sworn I went down that route and got an error, but then this is why I tossed it out for discussion. Works fine now... :-?

--SS