From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (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 048F52116DA13 for ; Fri, 12 Oct 2018 07:49:39 -0700 (PDT) Received: by mail-it1-x144.google.com with SMTP id i76-v6so18541738ita.3 for ; Fri, 12 Oct 2018 07:49:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=cKHNBMwgdy2dxJ46ZA3qfINYHiN0GUS2cdDAf9+nU1I=; b=THnvGJeQv1fLDQuto10CPmumTBD/jAPcOA+XTvbS4QzDu192oo96at+VEB61hP889o k3WHe+Bw+WXJVbrbrRf/xCJG4R/AkChGG+Gk6/YzIwjKCRuIO3DkiD7Q2d18JtbMCJZX w38hiTSTcA0v9mNmC5rzAtrRuPjpELk6mcZUU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=cKHNBMwgdy2dxJ46ZA3qfINYHiN0GUS2cdDAf9+nU1I=; b=Bf4TBaYIUmPWL5CnPCx5++gxsk9MrWxspcfCQRKoZ+HfD3UynntVocPMY3ykgcnyqZ qesngAE1F4Ie0GTCaj6LmGebPgOvOFgd/Uh0vEk0SqqYznAdz/qs0GybtRoopF3howRR yBy2RVfos/JRLAy/xC+BDKF/2fBBfhe+I8FL35sa5ic78vy0QsCrg4Oz/4l4pWwyqR+L p0cXwVMNYdt/x5KG3K31lNga+i862qDDYcWCVJuuONjctD0ugbyyGgyH4hKKTmw27YsA d7nRNmRV5od1lB83hlpIbaRaztUTR+g4ZNhybnUyv2zQdVrzwU6fsEWjj6s0qFMlNddC zdCA== X-Gm-Message-State: ABuFfogv/PRUgu9zI8Bdto44jWA3dMrJmfAeYS69v58mpaa7uERGtKM+ /JBsZF1lSfZms9o/6xM5xRmZ59Lt/SPTbPewACMYWFJ3 X-Google-Smtp-Source: ACcGV61qUvAEB4nXWb5FbKK7YoZ2OLaXavnJL21sEo9+HVpcsWRR25++k/omlQ71W90/F/zDyBMm/DdUSXf9Yf8oCSE= X-Received: by 2002:a24:4ac5:: with SMTP id k188-v6mr5489928itb.158.1539355778785; Fri, 12 Oct 2018 07:49:38 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:5910:0:0:0:0:0 with HTTP; Fri, 12 Oct 2018 07:49:38 -0700 (PDT) In-Reply-To: <20181012144009.48732-2-sami.mujawar@arm.com> References: <20181012144009.48732-1-sami.mujawar@arm.com> <20181012144009.48732-2-sami.mujawar@arm.com> From: Ard Biesheuvel Date: Fri, 12 Oct 2018 16:49:38 +0200 Message-ID: To: Sami Mujawar Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Ruiyu Ni , Evan Lloyd , Matteo Carlini , Stephanie Hughes-Fitt , nd Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Oct 2018 14:49:40 -0000 Content-Type: text/plain; charset="UTF-8" On 12 October 2018 at 16:40, Sami Mujawar wrote: > Some virtual machine managers like kvmtool emulate the PC AT > Serial Port UART in the MMIO space so that architectures that > do not support I/O Mapped I/O can use the UART. > > This patch adds MMIO support to the PC AT SerialPortLib. > > PcdSerialUseMmio is used to select I/O or MMIO support. > If PcdSerialUseMmio is > TRUE - The value is read/written from MMIO space. > FALSE - The value is read/written from I/O space. > The Default is I/O space. > IIUC kvmtool exposes a 8250, and our 8250 driver already implements support for MMIO mapped registers. Can't you use that instead? > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Sami Mujawar > --- > The changes can be seen at https://github.com/samimujawar/edk2/commit/84f908387d2031def5d374759e7ad4cf90786c91 > > Notes: > v1: > - Add support to read/write from UART registers using MMIO access [SAMI] > > PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 + > PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98 ++++++++++++++++---- > 2 files changed, 84 insertions(+), 18 deletions(-) > > diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf > index 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd930701b02dae51ff59ea16a10c4 100644 > --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf > +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf > @@ -2,6 +2,7 @@ > # Library instance for SerialIo library class > # > # Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
> +# Copyright (c) 2018, ARM Limited. All rights reserved.
> # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD License > # which accompanies this distribution. The full text of the license may be found at > @@ -23,6 +24,7 @@ [Defines] > > [Packages] > MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > > [LibraryClasses] > BaseLib > @@ -31,3 +33,5 @@ [LibraryClasses] > [Sources] > SerialPortLib.c > > +[FixedPcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES > diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c > index d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998624189c6b538912da0439dfe8 100644 > --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c > +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c > @@ -2,6 +2,8 @@ > UART Serial Port library functions > > Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2018, ARM Limited. All rights reserved.
> + > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -57,6 +59,66 @@ UINT8 gParity = 0; > UINT8 gBreakSet = 0; > > /** > + Reads an 8-bit value from the serial port. > + > + This function checks PcdSerialUseMmio to determine if I/O > + mapped IO or Memory Mapped IO operations must be performed on > + the serial port. It then uses the appropriate interface to > + read the Value from the serial port. > + > + If PcdSerialUseMmio is TRUE, then the value is read from MMIO space. > + If PcdSerialUseMmio is FALSE, then the value is read from I/O space. > + > + @param Address The UART register to read from. > + > + @return The value read from the serial port. > + > +**/ > +STATIC > +UINT8 > +SerialPortRead8 ( > + IN UINTN Address > +) > +{ > + if (FixedPcdGetBool (PcdSerialUseMmio)) { > + return MmioRead8 (Address); > + } > + > + return IoRead8 (Address); > +} > + > +/** > + Writes an 8-bit value to the serial port. > + > + This function checks PcdSerialUseMmio to determine if I/O > + mapped IO or Memory Mapped IO operations must be performed on > + the serial port. It then uses the appropriate interface to > + write the Value to the serial port. > + > + If PcdSerialUseMmio is TRUE, then the value is written to MMIO space. > + If PcdSerialUseMmio is FALSE, then the value is written to I/O space. > + > + @param Address The UART register to write. > + @param Value The value to write to the I/O port. > + > + @return The value written to the serial port. > + > +**/ > +STATIC > +UINT8 > +SerialPortWrite8 ( > + IN UINTN Address, > + IN UINT8 Value > +) > +{ > + if (FixedPcdGetBool (PcdSerialUseMmio)) { > + return MmioWrite8 (Address, Value); > + } > + > + return IoWrite8 (Address, Value); > +} > + > +/** > Initialize the serial device hardware. > > If no initialization is required, then return RETURN_SUCCESS. > @@ -91,19 +153,19 @@ SerialPortInitialize ( > // Set communications format > // > OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity << 3) | (gStop << 2) | Data); > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); > + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); > > // > // Configure baud rate > // > - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8)); > - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff)); > + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8)); > + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff)); > > // > // Switch back to bank 0 > // > OutputData = (UINT8) ( (gBreakSet << 6) | (gParity << 3) | (gStop << 2) | Data); > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); > + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); > > return RETURN_SUCCESS; > } > @@ -148,9 +210,9 @@ SerialPortWrite ( > // Wait for the serail port to be ready. > // > do { > - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); > + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); > } while ((Data & LSR_TXRDY) == 0); > - IoWrite8 ((UINT16) gUartBase, *Buffer++); > + SerialPortWrite8 ((UINT16) gUartBase, *Buffer++); > } > > return Result; > @@ -189,10 +251,10 @@ SerialPortRead ( > // Wait for the serail port to be ready. > // > do { > - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); > + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); > } while ((Data & LSR_RXDA) == 0); > > - *Buffer++ = IoRead8 ((UINT16) gUartBase); > + *Buffer++ = SerialPortRead8 ((UINT16) gUartBase); > } > > return Result; > @@ -220,7 +282,7 @@ SerialPortPoll ( > // > // Read the serial port status. > // > - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); > + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); > > return (BOOLEAN) ((Data & LSR_RXDA) != 0); > } > @@ -253,7 +315,7 @@ SerialPortSetControl ( > // > // Read the Modem Control Register. > // > - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET); > + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET); > Mcr &= (~(MCR_DTRC | MCR_RTS)); > > if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) == EFI_SERIAL_DATA_TERMINAL_READY) { > @@ -267,7 +329,7 @@ SerialPortSetControl ( > // > // Write the Modem Control Register. > // > - IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr); > + SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr); > > return RETURN_SUCCESS; > } > @@ -297,7 +359,7 @@ SerialPortGetControl ( > // > // Read the Modem Status Register. > // > - Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET); > + Msr = SerialPortRead8 ((UINT16) gUartBase + MSR_OFFSET); > > if ((Msr & MSR_CTS) == MSR_CTS) { > *Control |= EFI_SERIAL_CLEAR_TO_SEND; > @@ -318,7 +380,7 @@ SerialPortGetControl ( > // > // Read the Modem Control Register. > // > - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET); > + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET); > > if ((Mcr & MCR_DTRC) == MCR_DTRC) { > *Control |= EFI_SERIAL_DATA_TERMINAL_READY; > @@ -331,7 +393,7 @@ SerialPortGetControl ( > // > // Read the Line Status Register. > // > - Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); > + Lsr = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); > > if ((Lsr & LSR_TXRDY) == LSR_TXRDY) { > *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY; > @@ -470,19 +532,19 @@ SerialPortSetAttributes ( > // Set communications format > // > OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData); > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); > + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); > > // > // Configure baud rate > // > - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8)); > - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff)); > + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8)); > + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff)); > > // > // Switch back to bank 0 > // > OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData); > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); > + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); > > return RETURN_SUCCESS; > } > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > >