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

Use -xmode to switch PICkit4 and SNAP application modes #1596

Merged
merged 27 commits into from
Jan 3, 2024

Conversation

MCUdude
Copy link
Collaborator

@MCUdude MCUdude commented Dec 18, 2023

Resolves issue #1027

If you have a PICkit4 or an MPLAB SNAP that happens to be in PIC mode, it has to be switched to AVR mode before Avrdude can communicate with it. However, with this PR, Avrdude can now do the switching. Use -xmode=avr to perform the switch. This is important because it has been reported over at the Avrfreaks forums that the latest MPLAB X doesn't do the switch when programming AVRs. It stays in "PIC mode" and speaks a protocol similar (or identical) to the PICkit5, which Avrdude currently doesn't support.

TODO:

$ ./avrdude -csnap_updi -patmega4808

avrdude error: MPLAB SNAP in PIC mode detected
        use -xmode=avr to enter AVR mode

avrdude error: unable to open port usb for programmer snap_updi

avrdude done.  Thank you.

$ ./avrdude -csnap_updi -patmega4808 -xmode=avr

avrdude error: MPLAB SNAP in PIC mode detected
        switching to AVR mode
        please re-run Avrdude

avrdude error: unable to open port usb for programmer snap_updi

avrdude done.  Thank you.

$ ./avrdude -csnap_updi -patmega4808 -xmode=avr
avrdude warning: programmer is already in AVR mode. Ignoring -xmode
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9650 (probably m4808)

avrdude done.  Thank you.

@MCUdude MCUdude added the enhancement New feature or request label Dec 18, 2023
@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 18, 2023

It looks like it isn't going to be all that easy to get mode switching to work for pickit4_isp and snap_isp.
This is because the variable that "tells" avrdude to switch mode is local to stk500v2.c, and can't easily be copied into jtag3_open_common, which lives in jtag3.c. @stefanrueger If you have an idea of how this can be done in a way that isn't terribly ugly, I'm all ears!

@mcuee
Copy link
Collaborator

mcuee commented Dec 19, 2023

@MCUdude

One way is to ship this as a separate utility along with avrdude without integrating into avrdude, if it is a bit difficult to integrate into existing avrdude source codes.

Let's also hear from @stefanrueger and @dl8dtl.

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 19, 2023

@mcuee try this project:
Set_PIC_mode.X.zip

When the project is loaded you can click the green arrows to switch mode:

Skjermbilde 2023-12-19 kl  17 16 44

Thanks Wireshark
@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 20, 2023

@mcuee I managed to implement -xmode=pic. It did use a different "data packet" than what the command suggested by @xedbg earlier, but I managed to track down the correct package using Wireshark USB capture 🤓

Skjermbilde 2023-12-20 kl  22 00 09

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 20, 2023

@stefanrueger I'd like to hear what you would consider the neatest way to trigger the PIC/AVR mode switching from stk500v2.c. Now that the base functionality of this PR is up and running, this would be the next step.

It looks like it isn't going to be all that easy to get mode switching to work for pickit4_isp and snap_isp.
This is because the variable that "tells" avrdude to switch mode is local to stk500v2.c, and can't easily be copied into jtag3_open_common, which lives in jtag3.c. @stefanrueger If you have an idea of how this can be done in a way that isn't terribly ugly, I'm all ears!

@stefanrueger
Copy link
Collaborator

It isn't clear to me why a user option is needed.

If the user requests an operation in these programmers for which they need to be in AVR mode, would it not make sense to query which mode they are in, memorise the mode in an appropriate PDATA(pgm)->... component, switch to (presumably) AVR mode, do the operation(s) and then in the end switch to the memorised mode the programmer was in, if needed.

I am sure I have overlooked (or don't know) a crucial problem on the way.

@mcuee
Copy link
Collaborator

mcuee commented Dec 21, 2023

@MCUdude

I will try this PR later this evening. Now I know how to switch to PIC mode using MPLAB X IDE 6.15. And your libusb example code works fine to switch from PIC mode to AVR mode.

@mcuee
Copy link
Collaborator

mcuee commented Dec 21, 2023

@MCUdude

You mentioned that PICKit 4 and SNAP can not program the AVR EB chips in AVR mode and MPLAB X IDE uses PIC mode for that purpose. Is this confirmed?

If that is the case, we may have to block them and print a warning message for now.

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 21, 2023

You mentioned that PICKit 4 and SNAP can not program the AVR EB chips in AVR mode and MPLAB X IDE uses PIC mode for that purpose. Is this confirmed?

@mcuee read this post. It appears that PK4/SNAP can program AVR-EBs, but only in PIC mode using MPLAB X

https://www.avrfreaks.net/s/topic/a5C3l000000UkyzEAC/t193040

If that is the case, we may have to block them and print a warning message for now.

Ideally, yes. But it may just be a firmware update away from working in AVR mode as well.

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 21, 2023

It isn't clear to me why a user option is needed.

If the user requests an operation in these programmers for which they need to be in AVR mode, would it not make sense to query which mode they are in, memorise the mode in an appropriate PDATA(pgm)->... component, switch to (presumably) AVR mode, do the operation(s) and then in the end switch to the memorised mode the programmer was in, if needed.
I am sure I have overlooked (or don't know) a crucial problem on the way.

@stefanrueger There are good reasons why one wants to switch modes explicitly. First, this is a rather niche issue that only deals with two official programmers, and one of them has been discontinued.

After a switch, the programmer restarts and reenumerates. Avrdude would have to start over again after the switch, and this is something I don't want to deal with for now; hence returning -1 after the switch.

And if the programmer is in PIC mode, there are good reasons why you want to keep it in AVR mode. It's quicker, since the programmer doesn't have to switch modes all the time, and you can use it as a USB to serial adapter because it shows up as a serial port. But if you borrowed someone's programmer that originally was in PIC mode, you would probably like to switch it back before returning it. And having to install MPLAB plus a PIC compiler and create an empty project just to switch back is just dumb. So it's IMO much better to have -xmode where the user can do whatever he or she wants.

And I forgot to mention. MPLAB really struggles with switching modes sometimes. Sometimes it works, while others you have to restart MPLAB to get it to do the switch. Just having a utility that can do this that's not MPLAB is a good thing.

@stefanrueger
Copy link
Collaborator

After a switch, the programmer restarts and reenumerates. Avrdude would have to start over again after the switch

Could that be packed into the programmer's open() function? Don't we now have code that can figure out when a device re-enumerates? And the switching back to PIC could be put into the close function(), not?

Having an option makes it necessary to document it; users won't know the option exists and wouldn't know the procedures, so AVRDUDE would need to tell them that the programmer is in pic mode and they need to call avrdude again with some option and then again, to do the work and then again to put it into PIC mode. And somehow it would be much slicker if AVRDUDE did all that on its own accord...

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 21, 2023

Could that be packed into the programmer's open() function?

Yes, feel free! I'm leaving for Christmas holiday tomorrow and will be away for a little more than a week. I'm planning to bring my computer + some hardware, but I have limited time to spend on this PR. However, I can see if I can spend some time in the evenings to do a little testing with the SNAP and PICkit4.

Automatic switching would be nice, but I think it's important to also be able to explicitly set a mode, so the user can utilize features like the USB-UART interface the programmer provides in AVR mode.

@mcuee
Copy link
Collaborator

mcuee commented Dec 21, 2023

And I forgot to mention. MPLAB really struggles with switching modes sometimes. Sometimes it works, while others you have to restart MPLAB to get it to do the switch. Just having a utility that can do this that's not MPLAB is a good thing.

I will agree with this one. MPLAB X IDE is really troublesome to use for switching the mode.

@mcuee
Copy link
Collaborator

mcuee commented Dec 21, 2023

@MCUdude

Now this PR is actually working very well (switching from AVR mode to PIC mode and then from PIC mode to AVR mode). Tested under Windows 11 with PICKit 4 in JTAG mode.

Well done!

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a -U flash:v:m32a_blink_read.hex:i
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596: processing -U flash:v:m32a_blink_read.hex:i
avrdude_pr1596: verifying flash memory against m32a_blink_read.hex
Reading | ################################################## | 100% 0.12 s
avrdude_pr1596: 526 bytes of flash verified

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a -xmode=pic
               switching to PIC mode
               PIC mode switch successful
avrdude_pr1596 error: unable to open port usb for programmer pickit4

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a

avrdude_pr1596 error: PICkit4 in PIC mode detected.
               use -xmode=avr to enter AVR mode

avrdude_pr1596 error: unable to open port usb for programmer pickit4

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a -xmode=avr

avrdude_pr1596 error: PICkit4 in PIC mode detected.
               switching to AVR mode
               please re-run Avrdude

avrdude_pr1596 error: unable to open port usb for programmer pickit4

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c pickit4 -p m32a -U flash:v:m32a_blink_read.hex:i
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596: processing -U flash:v:m32a_blink_read.hex:i
avrdude_pr1596: verifying flash memory against m32a_blink_read.hex
Reading | ################################################## | 100% 0.11 s
avrdude_pr1596: 526 bytes of flash verified

avrdude_pr1596 done.  Thank you.

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 21, 2023

Another thing I should look into is to see how the -xmode performs when the SNAP or PICkit4 is in bootloader mode. When in bootloader mode, they have a different PID, so it's easy to detect. However, I doubt -xmode will work, because I'm pretty sure they would require a different command. But this can be fixed in a separate PR. If the programmer is in bootloader mode, it means that it's either in FW upgrade mode or something is wrong with it.

Well done!

Thanks!

@mcuee
Copy link
Collaborator

mcuee commented Dec 21, 2023

Mode switching (PIC <->AVR) is working well for Microchip SNAP as well.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c snap -p m32a
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c snap -p m32a -xmode=pic
               switching to PIC mode
               PIC mode switch successful
avrdude_pr1596 error: unable to open port usb for programmer snap

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c snap -p m32a

avrdude_pr1596 error: MPLAB SNAP in PIC mode detected
               use -xmode=avr to enter AVR mode

avrdude_pr1596 error: unable to open port usb for programmer snap

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c snap -p m32a -xmode=avr

avrdude_pr1596 error: MPLAB SNAP in PIC mode detected
               switching to AVR mode
               please re-run Avrdude

avrdude_pr1596 error: unable to open port usb for programmer snap

avrdude_pr1596 done.  Thank you.

PS> .\avrdude_pr1596 -C .\avrdude_pr1596.conf -c snap -p m32a
avrdude_pr1596: AVR device initialized and ready to accept instructions
avrdude_pr1596: device signature = 0x1e9502 (probably m32a)

avrdude_pr1596 done.  Thank you.

@stefanrueger
Copy link
Collaborator

OK, looks like a good enhancement. Thanks for tackling this problem, @MCUdude!

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 25, 2023

Time is still very limited, but I think this PR is soon ready for review.
Things I'd like to get/see fixed:

  • Modes cannot be switched using -cpickit4_isp and -csnap_isp. The problem is that you can't just pass private PDATA(pgm) data from stk500v2.c/h to jtag3.c/h, which is necessary to toggle a mode switch. @stefanrueger I don't want to make a full copy of jtag3_open_common in stk500v2.c with tiny modifications, but I don't know how it could be done differently without adding additional parameters to the function. And that wouldn't make it "common" anymore, wouldn't it?
  • After a switch attempt, Avrdude always prints avrdude error: unable to open port usb for programmer snap(/pickit4). It would be great if it could skip this like and just print the Avrdude done, thank you message instead.

still need to figure out how to actually trigger the mode switch form stk500v2.c
@MCUdude MCUdude changed the title Use libusb to switch a PICkit4 or SNAP to AVR mode Use -xmode to switch PICkit4 and SNAP application modes Dec 27, 2023
src/avrdude.1 Outdated Show resolved Hide resolved
src/doc/avrdude.texi Outdated Show resolved Hide resolved
src/jtag3.c Outdated Show resolved Hide resolved
src/jtag3.c Outdated Show resolved Hide resolved
src/jtag3.c Outdated Show resolved Hide resolved
src/jtag3.c Outdated Show resolved Hide resolved
src/jtag3.h Outdated Show resolved Hide resolved
src/stk500v2.c Outdated Show resolved Hide resolved
src/stk500v2.c Outdated Show resolved Hide resolved
src/stk500v2.c Outdated Show resolved Hide resolved
@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 28, 2023

Thanks for another review!

src/jtag3.c Outdated Show resolved Hide resolved
src/jtag3.c Outdated Show resolved Hide resolved
@stefanrueger stefanrueger merged commit a245cd5 into avrdudes:main Jan 3, 2024
10 checks passed
@MCUdude MCUdude linked an issue Jan 3, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch a PICkit4 or MPLAB SNAP programmer to AVR mode
3 participants