[En-Nut-Discussion] GPIO_CFG Definitions

[Ole Reinhardt]

Hi Uwe,

> the GPIO functions greatly help to write (somehow) portable user
> code. However at the moment every architecture defined its own set of
> GPIO_CFG_DISABLED. So things tend to diverge.  I think, this is not good, as
> so the user code may have a definition that is available in one architecture
> and not in some other. So compilation will choke.

At some points I agree, on others I don't. When implementing the LPC
GPIO driver I tried keep compatibility with different architectures as
much as possible.

Other operating systems (let me name linux here again) have their own
architecture specific implementation and also provide a generic API on
top of that. 

The generic API only provides the absolut minimum and common to all
architectures  set of functionality.

> I propose:
> - Move the definitions to the common header dev/gpio.h. 

I fully disagree, as we would end up in a confusing set of defines and
#ifdefs

Better use different headers and include them into the /dev/gpio.h like
it is done now.

We only might think about moving the architecture specific files to
include/arch/xxx/

> Add there new
> (architecture specific) definitions as needed. Definitions should
> have a somehow general meaning. Perhaps the MCU_LPC177x_8x definitions 
> #define GPIO_CFG_ADMODE     0x00000400
> #define GPIO_CFG_DAC_ENABLE 0x00000800 
> could somehow be merged with GPIO_CFG_DISABLED(see below) and some other
> more general properity. If we get over 32 feature bits, we should think
> again :-)

I don't understand what you mean here.

Let me explain the above settings:

GPIO_CFG_ADMODE:

This is a flag which is available for only very few pins on the LPC
which selects if the pin works in analog or in digital mode

GPIO_CFG_DAC_ENABLE:

same here.

Anyway this setting has nothing to do with the selection of alternate
functions.

> - Mark for each definition what should happen if the architecture doesn't
> have or implement that feature. Choices are ignore or abort. If some example
> requests GPIO_CFG_HYSTERESIS, this may work even on another architecture
> that don't have hysteresis.

Ok, but all these settings are _very_ architecture specific, so I think
a real-world example would _never_ use these settings, but these
settings are for sure used within architecture specific driver code,
where the I/O pins have to be configured accordingly.

> - get rid of NULL definitions as
> #define GPIO_CFG_INPUT      0x00000000

This one is just to make application code more understandable.

You can write: 

GpioPinConfigSet(1, 23, GPIO_CFG_INPUT);

instead of 

GpioPinConfigSet(1, 23, 0);


> that are not maskable. I see no sensible way to test for such a feature.

Why do you want to mask this bit?

> I think
> #define GPIO_CFG_SPEED       0x000000C0
> #define GPIO_CFG_SPEED_SLOW  0x00000040
> #define GPIO_CFG_SPEED_MED   0x00000000
> #define GPIO_CFG_SPEED_FAST  0x00000080
> #define GPIO_CFG_SPEED_HIGH  0x000000C0
> is okay, as there is a mask.
> 
> Otherwise I propose to rename (or at least alias) GPIO_CFG_DISABLED to
> GPIO_CFG_ANALOG, as that is more intuitive i.m.h.o.

But this is missleading too, isn't it?

The LPC uses GPIO_CFG_DISABLED only for GpioPinConfigGet and ignores it
at GpioPinConfigSet, because the alternate function is given using the
flags GPIO_CFG_PERIPHERAL0 .. GPIO_CFG_PERIPHERAL7


What I would propose:

Try to keep the same name for the basic settings:

GPIO_CFG_INPUT
GPIO_CFG_OUTPUT
GPIO_CFG_PULLUP
GPIO_CFG_PULLDOWN
GPIO_CFG_REPEATER
GPIO_CFG_MULTIDRIVE

use 

GPIO_CFG_DISABLED

If you need it. Perhaps the same way like it is done with the lpc:

Return it with GpioPinConfigGet() if the pin is configured to its
alternate function, ignore it with GpioPinConfigSet if not needed.

If a setting is not supported, define it to 0x00 like

GPIO_CFG_DEBOUNCE 

for the LPC176x

And add other architecture specific bits how you need them like

GPIO_CFG_HYSTERESIS
GPIO_CFG_INVERT
GPIO_CFG_SLEWCTRL
GPIO_CFG_ADMODE
GPIO_CFG_DAC_ENABLE


So if you follow these basic roules you can write samples which are
supported by all architectures, as long as they only use the basic
settings.

Next you could document in your driver code which settings are supported
by the hardware.

Don't mind about special bit positions, as your application code should
always rely on the named constants instead of bit positions to keep the
code portable.



This is just _my_ opinion. Perhaps I did not yet clearly understand your
point. If you like, send me a PM, so we could discuss this issues in
native language :)

Bye,

Ole

-- 

Thermotemp GmbH, Embedded-IT

Embedded Hard-/ Software and Open Source Development, 
Integration and Consulting

http://www.embedded-it.de

Geschäftsstelle Siegen - Steinstraße 67 - D-57072 Siegen - 
tel +49 (0)271 5513597, +49 (0)271-73681 - fax +49 (0)271 736 97

Hauptsitz - Hademarscher Weg 7 - 13503 Berlin
Tel +49 (0)30 4315205 - Fax +49 (0)30 43665002
Geschäftsführer: Jörg Friedrichs, Ole Reinhardt
Handelsregister Berlin Charlottenburg HRB 45978 UstID DE 156329280