On 01/26/16 03:55, Ni, Ruiyu wrote: > Please correct me if I was wrong: > Do all these discussions mean the FIFO depth fix is not the root cause?
After re-reading the below again -- I don't think so. The test results seem to be consistent as far as the cursor keys are concerned. With the patches applied, they work; without the patches, they break. So, the test results for the cursor keys have been pretty solid. The tests are inconsistent with regard to F9, which appears to require TTYTERM to be testable from a Linux xterm. However there seems to be some interference from "ArmPlatformPkg/Library/PlatformIntelBdsLib" that is supposed to connect PcdDefaultConInPaths and PcdDefaultConOutPaths. In order to experiment with F9, Roy massaged the last node of the terminal device path (for controlling the parsing of the escape sequences). But, some of those cases proved untestable. Unfortunately, I can't really help with "ArmPlatformPkg/Library/PlatformIntelBdsLib". ArmVirtPkg used that library instance as well originally, but I practically rewrote it separately for ArmVirtPkg, in SVN r16923 and r16924. Since then, the two instances have lived separate lives. Thanks Laszlo > > Regards, > Ray > > > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Friday, January 22, 2016 3:56 AM > To: Ryan Harkin <[email protected]> > Cc: Ni, Ruiyu <[email protected]>; Leif Lindholm <[email protected]>; > [email protected]; Zeng, Star <[email protected]>; Ard Biesheuvel > <[email protected]>; Roy Franz <[email protected]> > Subject: Re: [edk2] [PATCH 3/3] MdeModulePkg: TerminalDxe: select the UART's > default receive FIFO depth > > On 01/21/16 20:47, Ryan Harkin wrote: >> On 21 January 2016 at 18:14, Laszlo Ersek <[email protected]> wrote: >>> On 01/21/16 18:57, Ryan Harkin wrote: >>>> On 21 January 2016 at 17:19, Laszlo Ersek <[email protected]> wrote: >>> >>>>> Ryan, can you please test the following configuration: >>>>> - your PCD update in place, but this three-part series of mine >>>>> *reverted* >>>>> >>>>> This should break the cursor movement keys again, and also break the >>>>> function keys, despite the escape sequences being correct (thanks to >>>>> your PCD update). This would answer Ray's question. >>>>> >>>> >>>> I'm happy to give this a go. But I'm having trouble returning to a >>>> setup where the cursor keys are broken at all. I've reverted the PCD >>>> setting and F9 no longer works, but ... I don't know what I'm doing >>>> wrong. I think I need to take a break from it for 1/2 hour and come >>>> back to work it out. I'm pretty sure I've cleaned the repo and >>>> flushed the builds... it's weird. >>>> >>>> I'll report back soon. >>> >>> Right, I proposed the opposite config for testing -- *keep* the PCD >>> changes, and revert the series in your local tree whose thread we're >>> conducting this discussion in. :) >>> >> >> I wasn't clear - I reverted your three patches: >> >>> #1: revert 31ae446 MdeModulePkg: TerminalDxe: select the UART's default >>> receive FIFO depth >>> #2: revert ea01261 MdeModulePkg: SerialDxe: sync EFI_SERIAL_IO_MODE.Timeout >>> with the spec >>> #3: revert c0beed6 MdeModulePkg: SerialDxe: lay out mSerialIoMode >>> initializer more nicely >>> >> >> ... and cursor keys still worked on the serial port. So I reverted >> the PCD changes too. And cursor keys kept on working. >> >> Now that I've worked out what was happening, let me first do your test >> before I explain why the X Files came to visit. >> >> Using pure upstream EDK2 code: >> >> 1) No SerialDxe patches and no PCD fix >> This means no cursor keys. >> >> 2) Using the PCD fix only >> >> I made this change: >> >> - >> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenPcAnsi();VenHw(407B4008-BF5B-11DF-9547-CF16E0D72085)" >> - >> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenPcAnsi()" >> + >> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)"^M >> + >> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)"^M >> >> And it fails to boot, giving this error message: >> >> Fail to start the console with the Device Path >> 'VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)'. >> (Error 'Not Found') >> >> I guess there's something missing in the upstream .DSC file :-( >> >> So I went back to using OpenPlatformPkg - but without building Shell >> from source. >> >> And it doesn't work. >> >> 3) I add back in your 3 SerialDxe patches >> The serial port comes back to life. >> >> >> >> >>> (Although I find it somewhat flattering that I apparently fixed the cursor >>> keys so well that they *cannot* be broken, even if you try! ;)) >> >> :-) >> >> So I've worked out what my anomaly was. >> >> It turns out that having this patch in my tree also fixes cursor keys >> on the serial port, but not function keys like F9. No, I have no clue >> what this patch has to do with the serial port, other than I am >> testing the cursor keys AFTER Shell has run, so the latest version of >> Shell could be doing something funky with the serial port setup. Like >> changing the FIFO length. >> >> commit eaa23c12db4d32879bb4a6f826d7e003936e11df >> Author: Ryan Harkin <[email protected]> >> Date: Thu Jan 21 08:32:11 2016 +0000 >> >> Platforms/ARM: fvp: build Shell from source >> >> Rather than use a pre-built Shell binary, build the Shell from source. >> >> This allows the developer to debug and fix bugs in Shell, as well as >> test the latest source code. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ryan Harkin <[email protected]> >> --- >> Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf | 2 +- >> Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 19 +++++++++++++++++++ >> 2 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >> b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >> index e3ff0cc..a1ae35a 100644 >> --- a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >> +++ b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >> @@ -214,7 +214,7 @@ FvNameGuid = 87940482-fc81-41c3-87e6-399cf85ac8a0 >> # >> # UEFI application (Shell Embedded Boot Loader) >> # >> - INF ShellBinPkg/UefiShell/UefiShell.inf >> + INF ShellPkg/Application/Shell/Shell.inf^M >> >> # >> # Bds >> diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >> b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >> index 86bad01..d6b02b9 100644 >> --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >> +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >> @@ -483,3 +483,22 @@ >> >> # Legacy Linux Loader >> ArmPkg/Application/LinuxLoader/LinuxLoader.inf >> +^M >> + #^M >> + # UEFI application (Shell Embedded Boot Loader)^M >> + #^M >> + ShellPkg/Application/Shell/Shell.inf {^M >> + <LibraryClasses>^M >> + >> ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf^M >> + >> NULL|ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf^M >> + >> HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf^M >> + PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf^M >> + >> BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf^M >> + }^M >> >> >> Now I need a lie down after all that madness :-O >> > > Ugh. This leaves me confused. I think I'll have to read it very slowly > one more time at least. :) > > Thanks > Laszlo > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
