Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hello-pfs154 const string "Hello World!" is placed in code memory byte by byte #2

Closed
freepdk opened this issue Jan 30, 2019 · 36 comments

Comments

@freepdk
Copy link

freepdk commented Jan 30, 2019

I tried to compile the hello pfs154 and saw that the const string "Hello World!" is placed in code memory const section byte by byte (2 byte per instruction word).

The PFS154 only can store 14 bit per instruction word which means that 2 bits of every second byte will be missing.

I think a byte array like "const char*" should use 1 instruction word per byte (wasting the upper bits).

BTW: Since this is a sdcc related thingy what would be best place to report this kind of problems?

@spth
Copy link
Collaborator

spth commented Jan 30, 2019

Yes, it shall be implemented as one ret k per byte with k for the byte value.
It just didn't get implemented yet (originally my preferred solution was to do this in the assembler, but now I'm also considering doing it in the compiler instead).

Best place is probably an SDCC tracker or mailing list.

Philipp

@spth
Copy link
Collaborator

spth commented Feb 2, 2019

The transition to ret k should be complete in current SDCC (revision 10908).

Philipp

@spth spth closed this as completed Feb 2, 2019
@freepdk
Copy link
Author

freepdk commented Feb 3, 2019

Hi,

great work. Saw the ret k produced now. So much better than mini-C already :-)

unfortunately a clean build does not produce a pdk14.lib (SVN branch revision 10908):

time.asm:1829: Error: machine specific addressing or addressing mode error
...

Also a disassembly of the final hex/bin file revealed that some CALL and GOTO targets are calculated incorrect (e.g. totally wrong destinations for last jump in main endless loop and call to external lib function "_puts")

JS

EDIT: pdk tools can now disassemble BIN files by supplying the OTP_ID:
dispdk 2AA1 test.bin

@freepdk freepdk reopened this Feb 3, 2019
@freepdk
Copy link
Author

freepdk commented Feb 3, 2019

Ok,
checked out your latest commit (rev 10912) and pdk14.lib was created fine.

looks I'm wrong. Only last jump in main seems to end up nowhere. No specific correlation to a label.

I investigated the wrong jump targets and it looks like it is caused by having the same ?local? label multiple times:

e.g. in .asm intermediate output you get something like

; function send_bit
_send_bit:
push af
...
mov a, _sending+0
ceqsn a, #0x00
goto 00104$
...
00104$:
...

; function main
_main:
...
00104$:
...
goto 00104$
...

==> The goto in main jumps to the label in first function defining it (_send_bit).


Also all "return" statements from inside interrupt should generate a "ret i".
Right now a normal ret is inserted:
;>test.c: 34: if(!sending)
mov a, _sending+0
ceqsn a, #0x00
goto 00104$
;>test.c: 35: return;
ret
00104$:
...

@spth
Copy link
Collaborator

spth commented Feb 3, 2019

The return interrupts is fixed now. Regarding the labels, I wonder why it works for other architectures, but not pdk14.

Philipp

@freepdk
Copy link
Author

freepdk commented Feb 3, 2019

Hi,

the assembly looks fine. Maybe assembler or linker is causing the problem.

Meanwhile I debugged the HelloPFS154 (I ported it back to mini-c) and found some issues

  1. in interrupt you have to reset the interrupt request for TM2. Something like this:
    if( intrq & 0x40 )
    {
    intrq &= ~0x40;
    ...

  2. in putchar you must reset the TM2 counter since TM2 always is counting (otherwise startbit has wrong length):

__sfr __at(0x1d) tm2ct;

after sending = true; insert this
tm2ct = 0; //reset timer counter for first bit

3.) clkmd = 0x30; crashes since it disables IHRC
clkmd = 0x34; is all you need, it already disables watchdog

