| Message ID | 1458225414-19812-3-git-send-email-heyi.guo@linaro.org |
|---|---|
| State | Accepted |
| Commit | f423d76021df725814a9bebcfeeb8cf57e3c8beb |
| Headers | show |
On 03/23/16 09:33, Ni, Ruiyu wrote: > Laszlo, > Since the patch below lets SerialDxe driver use the new introduced > PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. > Would you mind to revert the check in @ 31ae446b > --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? If I understand correctly, this series increases the terminal polling rate, so that cursor movement escape sequences (and other burst-like sequences) can be processed even with a receive fifo depth of 1. Hence 31ae446b should become unnecessary. If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test your revert of 31ae446b -- on top of this series from Heyi --, and the cursor keys work with 31ae446b reverted, then I don't mind, sure. Thanks Laszlo > > Regards, > Ray > > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >> Sent: Thursday, March 17, 2016 10:37 PM >> To: edk2-devel@lists.01.org >> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >> >> Set UART receive FIFO depth with PCD instead of fixed number "1". >> The default value of PCD is also 1, so it makes no difference for >> platforms which do not explicitly set this PCD. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >> Cc: Feng Tian <feng.tian@intel.com> >> Cc: Star Zeng <star.zeng@intel.com> >> --- >> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >> 2 files changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >> index 164060b..a1453bd 100644 >> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >> @@ -41,10 +41,11 @@ >> gEfiDevicePathProtocolGuid ## PRODUCES >> >> [Pcd] >> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >> >> [Depex] >> TRUE >> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >> index f5b3064..d2383e5 100644 >> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >> @@ -236,7 +236,7 @@ SerialReset ( >> // >> // Set the Serial I/O mode >> // >> - This->Mode->ReceiveFifoDepth = 1; >> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >> This->Mode->Timeout = 1000 * 1000; >> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >> -- >> 2.7.0 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 March 2016 at 08:53, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/23/16 09:33, Ni, Ruiyu wrote: >> Laszlo, >> Since the patch below lets SerialDxe driver use the new introduced >> PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. >> Would you mind to revert the check in @ 31ae446b >> --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? > > If I understand correctly, this series increases the terminal polling > rate, so that cursor movement escape sequences (and other burst-like > sequences) can be processed even with a receive fifo depth of 1. Hence > 31ae446b should become unnecessary. > > If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test > your revert of 31ae446b -- on top of this series from Heyi --, and the > cursor keys work with 31ae446b reverted, then I don't mind, sure. > Unfortunately TC2 and Juno no longer with with 31ae446b reverted and these 3 patches from the series applied. It's a shame, because it works on FVP models and copy/paste then works. I haven't made any investigations on what's going wrong. But regular ASCII keys work, control codes like cursor keys don't. So it looks like the old FIFO setting problem we discussed a few weeks ago. > Thanks > Laszlo > >> >> Regards, >> Ray >> >> >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >>> Sent: Thursday, March 17, 2016 10:37 PM >>> To: edk2-devel@lists.01.org >>> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >>> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >>> >>> Set UART receive FIFO depth with PCD instead of fixed number "1". >>> The default value of PCD is also 1, so it makes no difference for >>> platforms which do not explicitly set this PCD. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>> Cc: Feng Tian <feng.tian@intel.com> >>> Cc: Star Zeng <star.zeng@intel.com> >>> --- >>> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >>> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >>> 2 files changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>> index 164060b..a1453bd 100644 >>> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>> @@ -41,10 +41,11 @@ >>> gEfiDevicePathProtocolGuid ## PRODUCES >>> >>> [Pcd] >>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >>> >>> [Depex] >>> TRUE >>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>> index f5b3064..d2383e5 100644 >>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>> @@ -236,7 +236,7 @@ SerialReset ( >>> // >>> // Set the Serial I/O mode >>> // >>> - This->Mode->ReceiveFifoDepth = 1; >>> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>> This->Mode->Timeout = 1000 * 1000; >>> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >>> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >>> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >>> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >>> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >>> -- >>> 2.7.0 >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 March 2016 at 16:24, Ryan Harkin <ryan.harkin@linaro.org> wrote: > On 23 March 2016 at 08:53, Laszlo Ersek <lersek@redhat.com> wrote: >> On 03/23/16 09:33, Ni, Ruiyu wrote: >>> Laszlo, >>> Since the patch below lets SerialDxe driver use the new introduced >>> PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. >>> Would you mind to revert the check in @ 31ae446b >>> --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? >> >> If I understand correctly, this series increases the terminal polling >> rate, so that cursor movement escape sequences (and other burst-like >> sequences) can be processed even with a receive fifo depth of 1. Hence >> 31ae446b should become unnecessary. >> >> If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test >> your revert of 31ae446b -- on top of this series from Heyi --, and the >> cursor keys work with 31ae446b reverted, then I don't mind, sure. >> > > Unfortunately TC2 and Juno no longer with with 31ae446b reverted and > these 3 patches from the series applied. I'm sure you would all work it out ok, but that should have been: "Unfortunately TC2 and Juno no longer work with 31ae446b reverted and these 3 patches from the series applied." > > It's a shame, because it works on FVP models and copy/paste then works. > > I haven't made any investigations on what's going wrong. But regular > ASCII keys work, control codes like cursor keys don't. So it looks > like the old FIFO setting problem we discussed a few weeks ago. > > >> Thanks >> Laszlo >> >>> >>> Regards, >>> Ray >>> >>> >>>> -----Original Message----- >>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >>>> Sent: Thursday, March 17, 2016 10:37 PM >>>> To: edk2-devel@lists.01.org >>>> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >>>> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >>>> >>>> Set UART receive FIFO depth with PCD instead of fixed number "1". >>>> The default value of PCD is also 1, so it makes no difference for >>>> platforms which do not explicitly set this PCD. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>> Cc: Feng Tian <feng.tian@intel.com> >>>> Cc: Star Zeng <star.zeng@intel.com> >>>> --- >>>> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >>>> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >>>> 2 files changed, 7 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>> index 164060b..a1453bd 100644 >>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>> @@ -41,10 +41,11 @@ >>>> gEfiDevicePathProtocolGuid ## PRODUCES >>>> >>>> [Pcd] >>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >>>> >>>> [Depex] >>>> TRUE >>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>> index f5b3064..d2383e5 100644 >>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>> @@ -236,7 +236,7 @@ SerialReset ( >>>> // >>>> // Set the Serial I/O mode >>>> // >>>> - This->Mode->ReceiveFifoDepth = 1; >>>> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>> This->Mode->Timeout = 1000 * 1000; >>>> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >>>> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >>>> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >>>> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >>>> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >>>> -- >>>> 2.7.0 >>>> >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/23/16 17:26, Ryan Harkin wrote: > On 23 March 2016 at 16:24, Ryan Harkin <ryan.harkin@linaro.org> wrote: >> On 23 March 2016 at 08:53, Laszlo Ersek <lersek@redhat.com> wrote: >>> On 03/23/16 09:33, Ni, Ruiyu wrote: >>>> Laszlo, >>>> Since the patch below lets SerialDxe driver use the new introduced >>>> PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. >>>> Would you mind to revert the check in @ 31ae446b >>>> --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? >>> >>> If I understand correctly, this series increases the terminal polling >>> rate, so that cursor movement escape sequences (and other burst-like >>> sequences) can be processed even with a receive fifo depth of 1. Hence >>> 31ae446b should become unnecessary. >>> >>> If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test >>> your revert of 31ae446b -- on top of this series from Heyi --, and the >>> cursor keys work with 31ae446b reverted, then I don't mind, sure. >>> >> >> Unfortunately TC2 and Juno no longer with with 31ae446b reverted and >> these 3 patches from the series applied. > > I'm sure you would all work it out ok, but that should have been: > > "Unfortunately TC2 and Juno no longer work with 31ae446b reverted and > these 3 patches from the series applied." Thank you very much for the feedback. Ray, can you please work with Heyi, Ryan and others to analyze why this series of Heyi's doesn't supplant 31ae446b? And, until that is determined, could you please hold off on reverting 31ae446b? Thank you, Laszlo >> >> It's a shame, because it works on FVP models and copy/paste then works. >> >> I haven't made any investigations on what's going wrong. But regular >> ASCII keys work, control codes like cursor keys don't. So it looks >> like the old FIFO setting problem we discussed a few weeks ago. >> >> >>> Thanks >>> Laszlo >>> >>>> >>>> Regards, >>>> Ray >>>> >>>> >>>>> -----Original Message----- >>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >>>>> Sent: Thursday, March 17, 2016 10:37 PM >>>>> To: edk2-devel@lists.01.org >>>>> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >>>>> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >>>>> >>>>> Set UART receive FIFO depth with PCD instead of fixed number "1". >>>>> The default value of PCD is also 1, so it makes no difference for >>>>> platforms which do not explicitly set this PCD. >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>>> Cc: Feng Tian <feng.tian@intel.com> >>>>> Cc: Star Zeng <star.zeng@intel.com> >>>>> --- >>>>> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >>>>> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >>>>> 2 files changed, 7 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>> index 164060b..a1453bd 100644 >>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>> @@ -41,10 +41,11 @@ >>>>> gEfiDevicePathProtocolGuid ## PRODUCES >>>>> >>>>> [Pcd] >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >>>>> >>>>> [Depex] >>>>> TRUE >>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>> index f5b3064..d2383e5 100644 >>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>> @@ -236,7 +236,7 @@ SerialReset ( >>>>> // >>>>> // Set the Serial I/O mode >>>>> // >>>>> - This->Mode->ReceiveFifoDepth = 1; >>>>> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>> This->Mode->Timeout = 1000 * 1000; >>>>> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >>>>> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >>>>> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >>>>> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >>>>> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >>>>> -- >>>>> 2.7.0 >>>>> >>>>> _______________________________________________ >>>>> edk2-devel mailing list >>>>> edk2-devel@lists.01.org >>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>> >>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi folks, Please see my thoughts below: On 03/24/2016 12:26 AM, Ryan Harkin wrote: > On 23 March 2016 at 16:24, Ryan Harkin <ryan.harkin@linaro.org> wrote: >> On 23 March 2016 at 08:53, Laszlo Ersek <lersek@redhat.com> wrote: >>> On 03/23/16 09:33, Ni, Ruiyu wrote: >>>> Laszlo, >>>> Since the patch below lets SerialDxe driver use the new introduced >>>> PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. >>>> Would you mind to revert the check in @ 31ae446b >>>> --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? >>> If I understand correctly, this series increases the terminal polling >>> rate, so that cursor movement escape sequences (and other burst-like >>> sequences) can be processed even with a receive fifo depth of 1. Hence >>> 31ae446b should become unnecessary. >>> >>> If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test >>> your revert of 31ae446b -- on top of this series from Heyi --, and the >>> cursor keys work with 31ae446b reverted, then I don't mind, sure. >>> >> Unfortunately TC2 and Juno no longer with with 31ae446b reverted and >> these 3 patches from the series applied. I think my patches may not be sufficient to replace patch 31ae446b; they have some different results. The result of patch 31ae446b is absolute; it does not rely on timer event or code execution but only on UART hardware. My patches are actually complementary software solution for platforms which have certain length of FIFO depth, e.g. 40 characters continuous input to UART with 32 FIFO depth. I don't think it will work well on FIFO depth of 1, as it does not have enough scalability for software processing. This also the reason why I reduce the precisely calculated interval. It also depends on the timer interrupt period (should be gEmbeddedTokenSpaceGuid.PcdTimerPeriod on ARM platforms), which must be less than the calculated polling interval. So my opinion is to keep patch 31ae446b if it is reasonable on its own, and take my patches as further enhancement. Thanks. Heyi > I'm sure you would all work it out ok, but that should have been: > > "Unfortunately TC2 and Juno no longer work with 31ae446b reverted and > these 3 patches from the series applied." > > >> It's a shame, because it works on FVP models and copy/paste then works. >> >> I haven't made any investigations on what's going wrong. But regular >> ASCII keys work, control codes like cursor keys don't. So it looks >> like the old FIFO setting problem we discussed a few weeks ago. >> >> >>> Thanks >>> Laszlo >>> >>>> Regards, >>>> Ray >>>> >>>> >>>>> -----Original Message----- >>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >>>>> Sent: Thursday, March 17, 2016 10:37 PM >>>>> To: edk2-devel@lists.01.org >>>>> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >>>>> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >>>>> >>>>> Set UART receive FIFO depth with PCD instead of fixed number "1". >>>>> The default value of PCD is also 1, so it makes no difference for >>>>> platforms which do not explicitly set this PCD. >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>>> Cc: Feng Tian <feng.tian@intel.com> >>>>> Cc: Star Zeng <star.zeng@intel.com> >>>>> --- >>>>> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >>>>> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >>>>> 2 files changed, 7 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>> index 164060b..a1453bd 100644 >>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>> @@ -41,10 +41,11 @@ >>>>> gEfiDevicePathProtocolGuid ## PRODUCES >>>>> >>>>> [Pcd] >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >>>>> >>>>> [Depex] >>>>> TRUE >>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>> index f5b3064..d2383e5 100644 >>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>> @@ -236,7 +236,7 @@ SerialReset ( >>>>> // >>>>> // Set the Serial I/O mode >>>>> // >>>>> - This->Mode->ReceiveFifoDepth = 1; >>>>> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>> This->Mode->Timeout = 1000 * 1000; >>>>> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >>>>> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >>>>> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >>>>> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >>>>> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >>>>> -- >>>>> 2.7.0 >>>>> >>>>> _______________________________________________ >>>>> edk2-devel mailing list >>>>> edk2-devel@lists.01.org >>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/24/2016 10:47 AM, Ni, Ruiyu wrote: > > Regards, > Ray > > >> -----Original Message----- >> From: Heyi Guo [mailto:heyi.guo@linaro.org] >> Sent: Thursday, March 24, 2016 9:09 AM >> To: Ryan Harkin <ryan.harkin@linaro.org>; Laszlo Ersek <lersek@redhat.com> >> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; >> Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Subject: Re: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >> >> Hi folks, >> >> Please see my thoughts below: >> >> On 03/24/2016 12:26 AM, Ryan Harkin wrote: >>> On 23 March 2016 at 16:24, Ryan Harkin <ryan.harkin@linaro.org> wrote: >>>> On 23 March 2016 at 08:53, Laszlo Ersek <lersek@redhat.com> wrote: >>>>> On 03/23/16 09:33, Ni, Ruiyu wrote: >>>>>> Laszlo, >>>>>> Since the patch below lets SerialDxe driver use the new introduced >>>>>> PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. >>>>>> Would you mind to revert the check in @ 31ae446b >>>>>> --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? >>>>> If I understand correctly, this series increases the terminal polling >>>>> rate, so that cursor movement escape sequences (and other burst-like >>>>> sequences) can be processed even with a receive fifo depth of 1. Hence >>>>> 31ae446b should become unnecessary. >>>>> >>>>> If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test >>>>> your revert of 31ae446b -- on top of this series from Heyi --, and the >>>>> cursor keys work with 31ae446b reverted, then I don't mind, sure. >>>>> >>>> Unfortunately TC2 and Juno no longer with with 31ae446b reverted and >>>> these 3 patches from the series applied. >> I think my patches may not be sufficient to replace patch 31ae446b; they >> have some different results. >> >> The result of patch 31ae446b is absolute; it does not rely on timer >> event or code execution but only on UART hardware. >> >> My patches are actually complementary software solution for platforms >> which have certain length of FIFO depth, e.g. 40 characters continuous >> input to UART with 32 FIFO depth. I don't think it will work well on >> FIFO depth of 1, as it does not have enough scalability for software >> processing. This also the reason why I reduce the precisely calculated >> interval. >> >> It also depends on the timer interrupt period (should be >> gEmbeddedTokenSpaceGuid.PcdTimerPeriod on ARM platforms), which must be >> less than the calculated polling interval. >> >> So my opinion is to keep patch 31ae446b if it is reasonable on its own, >> and take my patches as further enhancement. > NO. > I understand the fact that the patch 31ae446b did fix an issue in certain platform. > But the patch lets TerminalDxe driver changes the underlying UART's Receive > FIFO depth. Such behavior is bad. > > Now since we have added the new PCD PcdDefaultUartReceiveFifoDepth, > we can fix the original problem by setting the PCD value to 16 from platform DSC. > > In all, I wanted to say, with the new PCD patch, the patch 31ae446b can be reverted > without functionality impact. Sorry I misunderstood your previous email. Does it make sense to set the default value of PCD to 0 directly? > > And I will still talk with Heyi to understand why 3/3 timer interval patch needs to > coded like that. Did you see my early mail to explain the calculation? Regards. Heyi >> Thanks. >> >> Heyi >> >> >>> I'm sure you would all work it out ok, but that should have been: >>> >>> "Unfortunately TC2 and Juno no longer work with 31ae446b reverted and >>> these 3 patches from the series applied." >>> >>> >>>> It's a shame, because it works on FVP models and copy/paste then works. >>>> >>>> I haven't made any investigations on what's going wrong. But regular >>>> ASCII keys work, control codes like cursor keys don't. So it looks >>>> like the old FIFO setting problem we discussed a few weeks ago. >>>> >>>> >>>>> Thanks >>>>> Laszlo >>>>> >>>>>> Regards, >>>>>> Ray >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >>>>>>> Sent: Thursday, March 17, 2016 10:37 PM >>>>>>> To: edk2-devel@lists.01.org >>>>>>> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >>>>>>> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >>>>>>> >>>>>>> Set UART receive FIFO depth with PCD instead of fixed number "1". >>>>>>> The default value of PCD is also 1, so it makes no difference for >>>>>>> platforms which do not explicitly set this PCD. >>>>>>> >>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>>>>> Cc: Feng Tian <feng.tian@intel.com> >>>>>>> Cc: Star Zeng <star.zeng@intel.com> >>>>>>> --- >>>>>>> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >>>>>>> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >>>>>>> 2 files changed, 7 insertions(+), 5 deletions(-) >>>>>>> >>>>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>>>> index 164060b..a1453bd 100644 >>>>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>>>> @@ -41,10 +41,11 @@ >>>>>>> gEfiDevicePathProtocolGuid ## PRODUCES >>>>>>> >>>>>>> [Pcd] >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >>>>>>> >>>>>>> [Depex] >>>>>>> TRUE >>>>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>>>> index f5b3064..d2383e5 100644 >>>>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>>>> @@ -236,7 +236,7 @@ SerialReset ( >>>>>>> // >>>>>>> // Set the Serial I/O mode >>>>>>> // >>>>>>> - This->Mode->ReceiveFifoDepth = 1; >>>>>>> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>>>> This->Mode->Timeout = 1000 * 1000; >>>>>>> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>>>> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>>>> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >>>>>>> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>>>> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >>>>>>> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >>>>>>> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>>>> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>>>> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >>>>>>> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >>>>>>> -- >>>>>>> 2.7.0 >>>>>>> >>>>>>> _______________________________________________ >>>>>>> edk2-devel mailing list >>>>>>> edk2-devel@lists.01.org >>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>>>> _______________________________________________ >>>>>> edk2-devel mailing list >>>>>> edk2-devel@lists.01.org >>>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>>>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/24/16 03:47, Ni, Ruiyu wrote: > > > Regards, > Ray > > >> -----Original Message----- >> From: Heyi Guo [mailto:heyi.guo@linaro.org] >> Sent: Thursday, March 24, 2016 9:09 AM >> To: Ryan Harkin <ryan.harkin@linaro.org>; Laszlo Ersek <lersek@redhat.com> >> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; >> Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Subject: Re: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >> >> Hi folks, >> >> Please see my thoughts below: >> >> On 03/24/2016 12:26 AM, Ryan Harkin wrote: >>> On 23 March 2016 at 16:24, Ryan Harkin <ryan.harkin@linaro.org> wrote: >>>> On 23 March 2016 at 08:53, Laszlo Ersek <lersek@redhat.com> wrote: >>>>> On 03/23/16 09:33, Ni, Ruiyu wrote: >>>>>> Laszlo, >>>>>> Since the patch below lets SerialDxe driver use the new introduced >>>>>> PCD gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth. >>>>>> Would you mind to revert the check in @ 31ae446b >>>>>> --> MdeModulePkg: TerminalDxe: select the UART's default receive FIFO depth? >>>>> If I understand correctly, this series increases the terminal polling >>>>> rate, so that cursor movement escape sequences (and other burst-like >>>>> sequences) can be processed even with a receive fifo depth of 1. Hence >>>>> 31ae446b should become unnecessary. >>>>> >>>>> If Ryan and Ard (CC'd), who tested 31ae446b originally, can also test >>>>> your revert of 31ae446b -- on top of this series from Heyi --, and the >>>>> cursor keys work with 31ae446b reverted, then I don't mind, sure. >>>>> >>>> Unfortunately TC2 and Juno no longer with with 31ae446b reverted and >>>> these 3 patches from the series applied. >> >> I think my patches may not be sufficient to replace patch 31ae446b; they >> have some different results. >> >> The result of patch 31ae446b is absolute; it does not rely on timer >> event or code execution but only on UART hardware. >> >> My patches are actually complementary software solution for platforms >> which have certain length of FIFO depth, e.g. 40 characters continuous >> input to UART with 32 FIFO depth. I don't think it will work well on >> FIFO depth of 1, as it does not have enough scalability for software >> processing. This also the reason why I reduce the precisely calculated >> interval. >> >> It also depends on the timer interrupt period (should be >> gEmbeddedTokenSpaceGuid.PcdTimerPeriod on ARM platforms), which must be >> less than the calculated polling interval. >> >> So my opinion is to keep patch 31ae446b if it is reasonable on its own, >> and take my patches as further enhancement. > > NO. > I understand the fact that the patch 31ae446b did fix an issue in certain platform. > But the patch lets TerminalDxe driver changes the underlying UART's Receive > FIFO depth. Such behavior is bad. > > Now since we have added the new PCD PcdDefaultUartReceiveFifoDepth, > we can fix the original problem by setting the PCD value to 16 from platform DSC. > > In all, I wanted to say, with the new PCD patch, the patch 31ae446b can be reverted > without functionality impact. Okay, I understand. However, in that case, the series that reverts 31ae446b should first provide DSC patches that set this new PCD for platforms that currently depend on 31ae446b, and revert 31ae446b second. In other words, the testing that Ryan did thus far wasn't correct -- simply because we didn't ask him to test the right thing. The right thing would be to (a) apply Heyi's patches, (b) set the new PCD in his platform DSC(s), (c) revert 31ae446b, (d) test the cursor keys. Unfortunately, I don't know what exact value Ryan should set in this new PCD. I can imagine it cannot even be set in a platform DSC flexibly enough, i.e., maybe it should be set dynamically in some PEIM. Thanks Laszlo > And I will still talk with Heyi to understand why 3/3 timer interval patch needs to > coded like that. > >> >> Thanks. >> >> Heyi >> >> >>> I'm sure you would all work it out ok, but that should have been: >>> >>> "Unfortunately TC2 and Juno no longer work with 31ae446b reverted and >>> these 3 patches from the series applied." >>> >>> >>>> It's a shame, because it works on FVP models and copy/paste then works. >>>> >>>> I haven't made any investigations on what's going wrong. But regular >>>> ASCII keys work, control codes like cursor keys don't. So it looks >>>> like the old FIFO setting problem we discussed a few weeks ago. >>>> >>>> >>>>> Thanks >>>>> Laszlo >>>>> >>>>>> Regards, >>>>>> Ray >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo >>>>>>> Sent: Thursday, March 17, 2016 10:37 PM >>>>>>> To: edk2-devel@lists.01.org >>>>>>> Cc: Heyi Guo <heyi.guo@linaro.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com> >>>>>>> Subject: [edk2] [PATCH v2 2/3] MdeModulePkg/SerialDxe: Set FIFO depth with PCD >>>>>>> >>>>>>> Set UART receive FIFO depth with PCD instead of fixed number "1". >>>>>>> The default value of PCD is also 1, so it makes no difference for >>>>>>> platforms which do not explicitly set this PCD. >>>>>>> >>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org> >>>>>>> Cc: Feng Tian <feng.tian@intel.com> >>>>>>> Cc: Star Zeng <star.zeng@intel.com> >>>>>>> --- >>>>>>> MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- >>>>>>> MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- >>>>>>> 2 files changed, 7 insertions(+), 5 deletions(-) >>>>>>> >>>>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>>>> index 164060b..a1453bd 100644 >>>>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf >>>>>>> @@ -41,10 +41,11 @@ >>>>>>> gEfiDevicePathProtocolGuid ## PRODUCES >>>>>>> >>>>>>> [Pcd] >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>>>> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES >>>>>>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES >>>>>>> >>>>>>> [Depex] >>>>>>> TRUE >>>>>>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>>>> index f5b3064..d2383e5 100644 >>>>>>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>>>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c >>>>>>> @@ -236,7 +236,7 @@ SerialReset ( >>>>>>> // >>>>>>> // Set the Serial I/O mode >>>>>>> // >>>>>>> - This->Mode->ReceiveFifoDepth = 1; >>>>>>> + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>>>> This->Mode->Timeout = 1000 * 1000; >>>>>>> This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>>>> This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>>>> @@ -508,6 +508,7 @@ SerialDxeInitialize ( >>>>>>> mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); >>>>>>> mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); >>>>>>> mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); >>>>>>> + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); >>>>>>> mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); >>>>>>> mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); >>>>>>> mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity); >>>>>>> -- >>>>>>> 2.7.0 >>>>>>> >>>>>>> _______________________________________________ >>>>>>> edk2-devel mailing list >>>>>>> edk2-devel@lists.01.org >>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>>>> _______________________________________________ >>>>>> edk2-devel mailing list >>>>>> edk2-devel@lists.01.org >>>>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>>>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf index 164060b..a1453bd 100644 --- a/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf +++ b/MdeModulePkg/Universal/SerialDxe/SerialDxe.inf @@ -41,10 +41,11 @@ gEfiDevicePathProtocolGuid ## PRODUCES [Pcd] - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth ## CONSUMES [Depex] TRUE diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c index f5b3064..d2383e5 100644 --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c @@ -236,7 +236,7 @@ SerialReset ( // // Set the Serial I/O mode // - This->Mode->ReceiveFifoDepth = 1; + This->Mode->ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); This->Mode->Timeout = 1000 * 1000; This->Mode->BaudRate = PcdGet64 (PcdUartDefaultBaudRate); This->Mode->DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); @@ -508,6 +508,7 @@ SerialDxeInitialize ( mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits); mSerialIoMode.Parity = (UINT32) PcdGet8 (PcdUartDefaultParity); mSerialIoMode.StopBits = (UINT32) PcdGet8 (PcdUartDefaultStopBits); + mSerialIoMode.ReceiveFifoDepth = PcdGet16 (PcdUartDefaultReceiveFifoDepth); mSerialDevicePath.Uart.BaudRate = PcdGet64 (PcdUartDefaultBaudRate); mSerialDevicePath.Uart.DataBits = PcdGet8 (PcdUartDefaultDataBits); mSerialDevicePath.Uart.Parity = PcdGet8 (PcdUartDefaultParity);
Set UART receive FIFO depth with PCD instead of fixed number "1". The default value of PCD is also 1, so it makes no difference for platforms which do not explicitly set this PCD. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Heyi Guo <heyi.guo@linaro.org> Cc: Feng Tian <feng.tian@intel.com> Cc: Star Zeng <star.zeng@intel.com> --- MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 9 +++++---- MdeModulePkg/Universal/SerialDxe/SerialIo.c | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) -- 2.7.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel