From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0217121959CB2 for ; Fri, 12 Oct 2018 08:31:52 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Oct 2018 08:31:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,372,1534834800"; d="scan'208";a="99714696" Received: from orsmsx107.amr.corp.intel.com ([10.22.240.5]) by orsmga002.jf.intel.com with ESMTP; 12 Oct 2018 08:31:51 -0700 Received: from orsmsx162.amr.corp.intel.com (10.22.240.85) by ORSMSX107.amr.corp.intel.com (10.22.240.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 12 Oct 2018 08:31:51 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.89]) by ORSMSX162.amr.corp.intel.com ([169.254.3.24]) with mapi id 14.03.0319.002; Fri, 12 Oct 2018 08:31:51 -0700 From: "Kinney, Michael D" To: Sami Mujawar , Ard Biesheuvel , "Kinney, Michael D" CC: "Ni, Ruiyu" , nd , Stephanie Hughes-Fitt , "edk2-devel@lists.01.org" Thread-Topic: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib Thread-Index: AQHUYj0rO7iA7KvogEyYs9usG4+1AqUbvAzg Date: Fri, 12 Oct 2018 15:31:51 +0000 Message-ID: References: <20181012144009.48732-1-sami.mujawar@arm.com> <20181012144009.48732-2-sami.mujawar@arm.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.22.254.139] MIME-Version: 1.0 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 15:31:53 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sami, The BaseSerialPortLib16550 supports I/O port, MMIO, and PCI based UARTs. PCDs are used to select the modes. I would like to see the BaseSerialPortLib16550 used instead of the serial port lib in PcAtChipsetPkg. Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] > On Behalf Of Sami Mujawar > Sent: Friday, October 12, 2018 8:06 AM > To: Ard Biesheuvel > Cc: Ni, Ruiyu ; nd ; > Stephanie Hughes-Fitt ; > edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH v1 1/6] PcAtChipsetPkg: Add > MMIO Support to SerialIo Lib >=20 > Hi Ard, >=20 > I believe you are referring to > MdeModulePkg\Library\BaseSerialPortLib16550\BaseSerialPor > tLib16550.inf > I have tried using this Serial Port Library. However, > this has a dependency on PciLib which is difficult to > resolve as we use the Serial port early in the boot. > Please do let me know if you know a workaround to solve > this issue. >=20 > Regards, >=20 > Sami Mujawar >=20 > -----Original Message----- > From: Ard Biesheuvel > Sent: 12 October 2018 03:50 PM > 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 >=20 > 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. > > >=20 > IIUC kvmtool exposes a 8250, and our 8250 driver already > implements support for MMIO mapped registers. Can't you > use that instead? >=20 > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Sami Mujawar > > --- > > The changes can be seen at > > > https://github.com/samimujawar/edk2/commit/84f908387d2031 > def5d374759e7 > > ad4cf90786c91 > > > > 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..d0689793040fd93 > 0701b02dae51f > > f59ea16a10c4 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..898e5f957aae998 > 624189c6b5389 > > 12da0439dfe8 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 =3D 0; > > UINT8 gBreakSet =3D 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 =3D (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 =3D (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 =3D IoRead8 ((UINT16) gUartBase + > LSR_OFFSET); > > + Data =3D SerialPortRead8 ((UINT16) gUartBase + > LSR_OFFSET); > > } while ((Data & LSR_TXRDY) =3D=3D 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 =3D IoRead8 ((UINT16) gUartBase + > LSR_OFFSET); > > + Data =3D SerialPortRead8 ((UINT16) gUartBase + > LSR_OFFSET); > > } while ((Data & LSR_RXDA) =3D=3D 0); > > > > - *Buffer++ =3D IoRead8 ((UINT16) gUartBase); > > + *Buffer++ =3D SerialPortRead8 ((UINT16) gUartBase); > > } > > > > return Result; > > @@ -220,7 +282,7 @@ SerialPortPoll ( > > // > > // Read the serial port status. > > // > > - Data =3D IoRead8 ((UINT16) gUartBase + LSR_OFFSET); > > + Data =3D SerialPortRead8 ((UINT16) gUartBase + > LSR_OFFSET); > > > > return (BOOLEAN) ((Data & LSR_RXDA) !=3D 0); } @@ - > 253,7 +315,7 @@ > > SerialPortSetControl ( > > // > > // Read the Modem Control Register. > > // > > - Mcr =3D IoRead8 ((UINT16) gUartBase + MCR_OFFSET); > > + Mcr =3D SerialPortRead8 ((UINT16) gUartBase + > MCR_OFFSET); > > Mcr &=3D (~(MCR_DTRC | MCR_RTS)); > > > > if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) =3D=3D > > 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 =3D IoRead8 ((UINT16) gUartBase + MSR_OFFSET); > > + Msr =3D SerialPortRead8 ((UINT16) gUartBase + > MSR_OFFSET); > > > > if ((Msr & MSR_CTS) =3D=3D MSR_CTS) { > > *Control |=3D EFI_SERIAL_CLEAR_TO_SEND; @@ -318,7 > +380,7 @@ > > SerialPortGetControl ( > > // > > // Read the Modem Control Register. > > // > > - Mcr =3D IoRead8 ((UINT16) gUartBase + MCR_OFFSET); > > + Mcr =3D SerialPortRead8 ((UINT16) gUartBase + > MCR_OFFSET); > > > > if ((Mcr & MCR_DTRC) =3D=3D MCR_DTRC) { > > *Control |=3D EFI_SERIAL_DATA_TERMINAL_READY; @@ - > 331,7 +393,7 @@ > > SerialPortGetControl ( > > // > > // Read the Line Status Register. > > // > > - Lsr =3D IoRead8 ((UINT16) gUartBase + LSR_OFFSET); > > + Lsr =3D SerialPortRead8 ((UINT16) gUartBase + > LSR_OFFSET); > > > > if ((Lsr & LSR_TXRDY) =3D=3D LSR_TXRDY) { > > *Control |=3D EFI_SERIAL_OUTPUT_BUFFER_EMPTY; @@ - > 470,19 +532,19 @@ > > SerialPortSetAttributes ( > > // Set communications format > > // > > OutputData =3D (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 =3D (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)' > > > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel