From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54448)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1YlLPe-0001dg-Bu
	for qemu-devel@nongnu.org; Thu, 23 Apr 2015 13:59:47 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1YlLPb-0005IU-3A
	for qemu-devel@nongnu.org; Thu, 23 Apr 2015 13:59:46 -0400
Received: from mail-ie0-f180.google.com ([209.85.223.180]:33791)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1YlLPa-0005IO-Ve
	for qemu-devel@nongnu.org; Thu, 23 Apr 2015 13:59:43 -0400
Received: by iecrt8 with SMTP id rt8so66708936iec.0
	for <qemu-devel@nongnu.org>; Thu, 23 Apr 2015 10:59:42 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <f865c90b0ca53fce8d7e2a8a1970dd2e30986966.1427108387.git.peter.crosthwaite@xilinx.com>
References: <cover.1427108387.git.peter.crosthwaite@xilinx.com>
	<f865c90b0ca53fce8d7e2a8a1970dd2e30986966.1427108387.git.peter.crosthwaite@xilinx.com>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Thu, 23 Apr 2015 18:59:22 +0100
Message-ID: <CAFEAcA_AODA-mrx8zLxb9D6LWCR2fZES-JDDhbPgSevzsrcT0A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH target-arm v4 10/16] char: cadence_uart:
	Clean up variable names
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Edgar Iglesias <edgar.iglesias@xilinx.com>, zach.pfeffer@xilinx.com, Ryota Ozaki <ozaki.ryota@gmail.com>, QEMU Developers <qemu-devel@nongnu.org>, Alistair Francis <alistair.francis@xilinx.com>, michals@xilinx.com

On 23 March 2015 at 11:05, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> 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 <peter.maydell@linaro.org>

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