[En-Nut-Discussion] Porting means breaking the rules?

[Ulrich Prinz]

Hi!

Am 02.09.2010 13:12, schrieb Harald Kipp:
> On 01.09.2010 23:17, Ulrich Prinz wrote:
>
>> The reason for having lots of #ifdef in the USART code of an AVR was,
>> that you had to do a lot of things manually like handshake and direction
>> switching of RD485 drivers.
>
> There is one thing that is often overlooked with AVR code:
>
>   sbi(PORTA, 1);
>
> results in a single bitset machine instruction as long as constants are
> provided as parameters. This will be no longer the case for
>
>   sbi(base + PORTA_OFFSET, 1);
>
> or
>
>   sbi(PORTA, pin_info->rts_pin);
>
Yes, this wont work as the address needs to be calculated at runtime.
But if you do it like this:
#define sbi( b, p) (BITAREA_BASE+b*STM_PORTSIZE+PORT_BSRROFS+p*16)=1
#define cbi( b, p) (BITAREA_BASE+b*STM_PORTSIZE+PORT_BRROFS+p*16)=0
it will be calculated at precompiler run and result in a sinlge cacle 
instruction.

> With a single character hardware FIFO only, the UART driver is most
> sensitive to every single instruction that may be added to the code.
Yes and no. Normally you cannot change the handshake signals while a 
byte is flowing out. You can change the handshake while a byte is coming 
in. So most time you handle handshake signals in the transmission 
finished interrupt and the receiving interrupt.
Handling it in FIFO register empty interrupt is dangerous as it may get 
out early enough or it may not.

>
> I fully agree: As a result, the code is contaminated with tons of
> #ifdefs, which makes it more a less unreadable. Furthermore, in order to
> avoid duplicated code, the AVR driver includes a C source file (.c
> instead of .h), which sometimes confuses debugger tools and always
> confuses the programmer.
>
> OK, I'm simply repeating your concerns to underline them. :-)
>
>> Together with the fact, that all ARM and Cortex CPU unified their
>> peripheral register layout, this means, that you have a single driver
>> and just exchange the base address of the device.
>
> Right. The requirements of the low end 8-bit AVR do not apply to the
> higher end controllers. In fact, the AVR code results in larger code for
> the other CPUs.
>
>
>> base_address:
> ...
>> first_interrupt:
> ...
>> dev_icb:
> ...
>
> As suggested, you can try to re-use currently unused items as described,
> but you can also add your own, using these ugly #ifdefs. IMHO, the
> priority is
>
> 1. Execution speed
> 2. Code size
> 3. Code readability
>
>
>> Putting these information into dev_icb enables two things at once:
>> in nutconf you can select the configuration with a simple drop down list.
>> Giving the right values, in a lot of CPUs pins and bits can be handled
>> by special instructions making them atomic what saves some IRQ switching.
>
> This makes sense to me. In any case, if anyone is using an exotic board
> layout, he can always modify existing driver code.
>
I thought a bit about it today and think it must be possible to handle a 
usartX_conf.h that is included on the run by one single stm_usart.c.
In this header file all declarations and configurations are done.
It might be an option for certain supported codecs to make them #ifdef 
in the code too as they will add to much code.
But there is only one c file for all usarts.

>
>> There is a lot of troubles with the GPIO control interface.
>
> I feel guilty. Also Michael Fischer lately criticized this while trying
> to get the AVR32 boards running. As far as I understood, he made a few
> changes on the AVR32 code to make it more unified. But I haven't found
> the time to study his code in more detail. :-(
>
> He also suggested to sequentially number all port bits. Let's say, port
> 33 will be PE1 on the AVR8 and PB1 on the ARM. After thinking about this
> in more detail, I don't like it. It may confuse people even more and may
> not fit to all platforms. I still recommend to divide port access into
> banks and pins. This is more flexible.
>
I don't like this idea to as it adds again calculation to the port 
access. For the STM I will not add any virtual port numbering but may 
map it to the base address.
This doesn't slow down the speed as for an 32bit system a
switch(bank)
  case 0xA0004300:
is the same as a
switch(bank)
case 1:

>
>> This allows bit manipulation in a single assembler instruction but the
>> need is to have the bit manipulation definitions of Nut/OS done in
>> another way.
>
> FYI, we already have two variants in Nut/OS. One is the gpio_XXX.c
> library, which is easy to port and to use. And we do have macros in
> arch/porttran.h, which are a bit more difficult (based on the
> pre-processor, requiring #undefs), which produce optimal code.
>
I'll check that, but I'd like to have one single GPIO interface and may 
be some set of high-speed and some set of high-sophisticated functions.

> In summary, I welcome your ideas and your efforts in supporting the
> Cortex CPUs. As a short side note: Please commit your changes divided
> into comprehensible parts. Do not mix up cosmetic and logical code
> changes. Going this way, there are no limits for you, because everyone
> can easily follow them and they can be easily reverted, if required.
>
That's the reason why I not announced this code in the mailing list.
If you addd a complete new architecture, lots of things stream into the 
system, especially while trying to reuse code from STM.
Second, my editor does some beautifying ( kill trailing blanks...) in 
the background and so, some files may get touched that I only opened.
That will get less and less at every checkin.

At the point the basic things are working, I'll announce the code to the 
major developers for review and then merge it with the trunk.

Basic things are:
- Startup: DONE
- Memory Init: 90%
- Idle Task: DONE
- Main Task: DONE
- GPIO: 20%
- USART1: 50%

I have one problem in the Tasking as Idle and Main run but with starting 
one single new thread, the system crashes.

Best regards,
Ulrich