avrdude
avrdude copied to clipboard
avrdudes
[patch #7703] AT89S5x support, stk500 spi frequency bugfix, other minor fixes
Mon 16 Jan 2012 05:25:31 PM UTC
I apologize for the mess, but this is the result of attempting to add a more thorough AT89S5x support (including the 'S8253/'Sx051 subgroup (for which the "sck_polarity" config keyword was added) and accordingly modified STK500 firmware, to be published later); and while working on it, I could not resist to fix other bugs I came across...
This is a work in progress, so the '51-specific stuff is not polished up to expectations (including no documentation YET); I am submitting it as it is now as I have no idea when will I have the opportunity to get back to it. Incorporating them should make no harm to the "mainstream" though, IMO. Please note that this patch incorporates https://savannah.nongnu.org/patch/?7538 although with a different, more generic keyword ("reset_polarity" rather than "is_at89s", as the newer AT89LP family for which I plan to add support later too, has members with "normal" reset polarity).
On the other hand, items 1, 3 and 4 may be of general interest.
Adding to the mess, I tend to set my editor to strip extra spaces from line ends and convert tabs to spaces; sorry.
List of changes:
- sck frequency setting/printout in terminal mode for stk500V2
- fixed sck frequency reading
- fixed correct calculation of the frequency (based on AVR068 rather than AVR061 which would be correct for stk500v1)
- both sck period and frequency printed out
- correct xtal frequency for AVRISP used
- sck frequency can be entered as frequency in Hz/kHz/MHz now
- 'S5x programming support
- created a configuration file containing 'S52
- reset_polarity and sck_polarity (normal/invert) added to part configuration
- ** TODO ** add documentation
- reset pin for bitbanged-style programmers is inverted based on reset_polarity
- programming enable in bitbang.c now correctly checks returned value according to pollindex/pollvalue
- stk500v2 now issues the correct value for PARAM_RESET_POLARITY
- for paged memories, _byteblock and block (or any other name) mode added to stk500v2.c
- default retry_pulse changed to RESET, as SCK is used for retry_pulse only in the historical AT90S parts
- ** todo ** set explicit retry_pulse in conf file for those parts where this applies
- attempt to "spi" in terminal mode for stk500 crashed avrdude
- fixed (hopefully)
Jan Waclawek
This issue was migrated from https://savannah.nongnu.org/patch/?7703
Joerg Wunsch <joerg_wunsch> Mon 16 Jan 2012 05:43:18 PM UTC
I'm happy with about everything you are telling, except one:
I tend to set my editor to strip extra spaces from line ends and convert tabs to spaces; sorry.
There's one general rule:
Don't mess with other people's style. Style changes easily obfuscate actual code changes in the version history, and thus make it hard to track down bugs later on.
If you really have to mess with it, strictly separate the style-only changes from actual code changes, so both can be separated in two commits.
No, your editor's preferences don't serve as an excuse here. My editor's preferences frequently don't match the files' style here either (and AVRDUDE's files, unfortunately, don't follow a consistent style at all), so I have to adjust my editor settings anew for each file, so resulting changes will follow the existing style in the file I'm working on as best as possible.
Boris
Are there any chances to get stk500v2_set_sck_period() function fixed? This patch seems to contain correct implementation of it. As of now, stk500v2_set_sck_period() produces values which are absolutely inconsistent with either, protocol specification or stk500.exe program.
Joerg Wunsch <joerg_wunsch> Fri 13 Sep 2013 08:22:13 PM UTC
I tried to read through all the patch. Sorry, I give up. With all the messed up whitespace, it's really hard to see what's going on.
I'd love to incorporate these changes, but please:
. Submit one patch per problem/enhancement. It's virtually impossible for me to just cherry-pick the SCK period fix out of it now.
. As already said, keep everything else as it is, regardless of whether you love it the way it is or not. Mind you, I don't love all this myself, but I've learned my lesson years ago, and am simply not tempted to just do a sweeping whitespace fix over the entire repository. When trying to find a bug later on, the whitespace mess masks the actual changes so much you don't see the bug anymore.
. I love the idea of being able to specify the SPI frequency in kHz/MHz, but would love to extend this from terminal mode to the commandline's -B option as well. (Already thought about this before, but never got around to do it.)
. Please limit the line length to a more reasonable value. 80 columns as long as it makes sense, and not more than about 100 columns.
. Block comments are better written as /*
- Block of comments- goes here.*/ rather than using //.
Nevertheless, all this is really welcome, Jan!
Joerg Wunsch <joerg_wunsch> Fri 13 Sep 2013 09:35:02 PM UTC
I made my own attempt on fixing the STK500v2 SCK period bug, based on AVR068. Anyone interested, pleaes have a look.
Joerg Wunsch <joerg_wunsch> Fri 13 Sep 2013 09:35:02 PM UTC
I made my own attempt on fixing the STK500v2 SCK period bug, based on AVR068. Anyone interested, pleaes have a look.
In that case, I guess this issue is no longer applicable, right?
