Re: [Qemu-devel] [PATCH target-arm v4 10/16] char: cadence_uart: Clean up variable names

[Peter Maydell]

On 23 March 2015 at 11:05, Peter Crosthwaite
<address@hidden> wrote:
> In preparation for migrating the state struct and type cast macro to a public
> header. The acronym "UART" on it's own is not specific enough to be used in a
> more global namespace so preface with "cadence". Fix the capitalisation of
> "uart" in the state type while touching the typename. Preface macros
> used by the state struct itself with CADENCE_UART so they don't conflict
> in namespace either.

This is another non-standalone commit message. Other than that:
Reviewed-by: Peter Maydell <address@hidden>

> -    s->r[R_SR] |= s->rx_count == RX_FIFO_SIZE ? UART_SR_INTR_RFUL : 0;
> +    s->r[R_SR] |= s->rx_count == CADENCE_UART_RX_FIFO_SIZE ? 
> UART_SR_INTR_RFUL
> +                                                           : 0;

These look kind of ugly as the line length got long enough to wrap.
    if (s->rx_count == CADENCE_UART_RX_FIFO_SIZE) {
        s->r[R_SR] |= UART_SR_INTR_RFUL;
    }
looks nicer to me (and makes it clearer that the bit only updates
if the condition is true, rather than leaving you to figure it out
from the combination of the OR and the ternary operator with a zero
operand), but that kind of change doesn't belong in this
no-content-change patch. You could do it in a later patch, or
not do it at all if you don't think it's important enough (I
don't care strongly either way).

-- PMM