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 authentification level.

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

Submitted by:  Georg von Zengen <oni>
Submitted on:  Thu 06 Mar 2014 10:34:36 AM UTC  
 
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open

(Jump to the original submission Jump to the original submission)

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

 

Attached Files
file #30875:  custom_baud_linux_only.patch added by oni (4kB - text/x-patch)
file #30874:  custom_baud.patch added by oni (5kB - 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
  • -unavailable- added by joerg_wunsch (Posted a comment)
  • -unavailable- added by oni (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 12 Mar 2014 01:37:46 PM UTConiAttached File-=>Added custom_baud_linux_only_under.patch, #30876
    Wed 12 Mar 2014 12:54:39 PM UTConiAttached File-=>Added custom_baud_linux_only.patch, #30875
    Wed 12 Mar 2014 10:22:31 AM UTConiAttached File-=>Added custom_baud.patch, #30874
    Thu 06 Mar 2014 12:26:18 PM UTCjoerg_wunschStatusNeed Info=>None
    Thu 06 Mar 2014 10:49:20 AM UTCjoerg_wunschStatusNone=>Need Info
    Thu 06 Mar 2014 10:34:36 AM UTConiAttached File-=>Added baudrate.patch, #30791

    Back to the top


    Powered by Savane 3.1-cleanup