Well, a few things jump out at me (there's a big one at the end, so
you might want to read them all before starting to respond).
- If it's a boolean flag, why not declare it a bool rather than
unsigned char? It generates the same code.
> uart_console_write(port, s, count, serial8250_console_putchar);
>
> + if (up->writing_to_con)
> + up->writing_to_con = 0;
> /*
- Do you see what's wrong with that if() line, or so I have to
point it out?
> ignore_char:
> + /*
> + * We see BRK while buffering rx bytes. Stop buffering and
> + * let SysRq or SAK handling be done after console_write
> + */
> + if (port->sysrq && up->writing_to_con)
> + break;
> lsr = serial_in(up, UART_LSR);
} while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0));
- Why do you skip re-reading the lsr and return the old stale LSR value
if you receive a SysRq?
It seems that you're doing things twice: you set max_count to 1, so the
is guaranteed to exit immediately, and you add a break here.
It seems simpler to rely on the fact that sysrq is set to 1 if and only
if uart_handle_break returns 1, and do:
- if (uart_handle_break(port))
+ if (uart_handle_break(port)) {
+ if (up->writing_to_con)
+ max_count = 0;
goto ignore_char;
+ }
- On a higher level, could you make the flag a parameter to
wait_for_xmitr? That would make the logic much clearer;
this "set a magic flag which causes a low-level function to
do different magic stuff" is a particularly evil form of
kludgery which I generally like to avoid if at all possible.
Actually, it would probably be less code churn to create a new
3-parameter poll-wait function, and make wait_for_xmitr a wrapper.
The new function would also have to check port->sysrq by itself,
of course; the caller would only check oops_in_progress.
You'd also need another parameter to serial8250_rx_chars, of
course. It could just be max_chars.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
