patchAVR Downloader/UploaDEr - Patches: patch #7703, AT89S5x support, stk500 spi...


You are not allowed to post comments on this tracker with your current authentication level.

patch #7703: AT89S5x support, stk500 spi frequency bugfix, other minor fixes

Submitter:  Jan Waclawek <wek>
Submitted:  Mon 16 Jan 2012 05:25:31 PM UTC
Votes: 20
Category:  None Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open

Fri 13 Sep 2013 09:35:02 PM UTC, comment #4: 

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>
Group administrator
Fri 13 Sep 2013 08:22:13 PM UTC, comment #3: 

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>
Group administrator
Fri 10 Aug 2012 06:30:46 AM UTC, comment #2: 

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.

Boris <bxm>
Mon 16 Jan 2012 05:43:18 PM UTC, comment #1: 

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.

Joerg Wunsch <joerg_wunsch>
Group administrator
Mon 16 Jan 2012 05:25:31 PM UTC, original submission:  

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 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:

1. 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

2. '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
3. 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

4. attempt to "spi" in terminal mode for stk500 crashed avrdude
   - fixed (hopefully)

Jan Waclawek

Jan Waclawek <wek>


(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #24838: added by wek (20KiB - application/zip)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by bxm (Voted in favor of this item)
  • -email is unavailable- added by bxm (Posted a comment)
  • -email is unavailable- added by joerg_wunsch (Posted a comment)
  • -email is unavailable- added by wek (Submitted the item)

    There are 20 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.


    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-08-10 bxm Carbon-Copy- Added bxm
    2012-01-16 wek Attached File- Added, #24838

    Back to the top

    Powered by Savane 3.13-b921.
    Corresponding source code