Re: [Sdcc-user] TUSB3210 and TUSB2136 (8051 with USB) headers, library and demo programs

[Johann G. <Joh...@gm...>]

Hi!

Finally I come back to your suggestions.

Am Sonntag, den 11.01.2009, 19:58 +0100 schrieb Maarten Brock:
> Hansi,
> 
> > Am Mittwoch, den 31.12.2008, 17:43 +0100 schrieb Maarten Brock:
> > > The headers could be included and I'm glad you already used 
> > > compiler.h. But I do not like anything I see in uctypes.h. NULL, 
> > > bool and the int's are all already defined in stdlib.h, stdbool.h 
> > > and stdint.h. Please use those.
> > 
> > Using 'bool' as defined by stdbool.h as a bit makes problems when using
> > functions with a bool return type and making a pointer to such a
> > function.
> > 
> > typedef bool (*BoolReturnFunc_t)();
> > 
> > bool TestFunc() {
> >   return true
> > }
> > 
> > void test() {
> >   BoolReturnFunc_t FuncPtr;
> >   FuncPtr = &TestFunc;   // <-- error
> > }
> > 
> > gives the error
> >   error 35: '&' illegal operand , address of bit variable
> > 
> > Probably this is just a bug report. I've modified all such functions to
> > return an uint8_t. For all others I use bool and stdbool.h.
> 
> I agree that this is a bug, but it can easily be circumvented by 
> removing '&'. You don't need to take the address of a function to 
> turn it into a function pointer. E.g.
> 
> FuncPtr = TestFunc;  //should give no error  

Ok, I converted it back to a bool return type and removed the '&'.

> > I've also removed uctypes.h and use stdint.h everywhere. Thanks for the
> > hint that all this is already prepared. :-)
> > 
> > > And when compiling with --std-c89 you cannot use // comments nor 
> > > 'xdata'. Please use /* */ and __xdata instead.
> > 
> > Is the possibility to compile with --std-c89 mandatory?
> > 
> > I tried to compile with --std-c89 and it gives the warning
> >   warning 187: ISO C90 does not support flexible array members
> > for an "open" array element of
> >   typedef struct {
> >     ...
> >     int Field[];
> >   } name;
> > How can I get rid of this warning?
> 
> You can either use 'int Field[1];' or insert a pragma to ignore the 
> warning. But if you think you need C99 functionality and document it 
> in the comments I doubt it would be a problem.

Decided to compile everything with --std-c99 and added a comment to the
respective file.

> > Everything else in the library I've changed to be --std-c89 compliant.
> > 
> > > Personally, I am totally against function implementations in header 
> > > files unless 'inline'd. But inline again is C99 only.
> > 
> > I'm too against implementations in .h files. Where did you find those?
> 
> Huh? All functions in tusb3210.h are definitions! To make them 
> prototypes everything between the curly braces {} should be removed 
> and the braces themselves too. But I suggest to remove them 
> completely from this file.

Oops, I missed these. Ok, moved them to lib/usb.{c,h}.

> > > Finally, are you sure the TUSB is completely 8052 compatible? 
> > > Otherwise I'd prefer not to include 8052.h and copy the relevant 
> > > portions only.
> > 
> > Yes, the datasheet says:
> > 
> >  * Integrated 8052 Microcontroller With:
> >   ­ - 256 × 8 RAM for Internal Data
> >   ­ - 8K × 8 RAM Code Space Available for Downloadable Firmware From
> > Host
> >      or I2C Port.
> >   ­ - 512 × 8 Shared RAM Used for Data Buffers and Endpoint Descriptor
> >      Blocks (EDB)
> >    - Four 8052 GPIO Ports, Ports 0,1, 2, and 3
> >    - Master I2C Controller for External Slave Device Access
> >    -­ Watchdog Timer
> > 
> > So it is a real 8052 and not an 8051.
> 
> But have you verified every sfr and every bit in it? Is every 
> peripheral of the 8052 present in this mcu and not missing any 
> function? Nor enhanced? This summation does not convince me.

I'm not able to check this but there is a FAQ entry at TI's homepage
  http://focus.ti.com/lit/an/slla157/slla157.pdf
which says

> Can you tell us what micro controller it is based on?
> Can you give us a
> reference for it to make the memory addressing a bit easier?
>
> Mentor Graphics 8052. Try 
> www.intel.com/design/MCS51/MANUALS/272383.htm for reference. The user
> is expected to be familiar with the 8052 MCU.

So, I'm pretty confident this is a real 8052.

Please find attached the current version.

Bye
  Hansi