From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x236.google.com (mail-lf0-x236.google.com [IPv6:2a00:1450:4010:c07::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B44611A1E3D for ; Mon, 10 Oct 2016 11:58:33 -0700 (PDT) Received: by mail-lf0-x236.google.com with SMTP id b75so138403654lfg.3 for ; Mon, 10 Oct 2016 11:58:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=WsDR8ANdlakmy9UIVdP5ZNCxg+bz0RJWgISf+6MlLu8=; b=WygvtDN8qGC3cVzOXlnP5BJpfmU7uxVJUnzP8vvEXVmAM+9U0GtUkZr5H6Tx/bywMB Grrnzjsw2ldJZRbG9gEMxRRfqN06ES8Y9ui5oKx1sLUQbbGySwiWVMBfB1NufI7sEL68 lAz6Hnf8SeSi4rHNSYqx+ruUxf3p6BvI9nxI0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=WsDR8ANdlakmy9UIVdP5ZNCxg+bz0RJWgISf+6MlLu8=; b=T+Kf579fJNcodpajJNubA777b7J6nuvgn9SE4ZNiZPJEIEZ3GK3nehTIyDzqvDmu1S mUYMf+AGQTTQpYWA3khwCL+kp0aBL+4te7beuPy8J419OWmwjyQuO/WOK+cLvI2ULYED SysDeNlDBUVJOs26qafLLx/zSX83xrpFRoewd7thaiSruKzhFXXLNEd3f7fYo0UZhiJ5 xJGseb59QLxSE6dbaL8yqVOKcB6huVZWFdYHwf6lfLpVX6PrZj4GcwyI1K0y6pf5PrZ2 eqKdp2ubS7Xz3a1uh1SE4R6CyfISZt6UZCT5Z51HWSlHAfk7wjHbCE2eIYaM3AkCMoMz hDpQ== X-Gm-Message-State: AA6/9Rk6LvzvMI8X9GPyR9m9Q5ng0A7ja0YOE3MuyepiI+8UYm4ts4ozm7inDcwZglsamgiO X-Received: by 10.194.246.169 with SMTP id xx9mr15456303wjc.76.1476125910970; Mon, 10 Oct 2016 11:58:30 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id f2sm25525540wjr.2.2016.10.10.11.58.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Oct 2016 11:58:30 -0700 (PDT) Date: Mon, 10 Oct 2016 19:58:27 +0100 From: Leif Lindholm To: evan.lloyd@arm.com Cc: edk2-devel@lists.01.org, Ard Biesheuvel , Ryan Harkin Message-ID: <20161010185827.GL3471@bivouac.eciton.net> References: <20160921203315.11204-1-evan.lloyd@arm.com> <20160921203315.11204-2-evan.lloyd@arm.com> MIME-Version: 1.0 In-Reply-To: <20160921203315.11204-2-evan.lloyd@arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH 1/3] ArmPlatformPkg: Fix PL011 FIFO size test X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2016 18:58:34 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Apologies for delay, now catching up post plugfest and Linaro Connect. On Wed, Sep 21, 2016 at 09:33:13PM +0100, evan.lloyd@arm.com wrote: > From: Evan Lloyd > > This change updates PL011UartInitializePort to compare ReceiveFifoDepth > with the correct hardware FIFO size instead of the constant 32 used > previously. > This corrects a minor bug where a request for a fifo size > 15 and < 32 > would not have been honoured on a system with a 16 byte FIFO. Whoops. Well, even without the bug, the below is an improvement in readability. > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Sami Mujawar > Signed-off-by: Evan Lloyd > --- > ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > index 3748972acbfc80f1077b9b928756a72cc77b5c75..b3ea138bf60b93a1000dd29aedaad206f2d15f2b 100644 > --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.c > @@ -79,17 +79,18 @@ PL011UartInitializePort ( > UINT32 Divisor; > UINT32 Integer; > UINT32 Fractional; > + UINT32 HardwareFifoDepth; > > + HardwareFifoDepth = (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) \ > + > PL011_VER_R1P4) \ > + ? 32 : 16 ; I may drop that space before ';' before committing, but apart from that: Reviewed-by: Leif Lindholm > // The PL011 supports a buffer of 1, 16 or 32 chars. Therefore we can accept > // 1 char buffer as the minimum FIFO size. Because everything can be rounded > // down, there is no maximum FIFO size. > - if ((*ReceiveFifoDepth == 0) || (*ReceiveFifoDepth >= 32)) { > + if ((*ReceiveFifoDepth == 0) || (*ReceiveFifoDepth >= HardwareFifoDepth)) { > // Enable FIFO > LineControl = PL011_UARTLCR_H_FEN; > - if (PL011_UARTPID2_VER (MmioRead32 (UartBase + UARTPID2)) > PL011_VER_R1P4) > - *ReceiveFifoDepth = 32; > - else > - *ReceiveFifoDepth = 16; > + *ReceiveFifoDepth = HardwareFifoDepth; > } else { > // Disable FIFO > LineControl = 0; > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >