Forum Rule: Always post complete source code & details to reproduce any issue!
Page 4 of 4 FirstFirst ... 2 3 4
Results 76 to 95 of 95

Thread: EEPROM writing on T_3.6 in HSRUN mode

  1. #76
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    Quote Originally Posted by defragster View Post
    As I read this code - it will take the passed in memory address and use that as the offset?
    Code:
    void eeprom_write_word(uint16_t *addr, uint16_t value)
    {
    	uint32_t offset = (uint32_t)addr;
    	if (offset >= EEPROM_SIZE-1) return;
    // ...
    			*(uint16_t *)(&FlexRAM[offset]) = value;
    // ...
    It seems to me all of these should just be expecting the EEPROM address (i.e. offset) - not some pointer?::
    void eeprom_write_word(uint16_t addr, uint16_t value)

    If you agree I can change the code and test - but I didn't bother before and now I remember why.
    These EEPROM functions originate from AVR libc. Some AVRs can actually map the EEPROM address space into data or program memory (which may or may not be the reason for the signature).

    For Teensy, just think of the EEPROM as a separate address space starting at 0 and passing in an address makes some sense.

    You don't want to change the signature, that will break code. Adding new functions like:
    void eeprom_write_word(size_t offset, uint16_t value)
    that cast offset to a "uint16_t*" and call
    void eeprom_write_word(uint16_t *addr, uint16_t value)
    may make sense. An inline function would get optimized away.

    That wouldn't be C-compatible though (overloaded function), I don't know if that is a requirement.
    Last edited by tni; 10-16-2016 at 05:45 PM.

  2. #77
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    Okay so not such a noob - problem. I saw it and dismissed it as some majique for some reason - and went on waiting to find out if my HSRUN code would work acceptably.

    But Given that it turns a pointer into an offset - used internally as >> FlexRAM[offset]).

    @tni :: AVR or not I'm not sure it could ever work or has ever been used - except like where KurtE did a NULL pointer that looked like a ZERO offset when actualized - and that would work just as well if the majique *pointer modifiers were taken off. It seems I did a TEENSYDUINO GREP and found no references that demonstrated this majique working.

  3. #78
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    Casting an int offset to a pointer value works just fine.

    Those write functions are part of the public AVR libc interface. People are using these functions. I think Paul intended to be compatible when he wrote the Teensy EEPROM stuff.

  4. #79
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    @tni - but a pointer to what? By design Only the EEPROM has a 'uint8_t pointer' FlexRAM. As I read that code the pointer is not dereferenced - it is taken at face value. Which only majiquely resolves out when a CONSTANT is used, any variable must be &variable - which passes a memory address????

    Can you code a sample sketch that works to increment all of EEPROM by byte and word - the _dword and _block are similarly written. I think this would compile but is nonsense [ given the compiler requires the &address_of_variable ] so the code is a waste of space on Teensy unless properly misused (hardcoded constants like KurtE sample) - and that would work the same if the pointer decoration were removed:

    Code:
    uint8_t val8;
    for ( uint8_t ii=0; ii<E2END; ii++ ) {
    val8 = eeprom_read_byte(  &ii );
    val8++;
    eeprom_write_byte( &ii, val8 )
    }
    Code:
    uint16_t val16;
    for ( uint16_t jj=0; jj<E2END; jj+=2 ) {
    val16 = eeprom_read_word(  &jj );
    val16++;
    eeprom_write_word( &jj, val16 )
    }

  5. #80
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    You don't want to pass an address to your integer. Just cast the int value to a pointer like this (untested code):
    Code:
    uint16_t val16;
    for ( uint16_t jj=0; jj<E2END; jj+=2 ) {
        val16 = eeprom_read_word( (uint16_t*) jj );
        val16++;
        eeprom_write_word( &jj, val16 );
    }
    Edit:
    What is happening with that code is something like this:
    Code:
    int offset = 5;
    uint16_t* offset_as_ptr = (uint16_t*) offset;
    uint32_t offset_cast_back = (uint32_t) offset_as_ptr;
    // offset and offset_cast_back are now both 5
    Last edited by tni; 10-16-2016 at 09:20 PM.

  6. #81
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    Exactly - nonsense.

    Expert low-level understanding and abusive casting needed to produce a simple number.

  7. #82
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    Perhaps this would be a much clearer solution and only uses 16KB of memory to access the 4KB of EEPROM? Of course it only works without extra casting unless an added majique array for byte and dword is created.

    Code:
    uint16_t *majique16[E2END];
    for ( int ii=0; ii<E2END; ii++ ) {
       majique16[ii] = ii;
    }
    
    uint16_t val16;
    for ( uint16_t jj=0; jj<E2END; jj+=2 ) {
      val16 = eeprom_read_word(  majique16[jj] );
      val16++;
      eeprom_write_word( majique16[jj], val16 )
    }
    ** Code not compiled and could clearly be optimized since only every other word value is used - saving half the memory - with needed adjustments made on usage.
    Code:
    uint16_t *majique16[E2END/2];
    for ( int ii=0; ii<E2END; ii+=2 ) {
       majique16[ii] = ii*2;
    }
    *** Clearly the optimal way to write this could be::
    Code:
    uint16_t *majique16 = 1;
    
    uint16_t val16;
    for ( uint16_t offset16=0; offset16<E2END; offset16+=2 ) {
      val16 = eeprom_read_word(  majique16*offset16 );
      val16++;
      eeprom_write_word( majique16*offset16, val16 )
    }
    Last edited by defragster; 10-16-2016 at 10:28 PM.

  8. #83
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    I don't think your code is any clearer. And 16kb is more RAM than a Teensy LC has in the first place.

    When I mentioned possible wrapper functions, this is what I had in mind:
    Code:
    #ifdef __cplusplus
    
    #define FORCEINLINE __attribute__((always_inline, unused))
    
    FORCEINLINE static uint8_t eeprom_read_byte(size_t offset);
    FORCEINLINE static uint16_t eeprom_read_word(size_t offset);
    FORCEINLINE static void eeprom_write_byte(size_t offset, uint8_t value);
    FORCEINLINE static void eeprom_write_word(size_t offset, uint16_t value);
    
    static uint8_t eeprom_read_byte(size_t offset) {
        return eeprom_read_byte((uint8_t*) offset);
    }
    static uint16_t eeprom_read_word(size_t offset) {
        return eeprom_read_word((uint16_t*) offset);
    }
    static void eeprom_write_byte(size_t offset, uint8_t value) {
        eeprom_write_byte((uint8_t*) offset, value);
    }
    static void eeprom_write_word(size_t offset, uint16_t value) {
        eeprom_write_word((uint16_t*) offset, value);
    }
    
    #endif
    You can put that into a header file, it won't conflict with the existing C functions.

  9. #84
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    Opps - I forgot the [sarcasm] flag There is nothing clear about this.

    My guess is these functions as written are not being used - or not used correctly on any Teensy. I was really hoping the right answer would be to correct the functions to be clear and correct for any Arduino programmer on Teensy.

    Your wrapper code would work - but also need _dword and _block versions. These MultiByte write functions would be MUCH more efficient than any other access method - especially if on a T_3.6 in HSRUN.

    It would be better to create wrapper functions FROM the AVR style TO the native Teensy to be clean and clear and efficient.
    Code:
    uint8_t eeprom_read_byte(const uint8_t *addr) {
        return eeprom_read_byte( (uint32_t)addr );
    }
    
    uint8_t eeprom_read_byte(const uint32_t offset)
    {
    	if (offset >= EEPROM_SIZE) return 0;
    	if (!(FTFL_FCNFG & FTFL_FCNFG_EEERDY)) eeprom_initialize();
    	return FlexRAM[offset];
    }
    In my code I took to using .get() and .put() [libraries\eeprom\eeprom.h] assuming they resolved out to efficient multibyte access when possible (using this prior code) - but now looking they just do byte copies for length sizeof(). Unlike the prior word and dword writes that go to some length to respect appropriate word boundaries - only to be unclear in proper usage. I should have looked at this before as I went to great lengths to make structs of various even and odd sizes to prove the boundaries are respected.

    <edit> above to added simplified code for native teensy read.
    Last edited by defragster; 10-17-2016 at 12:13 AM.

  10. #85
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    There already is a new version of the EEPROM lib, that does use the proper block write:
    https://github.com/PaulStoffregen/EE...aster/EEPROM.h

    For some reason it's not included in the current Teensyduino version.

  11. #86
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    tni noted a hole in my "pull request" where an interrupt coming as the interrupt was about to be disabled ended up with a flag set poorly to indicate enable_irq() on exit - such that the interrupts would be left off when exited the first call.

    Did my change fix it?: PULL REQUEST UPDATED :: PaulStoffregen/cores/pull/177

    <updated>::
    tni post #85 linked to this usage of eeprom_write_block with cast of (void *), it doesn't work.
    This works ** - of course - to write/read a WORD::
    Code:
      // void eeprom_write_word(uint16_t *addr, uint16_t value)
      eeprom_write_word( (uint16_t *)EE_MULT, iiWORD);
      uint16_t ii = eeprom_read_word( (uint16_t *)EE_MULT );
    ** ouch - so wrong::
    "makes integer from pointer with a cast "

    <update 2>:: The (void *) works on the _block read/write (even boundaries so far), and all three multibyte appear to be working under T_3.6 HSRUN at 180 MHz.
    Last edited by defragster; 10-20-2016 at 11:06 PM.

  12. #87
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    For T_3.6 HSRUN :: Added test code finally to use the eeprom_write_???? code for word, dword, block on even and odd boundaries and at 240 MHz it is working fine.

    These 7 writes (with compare) { word, word, dword, block[25], word, dword, block[25] }::
    At 240 MHz Looks like about [ range 2.7-5.7ms ] ms to write 64 bytes :
    At 120 MHz Looks like about [ range 11-15ms ] ms to write 64 bytes :
    So HSRUN writes work - and they work faster than dropping to 120 MHz to use them.

    **I'd rewrite those funcs but changing uint32_t *addr to uint32_t offset doesn't work without using new function names. ? ? ?

  13. #88
    Senior Member+ MichaelMeissner's Avatar
    Join Date
    Nov 2012
    Location
    Ayer Massachussetts
    Posts
    3,269
    Quote Originally Posted by defragster View Post
    For T_3.6 HSRUN :: Added test code finally to use the eeprom_write_???? code for word, dword, block on even and odd boundaries and at 240 MHz it is working fine.

    These 7 writes (with compare) { word, word, dword, block[25], word, dword, block[25] }::


    So HSRUN writes work - and they work faster than dropping to 120 MHz to use them.

    **I'd rewrite those funcs but changing uint32_t *addr to uint32_t offset doesn't work without using new function names. ? ? ?
    If you are using code that does dword access on odd boundaries, I recall there was a bug discussed in the 3.1/3.2 context that might have a similar 'feature' in 3.6/3.5. Basically in 3.1/3.2 there was one address where a load/store that is unaligned and straddles the two 'pages'. Since the compiler never generates unaligned addresses, you only see it if you are hand generating code and have data in just the wrong position. Unfortunately, I don't think I saved a link, and I can't find it now with a quick google search.

  14. #89
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    5,679
    Yes, in this case, the silently Teensy crashes - and it is very difficult to find the bug.
    It's the boundary of SRAM_L and SRAM_U (see the different manuals for more details)

    For example, from the T3.6 manual:
    NOTE
    Misaligned accesses across the 0x2000_0000 boundary are not
    supported in the ARM Cortex-M4 architecture.
    If it is aligned - for example a 32-Bit access on this boundary - it takes a cycle more than usual.
    Last edited by Frank B; 10-21-2016 at 12:36 PM.

  15. #90
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    Paul: If you see this - even preliminary feedback on the likely acceptance of this EEPROM write at HSRUN speeds method would appreciated. I didn't get any web search to suggest anyone has used this or any other method to get this to work. Perhaps you've asked your contacts at FreeScale or have your own idea? To date I've probably done <10,000 writes to any one location - but with no problems.
    If you have any "EEPROM usable" but otherwise 'basket case' T_3.6's I could suggest a sketch that would do some exhaustive attempt at a destructive test to see where the write cycle life of this scheme is.

    @MM & Frank - thanks for looking:: The post 87 was just a small test just to confirm the HSRUN drop code was properly placed on the various multi byte write functions. Starting on an even boundary it writes 2,4,31 bytes and then from that odd boundary writes 2,4,31 bytes again.

    It is interesting that EEPROM write times (again) are more like 3 times slower at half the processor speed. And most of the code in this case is the read/verify step, so the write portion at 120 MHz seems to take inordinately long time.

    This is Using the Teensy3 eeprom.c multi byte access code with this active to always do boundary aware writes:
    // Writing unaligned 16 or 32 bit data is handled automatically when
    // this is defined, but at a cost of extra code size. Without this,
    // any unaligned write will cause a hard fault exception! If you're
    // absolutely sure all 16 and 32 bit writes will be aligned, you can
    // remove the extra unnecessary code.
    //
    #define HANDLE_UNALIGNED_WRITES

  16. #91
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    20,597
    For anyone following this thread (but not the github pull request), I've merged this change, and reworked the HSRUN mode switching a bit. This will be in the upcoming 1.31-beta2 installer.

  17. #92
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    I've pulled from github and tested this to work against my current multi type EEPROM write testing with read back compares.

    What Paul did was ONLY change the F_CPU and not F_BUS. My efforts on 9/10/16 posts were doing both and that was behind my troubles/concerns over peripherals getting confused.

    ... if only I had known . . .

    I just found a problem at 120 MHz - will post on GitHub thread . . .

  18. #93
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,779
    Paul has updated the Github code to fix the <=120 MHz issue, and it runs fine at 6 speeds over 120.

  19. #94
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    5,432
    Great, looks like I need to sync up again and try things out, once I do that, I think my fork should be back in sync with the master version

  20. #95
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    5,679
    Yes, looks interesting.
    But there shoud be a good documentation, at a prominent place, that serial communication during EEPROM-access does not work. (for 3.6 @ >120MHz)
    ...to prevent questions...
    Last edited by Frank B; 10-23-2016 at 02:30 PM.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •