From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Sami Mujawar <sami.mujawar@arm.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Leif Lindholm <leif.lindholm@linaro.org>,
Ruiyu Ni <ruiyu.ni@intel.com>, Evan Lloyd <evan.lloyd@arm.com>,
Matteo Carlini <Matteo.Carlini@arm.com>,
Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>,
nd <nd@arm.com>
Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
Date: Fri, 12 Oct 2018 16:49:38 +0200 [thread overview]
Message-ID: <CAKv+Gu_bqb=XJ++WBzQsUizsKxJeaDC3EZJkxTgz8jjXSjTEVA@mail.gmail.com> (raw)
In-Reply-To: <20181012144009.48732-2-sami.mujawar@arm.com>
On 12 October 2018 at 16:40, Sami Mujawar <sami.mujawar@arm.com> 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 <sami.mujawar@arm.com>
> ---
> 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.<BR>
> +# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> # 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.<BR>
> + Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> +
> 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)'
>
>
next prev parent reply other threads:[~2018-10-12 14:49 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
2018-10-12 14:40 ` [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib Sami Mujawar
2018-10-12 14:49 ` Ard Biesheuvel [this message]
2018-10-12 15:06 ` Sami Mujawar
2018-10-12 15:31 ` Kinney, Michael D
2018-10-12 15:33 ` Ard Biesheuvel
2018-10-15 2:38 ` Ni, Ruiyu
2018-10-12 14:40 ` [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
2018-10-13 10:51 ` Leif Lindholm
2018-10-12 14:40 ` [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory Sami Mujawar
2018-10-13 9:09 ` Zeng, Star
2018-10-13 21:28 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD Sami Mujawar
2018-10-13 21:35 ` Laszlo Ersek
2018-10-19 14:01 ` Ard Biesheuvel
2018-10-12 14:40 ` [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
2018-10-13 21:54 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform Sami Mujawar
2018-10-13 21:57 ` Laszlo Ersek
2018-10-13 21:42 ` [PATCH 0/6] Add kvmtool emulated platform support for ARM Laszlo Ersek
2018-10-16 3:00 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKv+Gu_bqb=XJ++WBzQsUizsKxJeaDC3EZJkxTgz8jjXSjTEVA@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox