Dangerous Prototypes

Dangerous Prototypes => Bus Pirate Support => Pirate PIC programmer => Topic started by: udif on November 25, 2011, 11:32:48 am

Title: PIC programming adapter problem
Post by: udif on November 25, 2011, 11:32:48 am
Hi,

I have a PIC programming adapter for a long time (yes, this is where all those were going :-)), and now I wanted to add support for 18F2553 (I assume it is very close, but may have a different ID than the 18F2550?)

Anyhow, I started a few hardware tests (set the power supply on, and the aux pin on), and verified I can see +12.5V on the MCLR pin in the adapter using a voltmeter.

I also ran HVPselftest.exe but I get the following message when I try:

 Press Esc to exit, any other key to start the self-test


Code: [Select]
 Starting test!
 Opening Bus Pirate on com7 at 115200bps...
 Configuring Bus Pirate HVP Adapter...
 Going into Binary Bitbang mode.. Entering binary mode...
ok
 configuring pins as output...
 sending 01000000... ERROR: BusPirate replied with 0x40 instead of 0x01
OK
 Sending Command to power on : 11000000.... ERROR: BusPirate replied with 0xffffffc0 instead of 0x01
OK
 Voltage Probe measurement...,sending 00010100
 ADC Reading:  0 Volts
 Voltage Measurement: !!!!FAIL!!!!
 powering off.....
 ERROR: BusPirate replied with 0xffffff80 instead of 0x01
 Press any key to continue...

Anyone has any idea why is the self test failing?
Is this test supposed to test anything above what I already tested with my DVM?
If the HW is indeed OK, should I bother trying to fix this problem?

I'm running on Windows 7/64 bit, and I'm able to communicate with my BPv3a over COM7 at 115200 baud.
Here is the version info of my BP:

Code: [Select]
1-WIRE>i
Bus Pirate v3a
Firmware v5.10 (r559)  Bootloader v4.3
DEVID:0x0447 REVID:0x3003 (24FJ64GA002 A3)
http://dangerousprototypes.com
CFG1:0xF9DF CFG2:0x3F7F
*----------*
Pinstates:
1.(BR)  2.(RD)  3.(OR)  4.(YW)  5.(GN)  6.(BL)  7.(PU)  8.(GR)  9.(WT)  0.(Blk)
GND    3.3V    5.0V    ADC    VPU    AUX    -      OWD    -      -
P      P      P      I      I      O      I      I      I      I
GND    3.31V  4.77V  4.28V  2.67V  H      H      H      L      H
Power supplies ON, Pull-up resistors ON, Open drain outputs (H=Hi-Z, L=GND)
MSB set: MOST sig bit first, Number of bits read/write: 8
a/A/@ controls AUX pin

*----------*
1-WIRE>

thanks,
Udi
Title: Re: PIC programming adapter problem
Post by: ian on November 25, 2011, 12:31:23 pm
Hi Udi,

Yes, it only checks those things. I'm not sure why it you get the errors. Does the programming app work at all?
Title: Re: PIC programming adapter problem
Post by: udif on November 25, 2011, 01:06:35 pm
Hi Ian,

I haven't gotten to that, as I haven't finished soldering the board . Judging the results so far (seeing that I got the required 13V), I'll go ahead assembling the board.

Udi
Title: Re: PIC programming adapter problem
Post by: ian on November 25, 2011, 07:34:39 pm
Ok, thank you. Please keep me updated :)
Title: Re: PIC programming adapter problem
Post by: tayken on November 26, 2011, 04:59:58 pm
I had some error messages before. Cannot remember why and how, will do a check tonight to see if that is still the case (on windows7 - 64bit system). But I can use my adapter without any problem, but I use a Linux machine for that. There might be some problem in the test program related to communications, have to try it out.
Title: Re: PIC programming adapter problem
Post by: udif on November 27, 2011, 04:33:16 am
A few updates:

I'm using a USB RGB color changer as my PIC proto board (thank Ian for the free PCB).
Using http://www.diylife.com/2008/01/25/make- ... ing-light/ (http://www.diylife.com/2008/01/25/make-a-usb-color-changing-light/) as a reference, I found a few issues:
1. PGD and PGC are swapped on this PCB's ICSP connector.
2. The PCB is different than what's appearing the link above.

As a VCC source I ended up picking V+ from the BP's own ICSP, because I think V+ on the HVP adater's connector seems to be off by default.

This is what I got:

Code: [Select]
W:downloadspicprog.v0.2>picprog.exe -p buspirate -u com7 -s 115200 -c 18F2550
(Bus) Pirate PIC Programer v0.2

Initializing interface
115200
Entering binary mode
BP: Setup mode...
Setup peripherals...
(OK)
Found '18F2550' in programming database :) index = 0
Checking for 18F2550 attached to programmer...

Wrong device: 0X2A47 (ID: 0X152 REV: 0X7)

This was expected ofcourse. The 18F2553 is not supported yet, but it should be just an alias of the 18F2550, except for the ID code.

Visiting the SVN , I'm not sure which code to work on.
Under http://code.google.com/p/dangerous-prot ... atePICprog (http://code.google.com/p/dangerous-prototypes-open-hardware/source/browse/#svn%2Ftrunk%2FPiratePICprog) I see both 'software' and 'framework' which appear to be very similar. 'software' seems to be more recent though.
There is also OpenProg which is a different code base, but seems to support more chips.

Which one should I take? A RAEDME file on the SVN could help.
Title: Re: PIC programming adapter problem
Post by: robots on November 27, 2011, 12:42:22 pm
"software" directory is the correct one. Others are for programmers reference only.
Title: Re: PIC programming adapter problem
Post by: tayken on November 27, 2011, 01:57:15 pm
Under "software" folder, there is a file named "pic.c", all definitions are there. You have to edit that one. I couldn't find my BP yesterday, turns out I left it at the hackerspace, I'll try the test program in a bit.
Title: Re: PIC programming adapter problem
Post by: tayken on November 27, 2011, 04:09:40 pm
I verify that I see the exact same errors. I'll try to see what can be the problem. The thing is, if I turn power supplies on and then measure ADC, I get:
Code: [Select]
UART>W
Power supplies ON
UART>d
VOLTAGE PROBE: 4.26V
So now my guess is that there is a comm problem: Pins cannot be set as output and then power supplies cannot be turned on. Actually I see MODE and VREG LEDs flash but I still get the same error. If I dive a little deeper:

1) Configuring pins as output:
Sent: 01000000, Received: 0x40 (01000000), Expected: 0x01
Here we are trying to configure AUX, MOSI, CLK, MISO & CS as outputs. The return is current state of the pins and they should be low, so there is no error with the returned byte.

2) Sending command to power on:
Sent: 11000000, Received: 0xffffffc0, Expected: 0x01
Here we are trying to turn POWER pin on while keeping PULLUP, AUX, MOSI, CLK, MISO & CS low. The reply is in the same format, showing the state of the pins, so actually the received byte should be 11000000 (0xc0). There is definitely a problem here.

3) Voltage probe measurement:
Sent: 00010100, Received: ?, Expected: ?
With this command, an ADC measurement is taken and as an answer a 2 byte ADC reading is sent, with high 8 bits first.

4) Powering off:
Sent: ?, Received: 0xffffff80, Expected: 0x01
My guess is that now we are sending 0x0f, the reset command. The answer should be 0x01, with possible garbage data afterwards as UART is initialized again.

Ian, can you also try it out? If you also get the same problems, that means there is a problem with the self test program. I'll try to see if I can work out the code and spot any bugs there. As there is no QC sticker on the adapter, my guess is that this check is not being performed yet?

Edit: Made a mistake on powering off part, it is now corrected.
Title: Re: PIC programming adapter problem
Post by: Sjaak on November 27, 2011, 08:17:01 pm
[quote author="tayken"]2) Sending command to power on:
Sent: 11000000, Received: 0xffffffc0, Expected: 0x01
Here we are trying to turn POWER pin on while keeping PULLUP, AUX, MOSI, CLK, MISO & CS low. The reply is in the same format, showing the state of the pins, so actually the received byte should be 11000000 (0xc0). There is definitely a problem here.
[/quote]

I guess this is a cast from signed char to signed long int .
Title: Re: PIC programming adapter problem
Post by: ian on November 28, 2011, 10:10:23 am
Hi guys, sorry for my late reply here. I can't find my adapter right now, but I'll try to find it and do the test. To be honest, I wrote the self-test app in 5 minutes maybe a year+ ago, I don't really remember what is going on. Jamz may have recompiled the app with the updated serial library and it could be broken now, the bp frmware may be changed, maybe it is wrong version or? Just thinking about it makes me shudder ;) I know seeed used a self-test app to test the 20 developer units made.
Title: Re: PIC programming adapter problem
Post by: udif on December 02, 2011, 09:11:37 am
Due to my limited free time, and the fact that I used this as an excuse to learn Eclipse, this took me some time, but I've managed to get my 18F2553 programmed.
On the way I also found a small bug: The datafile pointer was declared within main() and was not initialized, so when the -t option was not specified, it was not written, but still != NULL, which caused no warning to be displayed for not selecting a filetype, and when the program tried to read or write the data file, an illegal address is called (datafile->WriteFile() for example).

The 18F2553 patch is simple, and is based on cloning the 18F2550 entry in pic.c, but changing the ID from 0x92 to 0x152.

Do you want me to submit a patch (how?), or to apply this myself (I have commit access to the bus pirate SVN but not to this project).

thanks,
Udi
Title: Re: PIC programming adapter problem
Post by: tayken on December 02, 2011, 06:38:44 pm
You can post your edit here and I can commit the changes to SVN.
Title: Re: PIC programming adapter problem
Post by: udif on December 03, 2011, 09:54:39 pm
Here is the patch. This fixes:
1. uninitialized pointer in main()
2. Adds 18F2553 support
3. Eases debugging when using the Eclipse IDE for debugging
4. Small typo fix in one of the messages

Code: [Select]
Index: pic.c
===================================================================
--- pic.c (revision 1529)
+++ pic.c (working copy)
@@ -43,6 +43,25 @@
  }
  },
  {
+ .name = "18F2553",
+ .ID = 0x152,
+ .family = FAMILY_18Fx5xx,
+ .memmap = {
+ [PIC_MEM_FLASH] = {
+ .base = 0x0000,
+ .size = 32*1024
+ },
+ [PIC_MEM_FUSE] = {
+ .base = 0x300000,
+ .size = 14
+ },
+/* TODO
+ [PIC_MEM_EEPROM] = {
+ .base =
+ }*/
+ }
+ },
+ {
  .name = "18F4550",
  .ID = 0x90,
  .family = FAMILY_18Fx5xx,
Index: main.c
===================================================================
--- main.c (revision 1529)
+++ main.c (working copy)
@@ -70,12 +70,18 @@
  uint32_t i;
  uint16_t PICidver, PICrev, PICid;
 
+#ifdef ECLIPSE_DEBUG
+// WIthout these, console output within Eclipse will be buffered
+ setvbuf(stdout, NULL, _IONBF, 0);
+ setvbuf(stderr, NULL, _IONBF, 0);
+#endif
+
  char *param_write_file = NULL;
  char *param_read_file = NULL;
  char *param_prog = NULL;
  char *param_port = NULL;
  char *param_speed = NULL;
- struct file_ops_t *datafile;
+ struct file_ops_t *datafile = NULL;
  char *param_chip = NULL;
  uint16_t cmd = 0;
 
@@ -88,7 +94,7 @@
  struct memory_t *memwrite;
 
 
- printf("(Bus) Pirate PIC Programer v0.2 nn");
+ printf("(Bus) Pirate PIC Programmer v0.2 nn");
 
 #ifdef DEBUG
  cmd |= CMD_ERASE;
Title: Re: PIC programming adapter problem
Post by: robots on December 03, 2011, 10:04:36 pm
Kinda scary, that every developer would add DEBUG option for his/hers favorite debugging program ...

Why not make it less specific, and name the define DEBUG instead of ECLIPSE_DEBUG ?
Title: Re: PIC programming adapter problem
Post by: robots on December 03, 2011, 10:08:19 pm
Could you attach the patch file, rather than attaching the content? So we can keep the formating consistent (tabs are translated to spaces, etc etc)
Title: Re: PIC programming adapter problem
Post by: udif on December 03, 2011, 11:08:30 pm
Here is the patch file (#ifdef changed to DEBUG).
Title: Re: PIC programming adapter problem
Post by: robots on December 03, 2011, 11:14:41 pm
Thanks :) .... commited
Title: Re: PIC programming adapter problem
Post by: tayken on December 04, 2011, 07:31:00 pm
Sorry, was busy a lot today with Make. Nice thing that it got committed. Yay, we have another PIC in the library! :)

PS: I have two more that I'll add soon but ssshhhh :)
Title: Re: PIC programming adapter problem
Post by: robots on December 04, 2011, 10:00:36 pm
I have some clean-ups comming

It would be nice if someone could add pic16 protocol code :)
Title: Re: PIC programming adapter problem
Post by: udif on December 05, 2011, 12:22:01 am
Am I reading pic18.c correct , and writing configuration bits for 18Fxxx is not supported yet?
Title: Re: PIC programming adapter problem
Post by: tayken on December 05, 2011, 12:05:49 pm
[quote author="robots"]It would be nice if someone could add pic16 protocol code :)[/quote]
Ooops. I assumed that we have that. I had a 16f and a 12f in mind for adding the list. I can start looking at that once I finish all the stuff I had to postpone because of Make. And yes, that includes university stuff too. :S

[quote author="udif"]Am I reading pic18.c correct , and writing configuration bits for 18Fxxx is not supported yet?[/quote]
I was able to flash a PIC18f without any problems but that I have to check. Config bits should be there, that was the problem with the major haul. Are you looking the pic18.c file inside software folder? The ones inside software folder are the ones most up to date.
Title: Re: PIC programming adapter problem
Post by: udif on December 05, 2011, 12:53:06 pm
[quote author="tayken"][quote author="udif"]Am I reading pic18.c correct , and writing configuration bits for 18Fxxx is not supported yet?[/quote]
I was able to flash a PIC18f without any problems but that I have to check. Config bits should be there, that was the problem with the major haul. Are you looking the pic18.c file inside software folder? The ones inside software folder are the ones most up to date.[/quote]

Yes, I was looking into the one under 'software'.
I'm basing my conclusions on http://http://ww1.microchip.com/downloads/en/DeviceDoc/39622L.pdf

The reading part is OK, since reading configuration registers is identical to reading flash memory.
Quoting section 4.1 from the document above:
Quote
This technique will work to read any memory in the 000000h to 3FFFFFh address space, so it also applies to the reading of the ID and Configuration registers.

The writing part in pic.c treats flash memory the same way as configuration bits:
Code: [Select]
int PIC_WriteMemory(struct picprog_t *p, struct memory_t *mem)
{
        struct pic_chip_t *pic = PIC_GetChip(p->chip_idx);
        struct pic_family_t *fam = PIC_GetFamily(pic->family);
        struct proto_ops_t *proto = PIC_GetProtoOps(p->chip_idx);

        struct mem_page_t * page;

        proto->EnterICSP(p, fam->icsp_type);

        page = MEM_GetFirstPage(mem);
        while (page != NULL) {
                printf("Writing page %04lx... n", (unsigned long)page->base);

                // TODO: check address against pic memory map! EEPROM and Fuses might need separate handling
                proto->Write(p, page->base, page->data, page->size);
                page = MEM_GetNextPage(page);
        }

        proto->ExitICSP(p, fam->icsp_type);

}
As you can see, this code doesn'r care if the current page is flash memory or configuration registers.
However, configuration memory is written differently than flash memory.
Flash memory writing procedure is documented in section 3.2 of the spec above, while configuration register writing procedure is described in section 3.6
The differences are minor (a slightly different setup for the config registers, and each byte is written separately).
I plan adding it this week, since I have a blank PIC, and the bus pirate is my only PIC programmer.

There are two lines in the current code (pic18.c) that simply doesn't appear in the 2550 programming document (Table 3.5):
Lines 201-202:
Code: [Select]
        iface->PIC416Write(0x00, 0x6AA6);
        iface->PIC416Write(0x00, 0x88A6);
Line180:
Code: [Select]
        iface->PIC416Write(0x00, 0x84A6); //enable page writes

Are these really required? perhaps they were left here out of trial and error and never removed?
Title: Re: PIC programming adapter problem
Post by: robots on December 05, 2011, 02:07:18 pm
[quote author="udif"]
As you can see, this code doesn'r care if the current page is flash memory or configuration registers.
However, configuration memory is written differently than flash memory.

... snip ...

Are these really required? perhaps they were left here out of trial and error and never removed?[/quote]

Doesn't care is not the correct phasing. Not implemented is better :). Also the protocol methods are not prepared for fuses write. I will have expand the protocol structure.
I also have another changes in mind, that should ease maintenance of the application

I believe that Ian was the one who came up with the pic18 programming routines. I just copied/pasted them into this software.
Title: Re: PIC programming adapter problem
Post by: robots on December 05, 2011, 02:35:49 pm
I have just committed bunch of changed to the PiratePicProg repository. Mainly cleanups, and reorganization of the programming protocols.

I think the programming protocols should be extended with ReadFuse, WriteFuse and ReadEEPROM, WriteEEPROM functions.
Problem is with devices that share same space for flash and fuses. In that case solution number 2 should be used: Handle fuses and EEPROM in existing Read/Write functions.

Any suggestions?
Title: Re: PIC programming adapter problem
Post by: udif on December 05, 2011, 08:38:32 pm
I took a look at your latest commit.

I'm all for what you refer to as solution 2.
I don't think you should add special functions for EEPROM and flash. I think that the main code should keep calling the generic Read/Write functions, and let the device/family-specific code handle the different memory types.
After all, as far as the HEX file is concerned, configuration writes is simply data at 0x300000. There is no reason to expose all the memory types at the generic layers.

For example, the 18F read/Write code should detect that writes to 0x300000 are configuration writes and treat them as such.

Since the current API calls for transferring a pointer and a length for the memory to be read/written, then either the family/device-specific code needs to handle that (i.e. if you get a 32 byte write request to configuration space, you need to break it into byte writes yourself), or the Read/Write function will return the actual number of bytes read/written , and the calling code will need to adjust.

You can have separate read/write routines for EEPROM/flash/config/whatever, but I think they should be kept static, and be called from the main read/write functions according to the tblptr.
Title: Re: PIC programming adapter problem
Post by: udif on December 07, 2011, 08:42:49 am
Here is my first take at writing configuration bits for the 18F2553 (And probably other 18Fxxx devices).
It is still not working 100%, but I want to give it an early review by others, who may spot the remaining bugs.

At the moment it seems to write only SOME of the configuration words, or perhaps some of these are write only and can't be read back (I haven't read the whole configuration registers descriptions yet). The first two bytes and the last 6 always seems to be OK, but the middle 6 always seems to be partially corrupted (only 1-2 bytes are OK, and one of the values is in the wrong address).
My test file was the diolan bootrom from the USB IR Toy repository, which I hand-edited to use a 8MHz xtal instead of the original 20Mhz (0x300000: 0x24 -> 0x21, and fixed the checksum).

I also removed some unused functions from pic24.c and pic16.c (ReadFlash and WriteFlash) because I removed those fields from proto_ops_t. (Dead code can always be resurrected from version control - that's what its there for).

I will probably end hooking up my Open Logic Sniffer to see what's really coming out of the Bus Pirate (now we only need to code an ICSP mode to the OLS software).

comments are welcomed!
Title: Re: PIC programming adapter problem
Post by: robots on December 07, 2011, 11:37:23 am
I have quickly looked at the patch, and there are quite few problems:

1. Static functions are static for a reason, and not to be exposed in *.h files.

2. Write_mem_table and read_mem_table use the same type, Why not have them typedef-ed ?  Why do you have array of function pointers? (functions are pointers), you would have no & in the array and calling function from array would look like write_mem(xx) instead of (*write_mem)(xxx).

3. Functions PIC18_Write and PIC18_Read are too generic. I think we should go with my proposal 1. Have this generic function in pic.c and call different PICxx_writeEeprom from the selected protocol.

4. There is something wrong with the for-cycle in Write_fuse. 3/4 of the code are the same. :-)

These are of course non-functional bugs, just to keep the program in maintainable state.
Title: Re: PIC programming adapter problem
Post by: udif on December 07, 2011, 12:41:38 pm
[quote author="robots"]I have quickly looked at the patch, and there are quite few problems:

1. Static functions are static for a reason, and not to be exposed in *.h files.
[/quote]
You're right ofcourse. I moved those declarations to the beginning of pic18.c but that was on my laptop, and I forgot to sync it to my main machine.

[quote author="robots"]
2. Write_mem_table and read_mem_table use the same type, Why not have them typedef-ed ?  Why do you have array of function pointers? (functions are pointers), you would have no & in the array and calling function from array would look like write_mem(xx) instead of (*write_mem)(xxx).
[/quote]
typedefs are a good idea, I'll fix that later today.
As for arrays of functions - As far as I know, you can't have an array of functions in C, just array of function pointers.
http://c-faq.com/~scs/cclass/krnotes/sx8f.html (http://c-faq.com/~scs/cclass/krnotes/sx8f.html)
Quote
(The only possibilities that C doesn't support are functions returning arrays, and arrays of functions, and functions returning functions.)
I even tried that with GCC. The following code works:
Code: [Select]
#include "stdio.h"

void a(void) { printf("an");};
void b(void) { printf("bn");};
void c(void) { printf("cn");};
void d(void) { printf("dn");};

void (*arr[])(void) = {&a, &b, &c, &d};

int main(int argc, char *argv[])
{
int i;

  for (i = 0; i < sizeof(arr)/sizeof(arr[0]); i++) {
    (*arr[i])();
  }
  return 0;
}
The code below, however, doesn't work, producing the error:
Quote
b.c:8: error: declaration of 'arr' as array of functions
Code: [Select]
#include "stdio.h"

void a(void) { printf("an");};
void b(void) { printf("bn");};
void c(void) { printf("cn");};
void d(void) { printf("dn");};

void (arr[])(void) = {a, b, c, d};

int main(int argc, char *argv[])
{
int i;

  for (i = 0; i < sizeof(arr)/sizeof(arr[0]); i++) {
    (arr[i])();
  }
  return 0;
}
[quote author="robots"]3. Functions PIC18_Write and PIC18_Read are too generic. I think we should go with my proposal 1. Have this generic function in pic.c and call different PICxx_writeEeprom from the selected protocol.
[/quote]
I think this is a matter of personal style. The function in pic.c is iterating over the linked list of memory elements, calling the family-specific write function, which in turn decodes the memory address to decide what type of memory it is, and call the corresponding write function within that family. This was you isolate the fact that PIC18 has 3 different types of memory writes from the main program.
[quote author="robots"]
4. There is something wrong with the for-cycle in Write_fuse. 3/4 of the code are the same. :-)
[/quote]I assume you are bothered by the main loop in PIC18_Write_Fuse. Do you like this loop better?
Code: [Select]
	for(ctr = 0; ctr < length; ctr ++) {
if ((ctr & 1) == 0)
DataByte = (((uint8_t *)Data)[ctr + 1] << 8) + ((uint8_t *)Data)[ctr];
iface->PIC416Write(0x00, 0x0E00 | ctr);
iface->PIC416Write(0x00, 0x6EF6);
iface->PIC416Write(0x0F, DataByte);
iface->PIC416Write(0x40, 0x0000); // see PIC18_Write_Flash below
iface->flush();
}
[quote author="robots"]
These are of course non-functional bugs, just to keep the program in maintainable state.[/quote]
Generally speaking, when I contribute code to an existing project, I adapt to that project's coding style. Some of the choices I made were clearly no my personal coding style (The 3 line DataByte calculation, uppercase in variables, etc.).
Personally, for example, I would have avoided gcc extensions such as this:
Quote
   char tmp[21] = { [0 ... 20] = 0x00 };
Title: Re: PIC programming adapter problem
Post by: robots on December 07, 2011, 02:13:22 pm
2.
I was referring to something like this ....
Code: [Select]
#include "stdio.h"

typedef void (*fnc)(void);

void a(void) { printf("an");};
void b(void) { printf("bn");};
void c(void) { printf("cn");};
void d(void) { printf("dn");};

fnc func[] = {a,b,c,d};

int main(int argc, char *argv[])
{
int i;

  for (i = 0; i < sizeof(func)/sizeof(func[0]); i++) {
func[i]();
}
return 0;
}

3.
You are probably right about this.

4.
Designated inits are great addition to GCC, and I hope some day VC compiler will have them as well. These are used in the whole program (see pic_chip structure, etc). More here: http://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html (http://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html)

Adaptation is good and required. But copy/pasting (even existing) code, without thinking about is not. Uppercase variables are not my choice either and were added when I didn't look:-). Feel free to change them to lower case. I try to follow the "kernel coding style", but I fail at the function naming. Lot of my functions have mixed-case names.

I like the new cycle better, it doesn't duplicate the internal code. (but i doubt it correctness, i need to lookup the manual :-) )

Same could probably go for "setup" function that prepares Flash write. FUSE write setup (almost identical thing) is not in a separate function. How about merging these two setups into the same function?
Title: Re: PIC programming adapter problem
Post by: tayken on December 07, 2011, 02:21:26 pm
I guess we are getting a little sidetracked here...

Regarding the actual topic: I downloaded the whole svn for BP, the program under scriptsHVPselftest does not even compile as some header files are missing. That led me to search the actual directory, which is scriptspowertoolsHVPselftest. I've tested my adapter a couple of times, I guess we just need to synch those who folders or get rid of one of them. Can you also try the one I pointed udif?
Title: Re: PIC programming adapter problem
Post by: ian on December 07, 2011, 07:54:01 pm
I have nothing to add here except to say ready these three pages was glorious geek pr0n, and also I think the last of 5 pages of unread form posts I had to get through today ;)
Title: Re: PIC programming adapter problem
Post by: udif on December 08, 2011, 08:55:19 am
A quick update.
I've streamlined the code a bit, according to the suggestions here.
The code still behaves the same though, and I hope to take a look using a Logic Analyzer later today.
Title: Re: PIC programming adapter problem
Post by: robots on December 08, 2011, 12:59:47 pm
Looks much better :-) Should i wait till the fuse writing is working?

http://ww1.microchip.com/downloads/en/D ... 39622L.pdf (http://ww1.microchip.com/downloads/en/DeviceDoc/39622L.pdf)

Programming specification states that you should set all the TBLPRTx registers.

So the fuse writing code would be like:
Code: [Select]
setup fuse write:
iface->PIC416Write(0x00, 0x8EA6); //setup PIC - BSF  EECON1, EEPGD
iface->PIC416Write(0x00, 0x8CA6); //setup PIC - BSF  EECON1, CFGS
// set table ptrs
iface->PIC416Write(0x00, 0x0e30); //movlw 0x30
iface->PIC416Write(0x00, 0x6ef8); //movwf TBLPTRU
iface->PIC416Write(0x00, 0x0e00); //movlw 0x00
iface->PIC416Write(0x00, 0x6ef7); //movwf TBLPTRH

cycle through fuses:
the same thing you have.
If this won't help, only analyzer will.

Page 10 suggest that you have only 8 (16bit config words), thats 16writes.
Title: Re: PIC programming adapter problem
Post by: udif on December 08, 2011, 03:26:32 pm
[quote author="robots"]Looks much better :-) Should i wait till the fuse writing is working?
[/quote] Probably Yes, it will only be 1-2 more days, and currently the code adds no functionality beyond my previous patch, that already added code read/write ability for the 18F2553.

[quote author="robots"]http://ww1.microchip.com/downloads/en/DeviceDoc/39622L.pdf

Programming specification states that you should set all the TBLPRTx registers.
[/quote]
The link to the document is naturally the same one I'm looking at.
I don't think the document says I need to set all 3 TBLPRTx registers between writes.
I think the note on section 3.6 regarding setting the full address only relates to not using the 0xD command (write 2 bytes and post-increment).
I set all 3 TBLPRTx registers at the beginning, then for the next configuration writes, set only the new low address. I think this is consistent with their example as well. Even/Odd writes are separate, and they change only the lowest address register between writes.
In any case, if I will test this as well, just to be sure (this should be quick).
[quote author="robots"]So the fuse writing code would be like:
Code: [Select]
setup fuse write:
iface->PIC416Write(0x00, 0x8EA6); //setup PIC - BSF  EECON1, EEPGD
iface->PIC416Write(0x00, 0x8CA6); //setup PIC - BSF  EECON1, CFGS
// set table ptrs
iface->PIC416Write(0x00, 0x0e30); //movlw 0x30
iface->PIC416Write(0x00, 0x6ef8); //movwf TBLPTRU
iface->PIC416Write(0x00, 0x0e00); //movlw 0x00
iface->PIC416Write(0x00, 0x6ef7); //movwf TBLPTRH

cycle through fuses:
the same thing you have.
If this won't help, only analyzer will.

Page 10 suggest that you have only 8 (16bit config words), thats 16writes.[/quote]

I'm more worried about the write delay as set by the Bus Pirate. It is defined to be 1ms, which is what the spec says, with no safety margins.I'll take a look with the OLS to see what the actual delay is.
Title: Re: PIC programming adapter problem
Post by: udif on December 09, 2011, 07:43:58 am
I think you can commit the changes.

After all my efforts have made no change on the result (set full tblptr on each write, increase write delay to 2ms, etc.) I decided to take a close look at the actual data.
Here is the relevant data from the diolan bootrom:
Code: [Select]
:020000040030CA
:0100000021DE
:010001004EB0
:0100020038C5
:010003001EDE
:010005008377
:010006008178
:010008000FE8
:01000900C036
:01000A000FE6
:01000B008074
:01000C000FE4
:01000D0040B2
:00000001FF
Here is the same data, read back from the 18F2553:
Code: [Select]
:020000040030CA
:0E000000214E011E0083C4000FC00F800F4070
:00000001FF
At first, I noticed that the fist two bytes were and the last 6 bytes Identical, but not the middle bytes. I couldn't explain the other difference.

Then last night I noticed for the first time that the original Hex file skipped some addresses, namely 0x300004 and 0x300007.
Earlier, before I noticed this, I thought some of the data was written to the wrong locations (specifically, 0x83 at 0x300005), when in fact it was OK . Those undefined addresses are indeed read as 0x00, as defined by the spec.

This leaves me with 0x300006 (CONFIG4L) that is read as 0xc4 when written as 0x81, and 0x300002 (CONFIG2L) that is read as 0x01 when written as 0x38). I now believe this is something specific to these registers and my specific PIC type, the 18F2553.

Maybe someone who has more PIC experience than me can explain why?
Title: Re: PIC programming adapter problem
Post by: tayken on December 09, 2011, 09:28:32 am
[quote author="udif"]This leaves me with 0x300006 (CONFIG4L) that is read as 0xc4 when written as 0x81, and 0x300002 (CONFIG2L) that is read as 0x01 when written as 0x38). I now believe this is something specific to these registers and my specific PIC type, the 18F2553.

Maybe someone who has more PIC experience than me can explain why?[/quote]
There are some unused bits in all config registers, thought it may be related to that but the thing is there are some weird stuff going on. For example for CONFIG4L, written value is OK, but you are reading bit4 as 1 whereas unimplemented bits are read as 0. See 18F2550 datasheet (18F2553 datasheet points me there) Table 25-1
Title: Re: PIC programming adapter problem
Post by: robots on December 09, 2011, 11:16:35 am
I have committed the code into SVN, also added few checks, fixed compiler warning, removed some unused code, renamed DataByte and Data , to databyte and data.