On 15 January 2016 at 10:03, Zeng, Star <star.z...@intel.com> wrote: > On 2016/1/15 16:56, Ryan Harkin wrote: >> >> " >> >> On 15 January 2016 at 08:51, Zeng, Star <star.z...@intel.com> wrote: >>> >>> On 2016/1/15 16:08, Ryan Harkin wrote: >>>> >>>> >>>> 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. >>> >>> >>> >>> Sorry, I am not familiar with ARM platform and can only analyze the >>> problem >>> by code review. Up to now, I have no idea about the reason since the code >>> implementation are same but just in different library instances. >>> >>> Could you push your code into your fork? Then we can get it to have a >>> look. >>> >> >> Sure, I've just pushed it: >> >> >> https://git.linaro.org/landing-teams/working/arm/edk2.git/shortlog/refs/heads/serialdxe-fix-005 >> >> It's in the "serialdxe-fix-005" branch. >> > > I checked out the code and had a look. I found there is no one to link the > added back PL011SerialPortExtLib, and I do not understand why the added back > PL011SerialPortExtLib will cause PL011SerialPortLib to have compile failure > you said in previous email and commit log. >
You misunderstand me, or I was not clear, sorry. This is library all voodoo to me, so I was just reverting your patch in a way that compiles. When trying to revert your patch on the latest trunk code, the build fails because your patch is doing too many things in one step. It should have been three (or more) patches, in my opinion. Instead of reverting, I manually added back PL011SerialPortExtLib and removed the code from PL011SerialPortLib at the same time. Then, the code did not compile. So I changed my patch to also make PL011SerialPortLib return 0 instead of calling the PL011 functions. And everything started to work. I assumed this was because I added back the ExtLib, not because I changed the added code. I was wrong. I've just pushed a new branch, serialdxe-fix-006, that only changes PL011SerialPortLib.c to "return 0;" for all the new functions - and that works also. So I don't need PL011SerialPortExtLib at all. I guess it was never called from there? When you moved the code to PL011SerialPortLib, it started getting called - and the code is causing problems. > Could you check out to *SHA-1: 1b96428d92c01383dc437717db679f57cf70d980* > that just before my SerialDxe series changes, and help to confirm if there > is regression? > I already checked out the commit before yours [1] and it works fine. [1] git checkout 921e987b2b26602dc85eaee856d494b97b6e02b0^ _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
