Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2018-12

Writing to 16 bit timer registers on AVR (bug #34721)

4 messages · 3 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Christo Crause

According to AVR manuals 16 bit timer registers share a temp register which requires a specific sequence for reading/writing: "To do a 16-bit write, the high byte must be written before the low byte. For a 16-bit read, the low byte must be read before the high byte." - from atmega328p 15.3 Accessing 16-bit Registers. I'm trying to patch the AVR code generator to respect this sequence but I would appreciate some review of my attempt and suggestions on how to improve it.

Attached please find a patch where I added code to avr/cgcpu methods a_load_reg_ref and g_concatcopy. I try to identify 16 bit io registers by checking whether the offset of the reference falls within 32 to srambase-1 and the size is OS_16/OS_S16 (or length=2). With this patch the compiler generates (what I assume is) correct code for the following test cases which should write to register 0x87 then 0x86 (-Wpatmega328p, so timer1 is 16-bit, -O2):

var x: uint16 = 1023;
ICR1 := x;
lds r0, 0x0101
sts 0x0087, r0
lds r0, 0x0100
sts 0x0086, r0

ICR1 := 1234;
ldi r18, 0xD2
ldi r19, 0x04
sts 0x0087, r19
sts 0x0086, r18

ICR1 := OCR1A; // should first read 0x88 then 0x89 and then write 0x87, 0x86 lds r19, 0x0088
lds r18, 0x0089
sts 0x0087, r18
sts 0x0086, r19

x := OCR1A; // read from io reg should be low byte first lds r0, 0x0088
sts 0x0100, r0
lds r0, 0x0089
sts 0x0101, r0

Any further test cases I should consider (also other code which should not trigger this modification) and ways to improve the patch would be appreciated.

Regards,
Christo

Reply by email

Florian Klämpfl
to Christo Crause
Christo Crause wrote 39 lines
According to AVR manuals 16 bit timer registers share a temp register which requires a specific sequence for reading/writing: "To do a 16-bit write, the high byte must be written before the low byte. For a 16-bit read, the low byte must be read before the high byte." - from atmega328p 15.3 Accessing 16-bit Registers.  I'm trying to patch the AVR code generator to respect this sequence but I would appreciate some review of my attempt and suggestions on how to improve it. Attached please find a patch where I added code to avr/cgcpu methods a_load_reg_ref and g_concatcopy. I try to identify 16 bit io registers by checking whether the offset of the reference falls within 32 to srambase-1 and the size is OS_16/OS_S16 (or length=2).  With this patch the compiler generates (what I assume is) correct code for the following test cases which should write to register 0x87 then 0x86  (-Wpatmega328p, so timer1 is 16-bit, -O2): var x: uint16 = 1023; ICR1 := x; lds    r0, 0x0101 sts    0x0087, r0 lds    r0, 0x0100 sts    0x0086, r0 ICR1 := 1234; ldi    r18, 0xD2 ldi    r19, 0x04 sts    0x0087, r19 sts    0x0086, r18 ICR1 := OCR1A; // should first read 0x88 then 0x89 and then write 0x87, 0x86 lds    r19, 0x0088 lds    r18, 0x0089 sts    0x0087, r18 sts    0x0086, r19 x := OCR1A; // read from io reg should be low byte first lds    r0, 0x0088 sts    0x0100, r0 lds    r0, 0x0089 sts    0x0101, r0 Any further test cases I should consider (also other code which should not trigger this modification) and ways to improve the patch would be appreciated.

Thanks, I have applied it in r40678 (with a small fix to make it compilable). I am not aware of other places where changes could be necessary.

Reply by email

Christo Crause
to Florian Klämpfl
Florian Klämpfl < wrote
Thanks, I have applied it in r40678 (with a small fix to make it compilable). I am not aware of other places where changes could be necessary.

Apologies for missing the tempreg2 declaration in the patch. I make modifications to the compiler in git and then manually copy the changes to svn trunk to generate a svn based patch. I would prefer a more direct way of generating patches between git and svn...

Reply by email

Jonas Maebe
to Christo Crause
Christo Crause wrote
Apologies for missing the tempreg2 declaration in the patch. I make modifications to the compiler in git and then manually copy the changes to svn trunk to generate a svn based patch. I would prefer a more direct way of generating patches between git and svn...

You can just attach git patches, as many compiler developers use git-svn. And even if not, a git patch easily be applied with patch -p1.

Jonas

Reply by email