4.) timer prescalers and calculation are wrong
tm2s = 0x60; is SYSCLK, /64, /1 ==> timer = SYSCLK/( 2 * PRESCALER * (SCALER+1) => 62.5 kHz
tm2b = 60; // Divide by 52 ~> 1201 Hz //60 by oscilloscope on PFS154

(Not sure why is so big difference 52 <==> 60 , maybe because I was not tuning the IC)

5.) just a note:
for(unsigned long int i = 0; i < 150000; i++); // Wait approx. 3s.
requires 10 more instructions than
for(unsigned long int i=150000; i>0; i--); // Wait approx. 3s.

I pushed the ported and corrected version to the "simple-pdk-code-examples"

@Rakete1111
Copy link

Rakete1111 commented Feb 5, 2019

@freepdk can you check again please?

Compiling your updated hello.c gives me for the last goto in main:

0000E6                        276 00104$:
...
00010A 73 30                  295   goto  00104$

0x73 * 2 == 0xE6 so it seems to be working for me.

@spth
Copy link
Collaborator

spth commented Feb 6, 2019

I made some fixes in the example code a few days ago. 1) and 2) are fixed as suggested. For 3) I had used the example from the datasheet, but looking at the details again, I now changed it (though I did it in two steps again, since the datasheet says, that the ILRC may not be disabled at the moment of the switch to the IHRC). I'll redo the calculations for 4) later and fix it then.

@freepdk
Copy link
Author

freepdk commented Feb 6, 2019

sdcctestjump.zip

Hi,

I investigated a bit more (version 10921) and found out how to reproduce:

When using the HElloPFS example the jump is translated fine (as @Rakete1111 reported).

However when you insert more than 1 function call in the endless loop I get the problem.

In order to produce something usable I was replacing the printf("...") with a sequence of putchars(...):

main:

...

  for(;;)
  {
//    printf("Hello World!\n"); //does not work yet

    putchar('H');
    putchar('e');
    putchar('l');
    putchar('l');
    putchar('o');
    putchar(' ');
    putchar('W');
    putchar('o');
    putchar('r');
    putchar('l');
    putchar('d');
    putchar('!');
    putchar('\n');

    for(unsigned long int i = 0; i < 150000; i++); // Wait approx. 3s.
  }
}

When using only one putchar('X') call in the loop the jump is fine. As soon as I insert 2 or more putchar calls the JUMP of the loop is pointing to nowhere.

UPDATE:

It also happens if I insert more stuff to _sdcc_external_startup() function (for quick test I used "__asm" ... "__endasm;" and added 10 NOPs inside. So maybe an overflow in assembler when calculating jump targets?

=> I added the code as attachment (shown on top)

@Rakete1111
Copy link

Well that's embarrassing, my bit arithmetic was wrong when the address uses more than 8 bits :(. Thanks, here's a patch to fix this @spth

Index: sdas/linksrc/lkrloc3.c
===================================================================
--- sdas/linksrc/lkrloc3.c      (revision 10918)
+++ sdas/linksrc/lkrloc3.c      (working copy)
@@ -448,7 +448,7 @@
 
                         /* pdk addresses in words, not in bytes. */
                         rtval[rtp] /= 2;
-                        rtval[rtp] |= rtval[rtp + 1] & 1;
+                        rtval[rtp] |= (rtval[rtp + 1] & 1) << 7;
                         rtval[rtp + 1] /= 2;
 
                         /* Do the actual opcode fusion and ignore the extra

@freepdk
Copy link
Author

freepdk commented Feb 6, 2019

GREAT 🥇

This fixed the goto problem.

I can't express how happy I am :-)

So maybe today is the day when I can stop using the PADAUK IDE :-)

@freepdk
Copy link
Author

freepdk commented Feb 6, 2019

Hi,

almost there. I found 2 more issues using revision 10926 (manually disassembled and commented the final output binary ):

  1. assembler/linker does not insert position of "ret x" string

main:

...
;mov	a, #<(___str_1 + 0)
0x006c:   0x2f00    MOV A, 0x00    ;0 <================????
0x006d:   0x0b80    MOV [0x00], A
;mov	a, #((___str_1 + 0) >> 8)
0x006e:   0x2f00    MOV A, 0x00    ;0 <================????
0x006f:   0x0b81    MOV [0x01], A
;call	_puts
0x0070:   0x3884    CALL 0x084     ;CALL _puts
...
                                    295 	.area CODE
                                    296 	.area CONST
      00015C                        297 ___str_1:
      00015C 48 02                  298 	ret #0x48	; H
      00015E 65 02                  299 	ret #0x65	; e
      000160 6C 02                  300 	ret #0x6c	; l
      000162 6C 02                  301 	ret #0x6c	; l
      000164 6F 02                  302 	ret #0x6f	; o
      000166 20 02                  303 	ret #0x20	;  
      000168 57 02                  304 	ret #0x57	; W
      00016A 6F 02                  305 	ret #0x6f	; o
      00016C 72 02                  306 	ret #0x72	; r
      00016E 6C 02                  307 	ret #0x6c	; l
      000170 64 02                  308 	ret #0x64	; d
      000172 21 02                  309 	ret #0x21	; !
      000174 00 02                  310 	ret #0x00
                                    311 	.area CABS (ABS)
  1. __gptrget seems wrong (using IDXM to access memory but should use PCADD to jump to "ret x" position)

=> EDIT... found the source of __gptrget and it looks fine. Your idea was even better to manipulate the return pointer on stack for the long jump :-)

@freepdk
Copy link
Author

freepdk commented Feb 7, 2019

Update,

Form .lst file:

...
      0000DE CA 30                  301  goto 00108$
      0000E0                        302 00112$:
                                    303 ; test.c: 107: }
      0000E0 7A 00                  304  ret
                                    305  .area CODE
                                    306  .area CONST
      000000                        307 ___str_1:
      000000 48 02                  308  ret #0x48>; H
      000002 65 02                  309  ret #0x65>; e
...

As you can see the .area CODE . area CONST is starting a new segment and therefore the address resets to 0.

After removing the new segment starts the string was placed directly after code and the MOV A,#(___str1+0) produced some value.

Unfortunately the value is still wrong and same value is inserted for lo and hi byte.

When looking at other processor (e.g. STM8) .lst file of assert.asm (also contains a string) you can see that some ?relocation symbols are visible (r / s before the inserted address):

assert.lst from stm8:

...
00000C 4Br00            [ 1]    64 push #<(___str_0 + 0)
00000E 4Bs00            [ 1]    65 push #((___str_0 + 0) >> 8)
...
                                72 .area CODE
                                73 .area CONST
000000                          74 ___str_0:
000000 41 73 73 65 72 74 28     75 .ascii "Assert(%s) failed in function %s at line %u in file %s."

However the .lst file from pdk14 does not contain any of those markers:

      0001E8 00 2F                  277  mov a, #<(___str_1 + 0)
                                    278 ; mov _puts_PARM_1+0, a
      0001EA 00 2F                  279  mov a, #((___str_1 + 0) >> 8)
                                    280 ; mov _puts_PARM_1+1, a
...
                                    305 .area CODE
                                    306 .area CONST
      000282                        307 ___str_1:
      000282 48 02                  308 ret #0x48>; H
      000284 65 02                  309 ret #0x65>; e
      000286 6C 02                  310 ret #0x6c>; l

@spth
Copy link
Collaborator

spth commented Feb 11, 2019

Timer 2 setup should be fixed now.

Philipp

@freepdk
Copy link
Author

freepdk commented Feb 11, 2019

Nice :-)

So still 2 things to go:

  1. the relocation of the CONST segment from assembler is broken (see above)

  2. CLKMD is still wrong. Here the CLKMD bit definitions from PFS154.INC:

	CLKMD		IO_RW		0x03
		$ 7 ~ 5, 3 :	IHRC/4, IHRC/16, IHRC/2, IHRC/8,
				X, ILRC/16, EOSC/4, IHRC/32,
				EOSC/2, IHRC/64, EOSC/1, EOSC/8,
				ILRC/4, X, ILRC/1
		$ 4	:	X, En_IHRC
		$ 2	:	X, En_ILRC
		$ 1	:	X, En_WatchDog
		$ 0	:	X, En_Reset

->

Bit:
7 6 5 - 3

0 0 0 - 0  IHRC/4
0 0 0 - 1  IHRC/16
0 0 1 - 0  IHRC/2
0 0 1 - 1  IHRC/8
0 1 0 - 0  X
0 1 0 - 1  ILRC/16
0 1 1 - 0  EOSC/4
0 1 1 - 1  IHRC/32
1 0 0 - 0  EOSC/2
1 0 0 - 1  IHRC/64
1 0 1 - 0  EOSC/1
1 0 1 - 1  EOSC/8
1 1 0 - 0  ILRC/4
1 1 0 - 1  X
1 1 1 - 0  ILRC/1
1 1 1 - 1  X

Bit 4:
0: X
1: En_IHRC

So the value of 0x38 would translate to:

0 0 1 1  1 0 0 0
0 0 1 -  1  = IHRC/8
      1     = En_IHRC

-> instead of selecting IHRC/2 you select IHRC/8

The 0x3c translates to:

0 0 1 1  1 1 0 0
0 0 1 -  1       = IHRC/8
      1          = En_IHRC
           1     = En_ILRC

PADAUK IDE inserts CLKMD=0x34 when selecting IHRC/2 from wizard (it even shows "Clkmd = 34h") in wizzard)

@spth
Copy link
Collaborator

spth commented Feb 11, 2019

I see. I'll change it to

	clkmd = 0x34; // Use IHRC / 2 = 8 Mhz for system clock, disable watchdog.
	clkmd = 0x30; // Disable ILRC

Philipp

@spth
Copy link
Collaborator

spth commented Feb 11, 2019

Relocation seems to be still broken also for DATA (interestingly, the values in the .map file look correct, but the ones in the .ihx look wrong).

@Rakete1111
Copy link

r10969 should fix @freepdk's issue along with some other relocation issues found. Feel free to check again :)

@freepdk
Copy link
Author

freepdk commented Feb 14, 2019

@Rakete1111
Almost fixed.

  1. The address inserted seems to be double the value required (maybe it uses the byte address but the instruction word address is needed):

In my case the string is at word position 0x103 (@0x206 in bytes if you look at the binary output)

word offset, instruction, asm:
0x008b: 0x2f06 MOV A, 0x06
0x008c: 0x0b11 XOR [0x11], A
0x008d: 0x2f82 MOV A, 0x82
0x008e: 0x0b12 XOR [0x12], A
0x008f: 0x38d9 CALL 0x0D9 ;call to puts

0x06 0x82 => 0x206 but should be 0x103

2). The MOV MEM,A was somehow translated wrong to a XOR MEM,A with latest version (see above).
Maybe relocation code did it since ASM and RST file show correct assembly instructions but wrong opcodes (looks like relocator used a complete byte as mem address and cleared bit 7 which makes the MOV to a XOR):

  0000DCr00r2F                  299 mov a, #<(___str_1 + 0)
  0000DEr80r0B                  300 mov _puts_PARM_1+0, a
  0000E0r00r2F                  301 mov a, #>(___str_1 + 0x8000 + 0)
  0000E2r81r0B                  302 mov _puts_PARM_1+1, a
  0000E4r00r38                  303 call _puts

@freepdk
Copy link
Author

freepdk commented Feb 15, 2019

@spth
Revision 10970 calculates the correct offset now but your magic marker (+0x80) gets divided by 2 as well:

0x008b: 0x2f03 MOV A, 0x03
0x008c: 0x0b91 MOV [0x11], A
0x008d: 0x2f41 MOV A, 0x41 ;<== should be 0x81 ???
0x008e: 0x0b92 MOV [0x12], A
0x008f: 0x38d9 CALL 0x0D9

@spth
Copy link
Collaborator

spth commented Feb 15, 2019

Yes. Same for the offset from the start of the object, so in an array / struct in ROM, all members other than the first have a wrong address.

Philipp

@Rakete1111
Copy link

@freepdk Should be fixed now in r10971. :)

@freepdk
Copy link
Author

freepdk commented Feb 15, 2019

@spth @Rakete1111
Fixups are looking good for the example code, however there is still one last glitch in __gptrget final assembly output:

0x00b4:   0x2980    SUB A, 0x80
0x00b5:   0x1840    T0SN IO(0x00).1  ;FLAG.CF
0x00b6:   0x30b9    GOTO 0x0B9
0x00b7:   0x0381    IDXM A, [[0x00]]
0x00b8:   0x007a    RET
0x00b9:   0x0072    PUSHAF
0x00ba:   0x01c2    MOV A, IO(0x02)  ;SP
0x00bb:   0xffff    ?????                               ;<===== ????????????
0x00bc:   0x1380    XCH A, [0x00]
0x00bd:   0x0380    IDXM [[0x00]], A
0x00be:   0x007a    RET

Not sure if it was caused from relocate, linker or if pdk14.lib causes the trouble. Unfortunately the .rst / .lst file does not show the library code which is linked to it. So I used "dispdk 2AA1 output.bin"

Here is the source I used. Compiled with revision 10971 (clean sdcc build)
sdcc-test3.zip

@spth
Copy link
Collaborator

spth commented Feb 15, 2019

The .lst for __gptrget is in sdcc/device/lib/pdk14/__gptrget.lst. That instruction is an add a, #-1, which looks still ok in that .lst. Don't know what happened to it later.

Philipp

@Rakete1111
Copy link

I have no idea what is causing this. I presume this is because a value of 0xFF seems to violate some the linker's assumption about relocatable addresses. Anyways, this is trivially fixable by just not emitting relocatable data for constant instructions, which makes sense anyways. :) Here's a patch @spth:

Index: sdas/aspdk/pdkmch.c
===================================================================
--- sdas/aspdk/pdkmch.c (revision 10971)
+++ sdas/aspdk/pdkmch.c (working copy)
@@ -36,7 +36,12 @@
 }
 
 static VOID outpdkrm(struct inst inst, struct expr e) {
-        outrwp(&e, inst.op, inst.mask, /*jump=*/0);
+        /* Don't generate relocatable data if everything is constant. */
+        if (is_abs(&e)) {
+                outpdkaw(inst, e);
+        } else {
+                outrwp(&e, inst.op, inst.mask, /*jump=*/0);
+        }
 }
 
 static VOID outpdka(struct inst inst) {

@spth
Copy link
Collaborator

spth commented Feb 15, 2019

Thanks for the patch.

Would the 0xff issue also affect variables that happen to be in memory at address 0xff?

Philipp

@freepdk
Copy link
Author

freepdk commented Feb 15, 2019

Output looks great now :-)

Will try to write and execute it soon.

@spth:
One last thing missing is setting the IHRCR tuning register.

PFS154 has 2 factory tuning values located as RET x at:

0x7ED: IHRCR_FACTORY
0x7EE: BGTR_FACTORY

BGTR is bandgap tuning register (the internal 1.2V reference) needed for ADC or COMParator
IHRCR is the internal high speed rc tuning register needed if you use IHRC (which the example does)

I found that if you do not set the IHRCR (value stays =0) then the IC frequency is around 9.5MHz instead of 16MHz. A value of 0xFF will overclock it to around 24MHz :-)

Details here:
http://www.eevblog.com/forum/blog/eevblog-1144-padauk-programmer-reverse-engineering/msg2190855/#msg2190855

So either you insert a FIXED call to 0x7ED, get the IHRCR factory value in A and then move this IHRCR or you just use the "middle value" of 0x80 (which is the factory value of 5 PFS154 i checked) and move this to IHRCR directly.

@spth
Copy link
Collaborator

spth commented Feb 15, 2019

Is the value 0x7ed really a factory calibration value? From my understanding of the thread over at eevblog, it seemed to be a value written by the Padauk programmer, so the value at 0x7ed would not be available when programming the µC a different way.

Philipp

@freepdk
Copy link
Author

