On 15 January 2016 at 06:50, Zeng, Star <star.z...@intel.com> wrote: > On 2016/1/15 14:36, Ryan Harkin wrote: >> >> On 15 Jan 2016 01:41, "Zeng, Star" <star.z...@intel.com> wrote: >>> >>> >>> Hi Ryan, >>> >>> >>> On 2016/1/15 3:10, Ryan Harkin wrote: >>>> >>>> >>>> Hi Star, >>>> >>>> This patch breaks the serial terminal for ARM FVP and Juno platforms. >>>> I assume it also breaks TC2 and other such "vexpress" platforms >>>> effected by this change. >>>> >>>> Whilst simple text input seems to work ok, cursor support does not. >>>> And we need cursor support for Intel BDS. >>>> >>>> Below is my hack at fixing the problem. >>>> >>>> Basically, I reintroduce PL011SerialPortExtLib and stub out the >>>> functionality in the functions copied into PL011SerialPortLib. >>> >>> >>> >>> You just copied the function implementation back to >> >> PL011SerialPortExtLib, then it would work? It is so weird, >> >> Yes, that's exactly what I did! Very weird! >> >>> may I know where your fork? >>> >> >> I haven't pushed this branch yet because I had only just started moving to >> the latest EDK2 tree. But my branch is only tianocore plus >> OpenPlatformPkg and my patch below. I see the same problem on FVP models >> on upstream and Juno on upstream if I hack it to use IntelBDS. > > > You means it works well with latest EDK2 TRUNK code if no any change? But it > will not work any more after using IntelBDS? >
No, it does not work well using EDK2 trunk code. Normal text entry works on trunk, it is cursor keys that are broken. Probably some other special keys are broken too. You can use FVP to test trunk. So one can test this problem using the free FVP Foundation model from ARM. But if you want to test Juno on trunk, you have to make a change to ArmJuno.dsc to use Intel BDS instead of ARM BDS. Just like this FVP change: f46ac5f 2015-09-01 ArmPlatformPkg/ArmVExpress-FVP: add support for the Intel BDS [Ard Biesheuvel] git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18378 6f19259b-4bc3-4df7-8a09-765794883524 This is because Intel BDS using cursor keys, ARM BDS does not. > >> >>> Thanks, >>> Star >>> >>> >>>> >>>> Any suggestions about why this is broken or proposals of a proper fix >>>> are welcome. In the meantime, I guess I'll have to carry this patch >>>> in my own fork. >>>> >>>> Regards, >>>> Ryan. >>>> >>>> >>>> From 76352a589d5eaf6e9be28469aba63288f4defb25 Mon Sep 17 00:00:00 2001 >>>> From: Ryan Harkin <ryan.har...@linaro.org> >>>> Date: Thu, 14 Jan 2016 18:41:54 +0000 >>>> Subject: [PATCH] Fix "ArmPlatformPkg: Use SerialDxe in MdeModulePkg >>>> instead of EmbeddedPkg" >>>> >>>> The SerialDxe patch below breaks ARM Ltd. platforms: >>>> >>>>> commit 921e987b2b26602dc85eaee856d494b97b6e02b0 >>>>> Author: Star Zeng <star.z...@intel.com> >>>>> Date: Thu Nov 26 08:51:05 2015 +0000 >>>>> >>>>> ArmPlatformPkg: Use SerialDxe in MdeModulePkg instead of >>>>> EmbeddedPkg >>>>> >>>>> It is also to integrate PL011SerialPortExtLib to >>>>> PL011SerialPortLib. >>>>> >>>>> Cc: Michael D Kinney <michael.d.kin...@intel.com> >>>>> Cc: Liming Gao <liming....@intel.com> >>>>> Cc: Leif Lindholm <leif.lindh...@linaro.org> >>>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Star Zeng <star.z...@intel.com> >>>>> Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org> >>>>> >>>>> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18971 >> >> 6f19259b-4bc3-4df7-8a09-765794883524 >>>> >>>> >>>> >>>> I found that also reverting the change in PL011SerialPortLib.c resulted >>>> in failure to compilate, so I simply changed the return values of the >>>> copied in functions to zero. >>>> >>>> Signed-off-by: Ryan Harkin <ryan.har...@linaro.org> >>>> --- >>>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc | 1 + >>>> .../PL011SerialPortLib/PL011SerialPortExtLib.c | 137 >> >> +++++++++++++++++++++ >>>> >>>> .../PL011SerialPortLib/PL011SerialPortExtLib.inf | 43 +++++++ >>>> .../PL011SerialPortLib/PL011SerialPortLib.c | 12 +- >>>> 4 files changed, 184 insertions(+), 9 deletions(-) >>>> create mode 100644 >>>> ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.c >>>> create mode 100644 >>>> ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.inf >>>> >>>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>>> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>>> index 1b8127d..19c1955 100644 >>>> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>>> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc >>>> @@ -95,6 +95,7 @@ >>>> # ARM PL011 UART Driver >>>> PL011UartLib|ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf >>>> >> >> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf >>>> >>>> + >> >> >> SerialPortExtLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.inf >>>> >>>> # ARM SP804 Dual Timer Driver >>>> TimerLib|ArmPlatformPkg/Library/SP804TimerLib/SP804TimerLib.inf >>>> >>>> diff --git >> >> a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.c >>>> >>>> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.c >>>> new file mode 100644 >>>> index 0000000..c60e7a4 >>>> --- /dev/null >>>> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.c >>>> @@ -0,0 +1,137 @@ >>>> +/** @file >>>> + Serial I/O Port library functions with no library >> >> constructor/destructor >>>> >>>> + >>>> + Copyright (c) 2012-2014, ARM Ltd. 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 >>>> + http://opensource.org/licenses/bsd-license.php >>>> + >>>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >>>> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >> >> IMPLIED. >>>> >>>> + >>>> +**/ >>>> + >>>> +#include <Base.h> >>>> + >>>> +#include <Library/IoLib.h> >>>> +#include <Library/PcdLib.h> >>>> +#include <Library/SerialPortExtLib.h> >>>> + >>>> +#include <Drivers/PL011Uart.h> >>>> + >>>> +/** >>>> + Set new attributes to PL011. >>>> + >>>> + @param BaudRate The baud rate of the serial device. >>>> If the baud rate is not supported, >>>> + the speed will be reduced down to >>>> the nearest supported one and the >>>> + variable's value will be updated >> >> accordingly. >>>> >>>> + @param ReceiveFifoDepth The number of characters the device >>>> will buffer on input. If the specified >>>> + value is not supported, the >>>> variable's value will be reduced down to the >>>> + nearest supported one. >>>> + @param Timeout If applicable, the number of >>>> microseconds the device will wait >>>> + before timing out a Read or a Write >>>> operation. >>>> + @param Parity If applicable, this is the >>>> EFI_PARITY_TYPE that is computed or checked >>>> + as each character is transmitted or >>>> received. If the device does not >>>> + support parity, the value is the >>>> default parity value. >>>> + @param DataBits The number of data bits in each >> >> character >>>> >>>> + @param StopBits If applicable, the >>>> EFI_STOP_BITS_TYPE number of stop bits per character. >>>> + If the device does not support stop >>>> bits, the value is the default stop >>>> + bit value. >>>> + >>>> + @retval EFI_SUCCESS All attributes were set correctly >>>> on the serial device. >>>> + @retval EFI_INVALID_PARAMETERS One or more of the attributes has >>>> an unsupported value. >>>> + >>>> +**/ >>>> +RETURN_STATUS >>>> +EFIAPI >>>> +SerialPortSetAttributes ( >>>> + IN OUT UINT64 *BaudRate, >>>> + IN OUT UINT32 *ReceiveFifoDepth, >>>> + IN OUT UINT32 *Timeout, >>>> + IN OUT EFI_PARITY_TYPE *Parity, >>>> + IN OUT UINT8 *DataBits, >>>> + IN OUT EFI_STOP_BITS_TYPE *StopBits >>>> + ) >>>> +{ >>>> + return PL011UartInitializePort ( >>>> + (UINTN)PcdGet64 (PcdSerialRegisterBase), >>>> + BaudRate, >>>> + ReceiveFifoDepth, >>>> + Parity, >>>> + DataBits, >>>> + StopBits); >>>> +} >>>> + >>>> +/** >>>> + >>>> + Assert or deassert the control signals on a serial port. >>>> + The following control signals are set according their bit settings : >>>> + . Request to Send >>>> + . Data Terminal Ready >>>> + >>>> + @param[in] Control The following bits are taken into account : >>>> + . EFI_SERIAL_REQUEST_TO_SEND : assert/deassert >> >> the >>>> >>>> + "Request To Send" control signal if this bit >>>> is >>>> + equal to one/zero. >>>> + . EFI_SERIAL_DATA_TERMINAL_READY : >> >> assert/deassert >>>> >>>> + the "Data Terminal Ready" control signal if >> >> this >>>> >>>> + bit is equal to one/zero. >>>> + . EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE : >> >> enable/disable >>>> >>>> + the hardware loopback if this bit is equal to >>>> + one/zero. >>>> + . EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE : not >> >> supported. >>>> >>>> + . EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE : >> >> enable/ >>>> >>>> + disable the hardware flow control based on CTS >> >> (Clear >>>> >>>> + To Send) and RTS (Ready To Send) control >> >> signals. >>>> >>>> + >>>> + @retval RETURN_SUCCESS The new control bits were set on the >>>> serial device. >>>> + @retval RETURN_UNSUPPORTED The serial device does not support >>>> this operation. >>>> + >>>> +**/ >>>> +RETURN_STATUS >>>> +EFIAPI >>>> +SerialPortSetControl ( >>>> + IN UINT32 Control >>>> + ) >>>> +{ >>>> + return PL011UartSetControl ((UINTN)PcdGet64 >>>> (PcdSerialRegisterBase), Control); >>>> +} >>>> + >>>> +/** >>>> + >>>> + Retrieve the status of the control bits on a serial device. >>>> + >>>> + @param[out] Control Status of the control bits on a serial device : >>>> + >>>> + . EFI_SERIAL_DATA_CLEAR_TO_SEND, >>>> EFI_SERIAL_DATA_SET_READY, >>>> + EFI_SERIAL_RING_INDICATE, >> >> EFI_SERIAL_CARRIER_DETECT, >>>> >>>> + EFI_SERIAL_REQUEST_TO_SEND, >>>> EFI_SERIAL_DATA_TERMINAL_READY >>>> + are all related to the DTE (Data Terminal >>>> Equipment) and >>>> + DCE (Data Communication Equipment) modes of >>>> operation of >>>> + the serial device. >>>> + . EFI_SERIAL_INPUT_BUFFER_EMPTY : equal to >>>> one if the receive >>>> + buffer is empty, 0 otherwise. >>>> + . EFI_SERIAL_OUTPUT_BUFFER_EMPTY : equal to >>>> one if the transmit >>>> + buffer is empty, 0 otherwise. >>>> + . EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE : equal >>>> to one if the >>>> + hardware loopback is enabled (the ouput >>>> feeds the receive >>>> + buffer), 0 otherwise. >>>> + . EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE : equal >>>> to one if a >>>> + loopback is accomplished by software, 0 >> >> otherwise. >>>> >>>> + . EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE : >>>> equal to one if the >>>> + hardware flow control based on CTS (Clear >>>> To Send) and RTS >>>> + (Ready To Send) control signals is enabled, >>>> 0 otherwise. >>>> + >>>> + @retval RETURN_SUCCESS The control bits were read from the serial >> >> device. >>>> >>>> + >>>> +**/ >>>> +RETURN_STATUS >>>> +EFIAPI >>>> +SerialPortGetControl ( >>>> + OUT UINT32 *Control >>>> + ) >>>> +{ >>>> + return PL011UartGetControl ((UINTN)PcdGet64 >>>> (PcdSerialRegisterBase), Control); >>>> +} >>>> diff --git >> >> a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.inf >>>> >>>> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.inf >>>> new file mode 100644 >>>> index 0000000..38cdeee >>>> --- /dev/null >>>> +++ >>>> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortExtLib.inf >>>> @@ -0,0 +1,43 @@ >>>> +#/** @file >>>> +# >>>> +# Component description file for PL011SerialPortLib module >>>> +# >>>> +# Copyright (c) 2011-2012, ARM Ltd. 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 >>>> +# http://opensource.org/licenses/bsd-license.php >>>> +# >>>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>>> BASIS, >>>> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS >>>> OR IMPLIED. >>>> +# >>>> +#**/ >>>> + >>>> +[Defines] >>>> + INF_VERSION = 0x00010005 >>>> + BASE_NAME = PL011SerialPortExtLib >>>> + FILE_GUID = 2be281f1-c506-4558-bd98-d6930e6de9d6 >>>> + MODULE_TYPE = BASE >>>> + VERSION_STRING = 1.0 >>>> + LIBRARY_CLASS = SerialPortExtLib >>>> + >>>> +[Sources.common] >>>> + PL011SerialPortExtLib.c >>>> + >>>> +[LibraryClasses] >>>> + PL011UartLib >>>> + PcdLib >>>> + >>>> +[Packages] >>>> + EmbeddedPkg/EmbeddedPkg.dec >>>> + MdePkg/MdePkg.dec >>>> + MdeModulePkg/MdeModulePkg.dec >>>> + ArmPlatformPkg/ArmPlatformPkg.dec >>>> + >>>> +[Pcd] >>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits >>>> diff --git >> >> a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >>>> >>>> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >>>> index 7497b5e..8ab3dac 100644 >>>> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >>>> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >>>> @@ -144,13 +144,7 @@ SerialPortSetAttributes ( >>>> IN OUT EFI_STOP_BITS_TYPE *StopBits >>>> ) >>>> { >>>> - return PL011UartInitializePort ( >>>> - (UINTN)PcdGet64 (PcdSerialRegisterBase), >>>> - BaudRate, >>>> - ReceiveFifoDepth, >>>> - Parity, >>>> - DataBits, >>>> - StopBits); >>>> + return 0; >>>> } >>>> >>>> /** >>>> @@ -185,7 +179,7 @@ SerialPortSetControl ( >>>> IN UINT32 Control >>>> ) >>>> { >>>> - return PL011UartSetControl ((UINTN)PcdGet64 >>>> (PcdSerialRegisterBase), Control); >>>> + return 0; >>>> } >>>> >>>> /** >>>> @@ -222,5 +216,5 @@ SerialPortGetControl ( >>>> OUT UINT32 *Control >>>> ) >>>> { >>>> - return PL011UartGetControl ((UINTN)PcdGet64 >>>> (PcdSerialRegisterBase), Control); >>>> + return 0; >>>> } >>>> >>> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
