Change in flashrom[staging]: serial: support custom baud rates on linux

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20224 )

Change subject: serial: support custom baud rates on linux
......................................................................

Patch Set 1:

(10 comments)

With a generic set_custom_baudrate() and guarding done in the header
file, the code could be free of guards. See inline comments.

https://review.coreboot.org/#/c/20224/1//COMMIT_MSG
Commit Message:

https://review.coreboot.org/#/c/20224/1//COMMIT_MSG@7
PS1, Line 7: support
Upper-case "Support" please.

https://review.coreboot.org/#/c/20224/1//COMMIT_MSG@9
PS1, Line 9: the
Upper-case...

https://review.coreboot.org/#/c/20224/1//COMMIT_MSG@9
PS1, Line 9: because
           : broken
because *of* broken?

https://review.coreboot.org/#/c/20224/1/linux_baud.h
File linux_baud.h:

PS1: 
Missing include guards and license header.

https://review.coreboot.org/#/c/20224/1/linux_baud.h@1
PS1, Line 1: int linux_set_custom_baudrate(int fd, int speed);
Why not call it generically `set_custom_baudrate`? You could
provide a stub then in case `#if IS_LINUX != 1` and wouldn't
have to bother with the guard in the code.

https://review.coreboot.org/#/c/20224/1/linux_baud.c
File linux_baud.c:

https://review.coreboot.org/#/c/20224/1/linux_baud.c@35
PS1, Line 35:  * for more info. */
Urgh, who invented this comment style? Not your fault, I know
this style is common in flashrom. Though, I'd prefer the style
like used above for the license header, with the dangling /*
and */ on separate lines.

Also, I wouldn't place URLs in the code. They tend to break.
Rather put them into the commit message?

https://review.coreboot.org/#/c/20224/1/serial.c
File serial.c:

https://review.coreboot.org/#/c/20224/1/serial.c@44
PS1, Line 44: #if IS_LINUX
Guarding a #include shouldn't be necessary.

https://review.coreboot.org/#/c/20224/1/serial.c@126
PS1, Line 126:  * so setting a custom rate can be attempted. */
How about a use_custom_baud() function instead, that returns true in
case the requested baud rate isn't present in the sp_baudtable? That
is_custom_baud() could just always return false `#if IS_LINUX != 1`.

https://review.coreboot.org/#/c/20224/1/serial.c@210
PS1, Line 210: 	if (baud >= 0) {
With the mentioned use_custom_baud() and an always present
set_custom_baudrate() the code flow could be free of guards
and look as follows:

if (use_custom_baud()) {
            ...
            if (set_custom_baudrate())
                    ...
            ...
    } else {
            const struct baudentry *entry = round_baud(baud);
            if (cfsetispeed...) {
                    ...
            }
    }

https://review.coreboot.org/#/c/20224/1/serial.c@257
PS1, Line 257: 
Was this line break added intentionally?