[patch #6878] eewr_byte.S update for patch #6718

[Sebastian Gerhardt-2]

URL:
  <http://savannah.nongnu.org/patch/?6878>

                 Summary: eewr_byte.S update for patch #6718
                 Project: AVR C Runtime Library
            Submitted by: bpaddock
            Submitted on: Fri 24 Jul 2009 01:33:23 PM EDT
                Category: None
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email:
             Open/Closed: Open
         Discussion Lock: Any

    _______________________________________________________

Details:

I was not sure if you could attach a patch to a closed patch (#6718)?

Attached is eewr_byte.S that has been tested in a XMega128A1.




    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Fri 24 Jul 2009 01:33:23 PM EDT  Name: eewr_byte.S  Size: 5kB   By:
bpaddock
XMega tested eewr_byte.S
<http://savannah.nongnu.org/patch/download.php?file_id=18468>

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Sebastian Gerhardt-2]

Follow-up Comment #1, patch #6878 (project avr-libc):

No longer push/pop r18, use r20 instead.
Replaced XL with correct r19.
Corrected spelling.



(file #18531)
    _______________________________________________________

Additional Item Attachment:

File name: eewr_byte.S                    Size:4 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Sebastian Gerhardt-2]

Follow-up Comment #2, patch #6878 (project avr-libc):

There is a problem using r20 register: it is used to count bytes to write in
eewr_block (look in eedef.h) that uses eewr_byte function. If you use that
register you will overwrite the counter and the eeprom_write_block function
will not work anymore.

There is another point: there's no need to look for NVM busy after a buffer
erase, it will not erase eeprom locations but only the buffer.

I tested the eewr_byte function with all the write/update functions
(word/dword/block) because they all uses the byte write function.

I suggest to update the CVS with this file, no need to change the other write
and update functions.

P.S. The function 'eeprom_write_r18' uses one more scratch register, r17.
Update "readme_eeprom.txt" accordingly.

(file #18534)
    _______________________________________________________

Additional Item Attachment:

File name: eewr_byte.S                    Size:4 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Messaggio inviato con/da Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Bob Paddock-3]

> Follow-up Comment #2, patch #6878 (project avr-libc):
>
> There is a problem using r20 register: it is used to count bytes to write in
> eewr_block (look in eedef.h) that uses eewr_byte function. If you use that
> register you will overwrite the counter and the eeprom_write_block function
> will not work anymore.

Thanks for finding that.  I'll look at the rest of the files to see
what is free.

> There is another point: there's no need to look for NVM busy after a buffer
> erase, it will not erase eeprom locations but only the buffer.

Table 28-4 for command number 0x36 indicates that busy should be used,
28.11.5.2 says the same.

> I tested the eewr_byte function with all the write/update functions
> (word/dword/block) because they all uses the byte write function.
>
> I suggest to update the CVS with this file, no need to change the other write
> and update functions.

I agree that is the way it works now.  I think it would be better if they
were changed to use mapped memory.  The time overhead for a block could
be reduced by about 32 times (32 byte buffer) by removing the call to
byte write so many times.
I was hopping to get to this over the weekend.  Change
byte/write/dwrite to simply
be block writes of 1/2/4 bytes.

> P.S. The function 'eeprom_write_r18' uses one more scratch register, r17 would it not?
> Update "readme_eeprom.txt" accordingly.

I'm not sure I follow?  I don't see that r17 is used, at least not
finding with grep in the .S files.
Using r17 would be in confilct with what the FAQ says:
"Call-used registers (r18-r27, r30-r31):
May be allocated by gcc for local data. You *may* use them freely in
assembler subroutines. Calling C subroutines can clobber any of them -
the caller is responsible for saving and restoring."

Maybe you mean readme_eeprom.txt does need updated with the new
register usage, which won't be r17?
I'll take care of that too.

> (file #18534)
> File name: eewr_byte.S                    Size:4 KB


_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Sebastian Gerhardt-2]

Additional Item Attachment, patch #6878 (project avr-libc):

File name: eerd_byte.S                    Size:2 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Sebastian Gerhardt-2]

Follow-up Comment #3, patch #6878 (project avr-libc):

With Simone's help via email we got a couple of more cycles
out of the XMega byte write, and no longer trashing registers
of the callers.


(file #18555)
    _______________________________________________________

Additional Item Attachment:

File name: eewr_byte.S                    Size:5 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Sebastian Gerhardt-2]

Update of patch #6878 (project avr-libc):

             Assigned to:                    None => dmix                  


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Sebastian Gerhardt-2]

Update of patch #6878 (project avr-libc):

                  Status:                    None => Done                  
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #4:

Bob and Simone, thanks for doing this!
Now the patch for 'eewr_byte.S' is commited without any
severe changes to both HEAD and 1_6 branches.
As I understand, it is not needed to change the 'eerd_byte.S'
(at least the file #18546 does not contain any differences with CVS).


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6878>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

[Bob Paddock-3]

On Thu, Sep 10, 2009 at 1:50 AM, Dmitry Xmelkov <INVALID.NOREPLY@...> wrote:
>
> Update of patch #6878 (project avr-libc):


>Bob and Simone, thanks for doing this!

You are welcome.

>As I understand, it is not needed to change the 'eerd_byte.S'
>(at least the file #18546 does not contain any differences with CVS).

It has been working fine in my application.

Someday one of us should look at redoing the XMega EEWrite class of routines
to use memory mapping, and make use of the XMega buffering.  I looked at this
briefly.  The hard part is dealing with write requests that wrap around at
the end of the buffer, as doing so would corrupt the data.  It was going
to take longer than I had in my schedule to fix that problem.


_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@...
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev