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

Terminal mode improvements #1587

Merged
merged 20 commits into from
Dec 18, 2023
Merged

Terminal mode improvements #1587

merged 20 commits into from
Dec 18, 2023

Conversation

MCUdude
Copy link
Collaborator

@MCUdude MCUdude commented Dec 2, 2023

Extends #1574

  • Fix bug that makes Avrdude segfault when trying to set a new clock speed using the sck command using a JTAG3 compatible programmer (bug was introduced when we added SIB read support
  • Make it possible to use the sck command to set the clock speed in Hz, kHz, and MHz.
  • running sck without a second argument now prints the clock speed in kHz as well as us.
  • jtag3_get_vtarget and jtag3_get_sck_period added to jtag3.c

TODO:
Test all JTAG2, stk500v2, and stk600 compatible programmers I have to make sure all of them can read the target voltage in terminal mode using vtarg, and clock speed using sck. When this is done (and missing programmers added), I'll mark this PR as "ready to review".

Resolves issue #1329

@MCUdude MCUdude linked an issue Dec 2, 2023 that may be closed by this pull request
@mcuee mcuee added the enhancement New feature or request label Dec 2, 2023
src/jtag3.c Outdated Show resolved Hide resolved
src/term.c Outdated Show resolved Hide resolved
@mcuee
Copy link
Collaborator

mcuee commented Dec 5, 2023

It seems to work well for STK500v2, tested under Linux.

mcuee@UbuntuSwift3 ~/build/avr/avrdude_bin $ ./avrdude_pr1587 -C ./avrdude_pr1587.conf -c stk500v2 -P ch340 -p m32a -qqt
avrdude> vtarg
Vtarget = 5.1 V
avrdude> sck
SCK period = 8.7 us
SCK freq   = 115 kHz
avrdude> sck 230 kHz
Syntax: sck <value>
Function: set the SCK period in us or frequency in [kM]Hz
avrdude> sck 8.7
avrdude> sck
SCK period = 17.4 us
SCK freq   = 57 kHz
avrdude> sck 3
avrdude> sck
SCK period = 8.7 us
SCK freq   = 115 kHz
avrdude> sck 20
avrdude> sck
SCK period = 22.2 us
SCK freq   = 44 kHz
avrdude> sck 33
avrdude> sck
SCK period = 35.3 us
SCK freq   = 28 kHz
avrdude> quit

@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 6, 2023

Sorry for the slow pace, but this is a quite busy week for me with very little spare time. I'll pick this up again as soon as I have more time.

@MCUdude MCUdude marked this pull request as ready for review December 8, 2023 21:58
@MCUdude
Copy link
Collaborator Author

MCUdude commented Dec 9, 2023

I think this PR is ready for review. Tested with a Curiosity Nano (UPDI), an AVR Dragon (ISP, JTAG), and STK600 (ISP, HVPP).

src/main.c Outdated Show resolved Hide resolved
src/stk500v2.c Show resolved Hide resolved
src/term.c Outdated
if (endp == argv[1]) {
pmsg_error("(sck) cannot parse period %s\n", argv[1]);
if (*endp != 0) {
int suffixlen = strlen(endp);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe parse as above? Consider introducing a function so the same code doesn't need to be there twice

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced the entire implementation with the one you suggested. Would be great if you could have a look at it again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it still work as intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about this. Will check this evening.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appears to work perfectly fine!

$ ./avrdude -cstk600 -patmega2560 -t
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9801 (probably m2560)

avrdude: processing -t interactive terminal
avrdude> sck
SCK period = 3.4 us
SCK freq   = 296 kHz
avrdude> sck 100
avrdude> sck
SCK period = 100.0 us
SCK freq   = 10 kHz
avrdude> sck 120k
avrdude> sck
SCK period = 8.4 us
SCK freq   = 119 kHz
avrdude> sck 1MHz
avrdude> sck
SCK period = 1.0 us
SCK freq   = 1000 kHz
avrdude> sck 8us
avrdude> sck
SCK period = 8.0 us
SCK freq   = 125 kHz
avrdude> sck 6u
avrdude error: (sck) invalid bit clock unit u
avrdude> sck
SCK period = 8.0 us
SCK freq   = 125 kHz
avrdude> sck 32
avrdude> sck
SCK period = 32.0 us
SCK freq   = 31 kHz

src/jtagmkII.c Outdated Show resolved Hide resolved
src/jtagmkII.c Outdated Show resolved Hide resolved
@mcuee
Copy link
Collaborator

mcuee commented Dec 15, 2023

I should be able to give this PR some simple tests over the weekend when I have access to my HW.

@mcuee
Copy link
Collaborator

mcuee commented Dec 17, 2023

Looks good to me.

PS>.\avrdude_pr1587v1 -C .\avrdude_pr1587v1.conf -c avrispmkii -p m328pb -qqt
avrdude> sck
SCK period = 8.0 us
SCK freq   = 125 kHz
avrdude> sck 100
avrdude> sck
SCK period = 100.4 us
SCK freq   = 9 kHz
avrdude> sck 120k
avrdude> sck
SCK period = 10.4 us
SCK freq   = 96 kHz
avrdude> sck 1MHz
avrdude> sck
SCK period = 1.0 us
SCK freq   = 1000 kHz
avrdude> sck 8us
avrdude> sck
SCK period = 8.0 us
SCK freq   = 125 kHz
avrdude> sck 6u
avrdude_pr1587v1 error: (sck) invalid bit clock unit u
avrdude> sck
SCK period = 8.0 us
SCK freq   = 125 kHz
avrdude> sck 32
avrdude> sck
SCK period = 32.1 us
SCK freq   = 31 kHz
avrdude> quit

@stefanrueger stefanrueger merged commit 2bd6b56 into avrdudes:main Dec 18, 2023
10 checks passed
This pull request was closed.
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.

Query a programmer to figure out its current clock speed (low priority)
3 participants