freepdk commented Feb 15, 2019

@0x7ED and 0x7EE are the 2 words of PFS154 which even a erase can not clear.

Writer writes after calibration to 0x7FE
from forum thread: => @7fe IHRCR value from WRITER: 0x81 (used for normal execution)

So yes, the 2 values are coming from factory not erasable.

My guess is that they introduced this so a PFS154 can be programed in circuit when writer has no access to the extra IO for tuning.

@Rakete1111
Copy link

Would the 0xff issue also affect variables that happen to be in memory at address 0xff?

No, it would affect addresses greater than 0x3FFF, i.e. using more than 14 bits. That's because those last bits are treated specially by the linker and if they are set, like by using #-1 then they are ORed against the opcode which turns 0x2F into 0xFF :).

@spth
Copy link
Collaborator

spth commented Feb 16, 2019

@0x7ED and 0x7EE are the 2 words of PFS154 which even a erase can not clear.

Writer writes after calibration to 0x7FE
from forum thread: => @7fe IHRCR value from WRITER: 0x81 (used for normal execution)

So yes, the 2 values are coming from factory not erasable.

I see. Where is this ihrcr register located?
How about the PFS173?

Philipp

@freepdk
Copy link
Author

freepdk commented Feb 16, 2019

On PFS154 IHRCR is at IO 0x0B:

__sfr __at(0x0b) ihrcr;

so init could simply do this (this might work for all PADAUK ICs since 0x80 seems to be the middle position of tuning, everything below tunes down the frequency and above tunes up, most likely the IC is designed to function with 16MHz when IHRCR is 0x80. Manufacturing differences might require slighty smaller / bigger values. Much different values can be used to tune to completely different frequencies):

   mov a, #0x80
   mov ihrcr, a

On PFS154 you also could call the factory fixed address:

   call #0x7ED   ;this will work for PFS154 only, all OTP variants do not have a RET there. 
   mov ihrcr, a

PFS173 has bigger memory so the position will be different for sure. I did not have the time to play with this type yet. Right now I focus on PFS154 to get a stable tool chain for compile, flash, execute, tune, debug, ...

@spth
Copy link
Collaborator

spth commented Feb 16, 2019

The current versions now use the factory calibration value for IHRC.

Philipp

@freepdk
Copy link
Author

freepdk commented Feb 16, 2019

@spth

Can you explain this cast and number magic?

ihrcr = *((const unsigned char*)(0x87ed));

ptr = 0x8000 + 0x7ED
ihrcr = *ptr;

  • the +0x8000 seems to switch reference mode from RAM to CodeMem?
  • the (unsigned char*) cast uses just the byte from it?
  • is it also possible to get 16 bit (13/14/15) with another cast?

@spth
Copy link
Collaborator

spth commented Feb 17, 2019

The (unsigned char*) makes a pointer to unsigned char from the integer. The topmost bit indeed is used by SDCC to mark pointers to code memory.
There currently is no way to get the upper 5/6/7 bit from a code memory location, for two reasons: 1) The first goal was to implement support for standard C types, so what I needed was the 8-bit case. 2) It is not clear to me how to get the upper bits. pdk15 has ldtabh, but pdk13 and pdk14 only have ldspth, which, AFAIK, is not supported by all devices.
So if you want to get the upper bits, for now, you'll have to use assembler (e.g. by using SDCC inline assembler or just writing a function in an assembler source file and linking it together with your C code).

SDCC currently supports pdk14 and pdk15; I hope we will get pdk13 support soon; pdk16 support is much harder, and will take longer.

Philipp

@freepdk
Copy link
Author

freepdk commented Mar 7, 2019

GREAT!

Everything is working.

I did some small changes so the sample can be used with easy pdk programmer directly (move output to PA.7). It also can use a higher baud rate (115200).
The delay loop is much smaller in code memory when counting down to 0.

See PR #3

This issue can be closed :-)

@spth spth closed this as completed Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants