[En-Nut-Discussion] Unified GPIO implementation

[Ulrich Prinz]

Hi Duane,

Am 10.10.2010 02:07, schrieb Duane ellis:
>
>
>> AVR:
>> int8_t GpioPinConfigSet( uint8_t port, uint8_t pin, uint8_t cfg)
>>
>> AVR32,ARM,Cortex
>> int GpioPinConfigSet( uint32_t port, uint32_t pin, uint32_t cfg)
>>
>
> (a) Use "int" - as the gpio id - and nothing else.
>       Use "int' as the configuration
>       Use "int' as the value
>
>       Int is the natural size of the machine, and is naturally fast.

That is not correct. int is specified as size of the architecture but 16 
bit minimum. So for an ARM it is 32 bit, but for an AVR it is 16 bit 
witch is no the size of the architecture. AVR has only 6 registers that
can be combined for 16 bit arithmetics.
>
> if you *insist* - then create an ARCH specific typedef - like this:
>
> #if !defined(GPIO_ID_TYPEDEFFED)
> #define GPIO_ID_TYPEDEFFED 1
>      // default is an int type
>       typedef  int GPIO_ID;
> #endif
>
??
Ok, yes I think it is a good idea to use typedefs for such definitions 
as it automatically issues a compiler error / warning if you go out of 
bounds. On the other hand it then is needed to rewrite almost all 
drivers as it will rise compiler warnings if you use a new port 
definition with an older function header, telling about incompatible types.
To be correct, if you just use typedef int you have no benefit so you 
need to use typedef enum to rise warnings if you call with wrong type of 
parameter or parameters out of bounds.

> (b) Use "-1" as invalid, for example some chips have built in USB
> resistors, others have the USB resistor external

That is the way it is. Negative is bad, 0 is good and >0 is a status 
reply. What I'd like to see is that more than only -1 is used for error 
reports. Sometimes it is important to know the specific error and I'd 
like to see -2, -3...
I like this way more than the linux way where -1 alway tells error and a 
separate function is used to request the specific error. The the 
function that produces the error already set up additional variables 
with the detailed error status, for Nut/OS you can save these variables 
and the additional function call.
>
> You need a simple "invalid" - here's why:  Example USB - some
> controllers have a built in "attach" resistor  some use a GPIO pin ...
> you must be able to #define USB_GPIO_RESISTOR as "invalid" - thus you
> can can specify
>
> // Port C, Bit 4 - is the usb resistor
> #define USB_GPIO_RESISTOR_PIN  MAKE_GPIO_ID(  PORT_ID_C,  4 )
> OR
> #define USB_GPIO_RESISTOR_PIN   GPIO_ID_INVALID
>
??
#define USB_USBV_DETECT_B NUTGPIO_PORTA   /* USB VDETECT BANK */
#define USB_USBV_DETECT_P 5               /* USB VDETECT PIN */

if( GpioPinGet( USB_USBV_DETECT_B, USB_USBV_DETECT_P)) {
   usb_connect();

I can read this much better than you construction, sorry.

or you might like this?
inline int IsUsbV(void)
{
   return GpioPinGet( USB_USBV_DETECT_B, USB_USBV_DETECT_P)?1:0;
}

>
> (d) Don't worry about "signed" numbers here - signed numbers are just
> fine, reason:
>
>       most 8bit chips very few gpio pins... (ie: less then 64) - an INT
> works
>       most 16bit machines - have a few more - but still not many - an INT
> works
>      Most 32bit machines - no chip has 2billion pins - an INT works.
>
What you mix up is the reason of existence of Nut/OS. Nut/OS tries to 
tweak out performance on chips that are small and cheap and easy to 
understand. Nut/OS is not Linux where compatibility and overall 
programming is standardized by cost of performance.

> Also - LESS THEN 0 tests are simple and fast on nearly all machines.
>
Yes that's right, the cheapest and therefore best way of numeric test is 
equal 0 or not equal 0. All other tests, independent of the numeric 
sign, is expensive.

> (d) Often generic code needs a GPIO as a function...
>
> If the gpio operation is a function - it costs *MORE* in code and
> *CLOCK*CYCLES* to (a) loading two values into two parameter registers,
> (b) passing these through a *MULTIPLE* few layers of boiler plate code,
> and you also have configuration values in structures and #defines.
>
That is right but here again something comes from the intention of 
Nut/OS: The system tries to lead beginners into using operating systems 
on small devices. So functions are self explaining and follow the same 
naming convention through all architectures as far as it is possible.
One should be able to switch existing software from one platform to 
another with only few changes in the existing application code.

But sometimes there is high speed access to a function ( not only GPIO) 
needed. But this is mostly the case in low level drivers and these are 
mostly attached to the architecture. So nothing is to say against having 
the educative, easy to use functions on one hand and the very highly 
optimized architecture specific functions on the other hand.

Example:
Beginner likes to flash a LED:
#define MYLED_B NUTGPIO_PORTA
#define NYLED_P 4
NutGpioPinConfigSet( MYLED_B, MYLED_P, GPIO_CFG_OUTPUT);

while(1) {
   NutGpioPinSetHigh( MYLED_B, MYLED_P);
   NutSleep(500);
   NutGpioPinSetLow( ...


If I write a driver part for a LCD display I fire data at it with 5MBit 
or more. Using this function calling overhead makes /CS handling longer 
than writing half of the display with data. So I use
LCD_NCS_B->BRR = _BV(LDC_NCS_P)

No beginner can understand that, but that is not needed, as the beginner 
uses LcdWriteData( buf, len);

> (e) It is easy to create a macro  (Each arch/chip makes its own)
>
> #define MAKE_GPIO_ID(  PORT_NUM, BIT_NUM )    ((PORTNUM * BITSPER_PORT)
> + BIT_NUM)
> #define GPIO_ID_INVALID  (-1)
>
> (d) That SHIFT/MASK can be *UNDONE* in a macro
>
>        GPIO_GET_PORT_NUM( ID )     ((ID) / BITSPERPORT)
>       GPIO_GET_PORT_BITNUM(ID)  ((ID) % BITSPERPORT)
>
Oh, wow, now you have 6? macros to handle one single pin status and you 
didn't even include the initialization of the pin.... Who do you think 
does understand your code if you have to hand over that project to 
someone else? And do you understand your own code if you have to fix 
something in 4 years when you had 10 different projects in between?

As a senior programmer, software architect and integrator I would reject 
this code, sorry.

> Because BITSPERPORT - is a constant, and is a power of 2 -  the compiler
> turns these into SHIFT or AND operations, can't get much faster then
> that, no need to optimize...
That is not always true. ARM has a barrel shifter and can shift any size 
of variably by any count of bits in any direction. AVR does not have 
this, it only knows about one shift left or right. A multiple shift that 
cannot be decoded into a numeric value by the precompiler run will cause 
lots of assembler code, either mutiple shifts or a loop with counter.
>
>> The AVR32 toolchain defines which references the GPIO set their values as
>> pin numbers, not ports/pins. [snip]
>>
>> #define AVR32_USART2_RXD_0_PIN 33
>>    Which would translate to port B, pin 1.
>
> Exactly my point - simple macros {I presume this is what they are doing,
> I don't have the docs in front of me}
>
>     #define   GPIO_PORT_NUM( ID )   (  ID / 32 )
>     #define   GPIO_PIN_NUM(ID)         (ID % 32)
>     #define   GPIO_PIN_MASK(ID)      (1<<  (ID % 32))
>
Hm...
That is already in. Even for Cortex I did something else by having some 
defines, that recalculate from Nut/OS numbering to Cortex GPIO register 
base addresses and vice versa.

With the GPIO handling we discuss this will be obsolete. If you give a 
document of what is mandatory and what is recommended, any architecture 
can do what it needs to be fast and simple. My and Thiagos intentions 
was to split the names from the content to be more flexible. For the 
beginner the behavior is fully transparent.

So with AVR NUTGPIO_PORTC is 3 and NUTGPIO_PIN5 is 5
With ARM/Cortex it is
NUTGPIO_PORTC 0x40010800 and NutGPIO_PIN5 is _BV(5).

With AVR it is still needed to do NutGpioPinSetHigh(port, pin)
switch(port)
   case NUTGPIO_PORTA
   case NUTGPIO_PORTB

while in ARM/Cortex you could do
#define NutGpioPinSetHigh( port, pin) port-BSRR = pin

By the way, you only need USBV detection on systems where energy saving 
is needed or where it is important to know from which source you are 
powered. On any other system it is enough to check the frame status 
register of the receiver to know if you are connected to USB or not.

Best regards
Ulrich