patchAVR Downloader/UploaDEr - Patches: patch #8380, adds 500k 1M 2M baud to ser_posix.c

 
 

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

patch #8380: adds 500k 1M 2M baud to ser_posix.c

Submitter:  Georg von Zengen <oni>
Submitted:  Thu 06 Mar 2014 10:34:36 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Wont Do Privacy:  Public
Assigned to:  joerg_wunsch Open/Closed:  Open

Jump to the original submission

Tue 09 Feb 2016 03:23:38 PM UTC, comment #15: 

Reopening this patch, as the fix has been causing too much
collateral damange, and thus been backed out again.

See bug #46610.

Patch needs a major overhaul before it can be accepted again.

Joerg Wunsch <joerg_wunsch>
Group administrator
Tue 25 Nov 2014 09:33:38 PM UTC, comment #14: 

Thanks, applied in SVN r1351.

Joerg Wunsch <joerg_wunsch>
Group administrator
Wed 12 Mar 2014 02:44:04 PM UTC, comment #13: 

Ok just to clarify it.
Are you ok with the last patch or should I change anything else?

Georg von Zengen <oni>
Wed 12 Mar 2014 01:44:01 PM UTC, comment #12: 

Thanks for verifying _linux_ vs. linux.

No, I didn't want to imply we were already supporting strict
standards compliance mode, but for new code, it's always a
good idea to follow the standard as closely as possible.

(I think in your example, some option or define would be needed
that tells we are compiling a Posix-compliant source.  That
should resolved most if not all of the complaints.)

Joerg Wunsch <joerg_wunsch>
Group administrator
Wed 12 Mar 2014 01:37:46 PM UTC, comment #11: 

ok useing _linux_ is possible (new patch).
But putting -std=c99 is a bad idea for other reasons also (sea below).



make  all-recursive
make[1]: Entering directory `/mnt/ssd/projects/avrdude/avrdude'
Making all in .
make[2]: Entering directory `/mnt/ssd/projects/avrdude/avrdude'
gcc -DHAVE_CONFIG_H -I.  -DCONFIG_DIR=\"/usr/local/etc\"  -Wall -Wno-pointer-sign -std=c99 -g -O2 -MT avrdude-main.o -MD -MP -MF .deps/avrdude-main.Tpo -c -o avrdude-main.o `test -f 'main.c' || echo './'`main.c
In file included from avr.h:26:0,
                 from main.c:47:
avrpart.h:166:29: error: ‘PATH_MAX’ undeclared here (not in a function)
   char          config_file[PATH_MAX]; /* config file where defined */
                             ^
main.c: In function ‘main’:
main.c:456:3: warning: implicit declaration of function ‘getopt’ [-Wimplicit-function-declaration]
   while ((ch = getopt(argc,argv,"?b:B:c:C:DeE:Fi:l:np:OP:qstU:uvVx:yY:")) != -1) {
   ^
main.c:460:27: error: ‘optarg’ undeclared (first use in this function)
         baudrate = strtol(optarg, &e, 0);
                           ^
main.c:460:27: note: each undeclared identifier is reported only once for each function it appears in
main.c:660:36: error: ‘S_IFREG’ undeclared (first use in this function)
     if ((rc < 0) || ((sb.st_mode & S_IFREG) == 0)) {
                                    ^
main.c:891:7: warning: implicit declaration of function ‘strdup’ [-Wimplicit-function-declaration]
       if ((upd->memtype = strdup(mtype)) == NULL) {
       ^
main.c:891:25: warning: assignment makes pointer from integer without a cast [enabled by default]
       if ((upd->memtype = strdup(mtype)) == NULL) {
                         ^
main.c:1032:5: warning: implicit declaration of function ‘usleep’ [-Wimplicit-function-declaration]
     usleep(waittime);
     ^
main.c:1166:9: warning: implicit declaration of function ‘strcasecmp’ [-Wimplicit-function-declaration]
         if ((strcasecmp(m->desc, memname) == 0) && (upd->op == DEVICE_WRITE)) {
         ^
main.c:324:11: warning: unused variable ‘usr_config’ [-Wunused-variable]
   char    usr_config[PATH_MAX]; /* per-user config file */
           ^
main.c:323:11: warning: unused variable ‘sys_config’ [-Wunused-variable]
   char    sys_config[PATH_MAX]; /* system wide config file */
           ^
make[2]: * [avrdude-main.o] Error 1
make[2]: Leaving directory `/mnt/ssd/projects/avrdude/avrdude'
make[1]: * [all-recursive] Error 1
make[1]: Leaving directory `/mnt/ssd/projects/avrdude/avrdude'
make: * [all] Error 2


(file #30876)

Georg von Zengen <oni>
Wed 12 Mar 2014 12:58:54 PM UTC, comment #10: 

Thanks again, looks good!

Curious, would it also work with #ifdef _linux_?

I think the macro without a leading underscore won't be defined
if you put the compiler into strict standards compliance mode
(like, -std=c99) since this macro collides with the application
namespace.

Joerg Wunsch <joerg_wunsch>
Group administrator
Wed 12 Mar 2014 12:54:39 PM UTC, comment #9: 

And one more patch file.

- removed OSX stuff
- linux still works (tested)
- hopefully removed all unrelated changes




(file #30875)

Georg von Zengen <oni>
Wed 12 Mar 2014 11:23:57 AM UTC, comment #8: 

Thank you for the patch!

Some remarks:

. I don't think you have to special-case OSX in any way.  It does
  already work the way it is now, just the same like *BSD (which it is
  derived from in that respect); the Bxxxx names are just aliases for
  number xxxx there, so cfsetispeed() and cfsetospeed() can be fed the
  raw baudrate numbers as well as the Bxxxx defines.

. Your patch for Linux is probably fine, though I cannot test it
  (lacking any device with a non-standard baudrate).

. Please don't add unrelated changes to a patchfile (indentation
  changes in line 96 ff.).

Joerg Wunsch <joerg_wunsch>
Group administrator
Wed 12 Mar 2014 10:22:31 AM UTC, comment #7: 

The last days I put a bit more work into this and think I did some progress.
The custom_baud.patch adds not only the three rate mentioned in the topic. It enables avrdude to set "every" custom baud rate.
It is "every" because of kernel/hardware limitations.

The patch supports mac os x and linuxs.
For mac os x 921600 is the maximum baud rate, it seem to be a kernel/hardware limitation.
For linux I tested it up to 2M baud on different debian and arch systems and a fedora virtualbox.

For all not os x or linux systems the patch does not change anything, here the standard posix way is used.
This is mostly because of my missing knowledge of BSDs.

The Patch also fixes the issue of checking the return values of cfsetospeed() and  cfsetispeed() you mentioned before.

Custom baud rates are useful for avrdude because it makes programming this serial bootloaders faster even if the maximum baud rate of the avr is kind of odd.
 
So this patch makes baudrate.patch obsolete.

(file #30874)

Georg von Zengen <oni>
Fri 07 Mar 2014 07:18:04 AM UTC, comment #6: 

As an additional note, the calls to cfsetispeed() /
cfsetospeed() should really check their return values, so
attempts to set baud rates which are not supported by the
underlying system are caught.

Joerg Wunsch <joerg_wunsch>
Group administrator
Fri 07 Mar 2014 06:48:24 AM UTC, comment #5: 

I have cross-checked myself.

OSX uses the BSD model where Bxxxx == xxxx, so no special
treatment is necessary.  (They don't have Bxxxx constants for
the baudrates you mention but for other rates like 230400 etc.)
Their struct termios has baudrate entries separate from the
historic c_*flags fields.

Solaris uses the same model as Linux where the baudrate is
encoded in the lower bits of c_cflags, but it only has standard
baudrates up to B38400 anyway.

So it's only Linux that needs the patch at all.

Joerg Wunsch <joerg_wunsch>
Group administrator
Thu 06 Mar 2014 10:19:54 PM UTC, comment #4: 

I also had some more research about this.
Here my results:
-no linux version (had a look at debian derivatives, fedora, arch)     follows the idea of Bxxxx = xxxx
-freebsd, openbsd, and os x follow this idea until B38400, everything above is handled differently in each of the three as far as I saw.
-What did not found was a system where -b 1000000 is supposed to work correctly

During the test for my last comment the baud rate was accidentally  set to 1000000 by a previous test with my patched version of avrdude.

So I think for Linux we are fine with this patch.
For OS X I may have a look tomorrow.
And for XXXXbsd, I do not know anyone using it on the development machine so I have no chance to test it.

Georg von Zengen <oni>
Thu 06 Mar 2014 12:26:18 PM UTC, comment #3: 

After some more review, it appears that (at least some versions
of) Linux don't follow the idea of using Bxxxx == xxxx, so I'm
probably going to accept your patch.

Joerg Wunsch <joerg_wunsch>
Group administrator
Thu 06 Mar 2014 11:49:37 AM UTC, comment #2: 

Oops,

I did not know that it is already working. A colleague of mine told me that it is not working.
Now I tested it and it is working.
So the can be closed.
Thanks for the fast reply


Georg von Zengen <oni>
Thu 06 Mar 2014 10:49:20 AM UTC, comment #1: 

Curious, what's the point of adding more baudrates?

The code is supposed to allow arbitrary baudrates already as
it is.  The historical Bxxxx constants are mainly there to
support historic code; on all modern systems, Bxxxx is just
the numerical value xxxx anyway.  (The Single Unix Specification
only allows for the use of Bxxxx constants as the portable
method; but then, baud rates beyond 38400 Bd are inherently
non-portable anyway.)

Joerg Wunsch <joerg_wunsch>
Group administrator
Thu 06 Mar 2014 10:34:36 AM UTC, original submission:  

Hi,

This patch add three faster baud rates for posix systems.
500k, 1M, and 2M are the missing "standard" baud rates supported by most newer AVRs.
Please consider to apply this patch to the trunk version of avrdude so it can be part of the next release.

Regards
Georg von Zengen

Georg von Zengen <oni>

 

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

Attached Files
file #30875:  custom_baud_linux_only.patch added by oni (4KiB - text/x-patch)
file #30874:  custom_baud.patch added by oni (5KiB - text/x-patch)
file #30791:  baudrate.patch added by oni (424B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by joerg_wunsch (Posted a comment)
  • -email is unavailable- added by oni (Submitted the item)
  •  

    There are 0 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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-09 joerg_wunsch StatusDone Wont Do
        Open/ClosedClosed Open
    2014-11-25 joerg_wunsch StatusNone Done
        Assigned toNone joerg_wunsch
        Open/ClosedOpen Closed
    2014-03-12 oni Attached File- Added custom_baud_linux_only_under.patch, #30876
    2014-03-12 oni Attached File- Added custom_baud_linux_only.patch, #30875
    2014-03-12 oni Attached File- Added custom_baud.patch, #30874
    2014-03-06 joerg_wunsch StatusNeed Info None
    2014-03-06 joerg_wunsch StatusNone Need Info
    2014-03-06 oni Attached File- Added baudrate.patch, #30791

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code