patchAVR C Runtime Library - Patches: patch #6352, Far pointer library

 
 

You are not allowed to post comments on this tracker with your current authentification level.

patch #6352: Far pointer library

Submitted by:  Carlos Lamas <carloslamas>
Submitted on:  Fri 21 Dec 2007 02:40:29 PM UTC  
 
Category: NonePriority: 9 - Immediate
Status: Ready For TestPrivacy: Public
Assigned to: Eric Weddington <arcanum>Open/Closed: Open

(Jump to the original submission Jump to the original submission)

Mon 26 Sep 2011 09:29:49 PM UTC, comment #9:

You seem to have misunderstood my comment.

Carlos's library is fully adequate for its purpose and has been used for that purpose by several users months or even years before it has been accepted in 1.7.0; the problem is that it is incorrectly compiled within avr-libc.

My comment thus describes an identical problem to https://savannah.nongnu.org/bugs/?32945 (I wasn't aware of at the time when this problem was brought to my attention in the linked avrfreaks thread).

JW

Jan Waclawek <wek>
Mon 26 Sep 2011 08:43:32 PM UTC, comment #8:

> They don't use the ELPM instruction rather, incorrectly,
> the LPM instruction.


The very problem is that AVR with > 64k flash is a segmented architecture and it's not easy to handle them correctly.

For example, ELPM Z+ might change RAMPZ so that this instruction is not of much use.

The root problem is the ABI and that it does not disallow crossing section boundaries, i.e. 64k boundaries. No data must cross such boundaries and if ELPM Z+ shall be used to get efficient code (users like small/fast/low-ISR-overhead code) the last byte of each segment must be kept free.

Any other approach will lead to very inefficent code and/or strage artifacts.

The alternative is to take away RAMPZ from the compiler altogether and let the user set it appropriately, but the fact that crossing section boundaries does not work smooth won't go away with it, either.

Georg-Johann Lay <gjlayde>
Wed 03 Aug 2011 09:06:02 PM UTC, comment #7:

The _PF functions in the respective binary libraries are broken, and they ever were, since the've been added to avr-libc in 1.7.0.

They don't use the ELPM instruction rather, incorrectly, the LPM instruction.

The reason is, that these functions rely on LPM_R0_ZPLUS_NEXT macro in common/macros.inc, which uses the BIG_CODE macro defined in the same file to determine whether LPM or ELPM is appropriate.

BIG_CODE is defined according to FLASHEND macro which comes from the individual files' headers (additionally, the RAMPZ register's address might be needed for some of the files). However, these files are compiled with the "generic" -mavrN switch, so the FLASHEND macro is unavailable thus considered zero by preprocessor.

This bug was originally reported here http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=110170

Jan Waclawek <wek>
Wed 30 Jun 2010 12:50:06 PM UTC, comment #6:

The xxx_PF() functions modify the RAMPZ register, but on XMegas with EBI, RAMPZ influences also LD/LDD/ST/STD, making code after using xxx_PF() to access unintended data memory "pages".

In the attached file, in macros.inc, macro LPM_R0_ZPLUS_INIT is modified and macro LPM_R0_ZPLUS_FINI added, storing and restoring RAMPZ, for targets with _AVR_HAVE_RAMPD_ defined (which IMHO is unique indication of presence of EBI). Only LPM_R0_ZPLUS_FINI needs to be added to xxx_PF() functions (and documentation modified accordingly), as shown in the attached memcpy_PF(). All the xxx_PF() functions are affected and should be modified accordingly after consensus happens.

As a cheaper alternative, RAMPZ could be simply zeroed as all "standard" RAM access in current avr-gcc is restricted to 16-bit addresses; whether this is a better option I leave to decide for the more experienced developers on this forum.

This resolves https://savannah.nongnu.org/bugs/?func=detailitem&item_id=25120 which thus could be closed as duplicate (the issue with RAMPZ mods being non-interrupt safe is resolved by saving RAMPZ in the standard interrupt prologue/epilogue as of avr-gcc version 4.3+).

(file #20859)

Jan Waclawek <wek>
Thu 03 Jun 2010 03:23:06 PM UTC, comment #5:

Adding one more function, memcmp_PF().

Jan Waclawek

(file #20682)

Jan Waclawek <wek>
Wed 22 Oct 2008 04:50:15 AM UTC, comment #4:

Carlos,

Could you create a patch to avr-libc? Otherwise we will have to manually merge your separate files and it will take longer to commit this.

Thanks,
Eric

Eric Weddington <arcanum>
Project AdministratorIn charge of this item.
Thu 07 Aug 2008 02:20:25 PM UTC, comment #3:

Hi there

I'd suggest using '"p" (var) ' instead of '"p" (&var)'
This allows to define:

typedef uint_farptr_t PGM_PF;
#define PFSTR(str) ((PGM_PF)GET_FAR_ADDRESS(PSTR(str)))

which again makes it easy to create new functions like printf_PF.

Just my two cent.

Cheers

Moritz 'morty' StrĂ¼be <cmorty>
Thu 03 Jan 2008 12:54:07 AM UTC, comment #2:

This is a good point. I've considered this at the time of writing the functions but then I've discarded it considering that the code for pgm_read_xxxx_far() macros in avr/pgmspace.h has also the same problems and EEPROM routines are also affected. RAMPZ modification is documented but I agree that the side effects could be explained more clearly.

Making these functions reentrant secured imposes a small penalty (just to save and restore RAMPZ in any of the free call-used registers, 2 instructions, 2 machine cycles) for each function, probably small enough to avoid having two different versions, one reentrant and the other not. pgm_read_xxxx_far() macros probably could live in two flavors, reentrant and not reentrant. These macros will suffer more (relative to their small size) overhead costs adding RAMPZ preserving code.

This is all referred to interrupt handlers only. Preemptive task execution will require to save and restore RAMPZ as part of the task state for each context swap unless interrupts were disabled in the whole function (absurd)

Maybe it's time to debate the creation of a reentrant avr-libc version...

> 3. In future add support of "far" pointer in avr-gcc.


This sounds a bit optimistic but... who knows?

Carlos Lamas <carloslamas>
Wed 02 Jan 2008 04:20:32 PM UTC, comment #1:

These functions modify RAMPZ register and is non-reenterable. They cannot be used in interrupt handlers and pre-emptive Real Time OS. It should be written in documentation.

Your comments on such offer:

Add in AVR GCC toolchain architectures for 128KB devices. Define "__AVR_HAVE_RAMPZ__" macro and save RAMPZ register in interrupt handlers for these devices.

Pro:
1. Safely use more 64KB data stored in FLASH.
2. Add xxxx_PF string functions in avr-libc.
3. In future add support of "far" pointer in avr-gcc.

Contra:
1. Longer prologue/epologue code in interrupt handlers (4 instructions + 1 byte stored in stack).

Anatoly.

Anatoly Sokolov <aesok>
Project Administrator
Fri 21 Dec 2007 02:40:29 PM UTC, original submission:

This code is to intended to access the whole flash memory in devices with flash address space bigger than 64KB.

The macro GET_FAR_ADDRESS() is the best version I've got after testing several ones but is not always a direct replacement of the & operator for getting far pointers. It has not been tested on ATmega2560, not even in simulation.

The string and memory copy functions for far program memory xxxx-PF() are still in beta stage and need more tests (not known bugs but not extensively tested). They had been parked in my hard disk for a long time but the big ROM devices users claim for some standard way to access the data in avr-libc.

Comments are welcomed.

Carlos Lamas.

Carlos Lamas <carloslamas>

 

Attached Files
file #20859:  memcpy_PF.zip added by wek (5kB - application/zip)
file #20682:  memcmp_PF.zip added by wek (5kB - application/zip)
file #14680:  extra pgmspace.zip added by carloslamas (26kB - application/zip - Sources to access the flash beyond 64KB)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by gjlayde (Posted a comment)
  • -unavailable- added by wek (Updated the item)
  • -unavailable- added by arcanum (Posted a comment)
  • -unavailable- added by cmorty (Posted a comment)
  • -unavailable- added by aesok (Posted a comment)
  • -unavailable- added by carloslamas (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 30 Jun 2010 12:50:06 PM UTCwekAttached File-=>Added memcpy_PF.zip, #20859
    Sun 13 Jun 2010 05:19:39 AM UTCarcanumPriority5 - Normal=>9 - Immediate
    Sun 13 Jun 2010 05:19:26 AM UTCarcanumStatusNone=>Ready For Test
      Assigned toNone=>arcanum
    Thu 03 Jun 2010 03:23:06 PM UTCwekAttached File-=>Added memcmp_PF.zip, #20682
    Fri 21 Dec 2007 02:40:29 PM UTCcarloslamasAttached File-=>Added extra pgmspace.zip, #14680

    Back to the top


    Powered by Savane 3.1-cleanup