I thought unit of Stall is 100ns. Then no issues now. Reviewed-by: Ruiyu Ni <[email protected]>
Thanks/Ray > -----Original Message----- > From: Zeng, Star > Sent: Friday, August 4, 2017 5:25 PM > To: Ni, Ruiyu <[email protected]>; [email protected] > Cc: Heyi Guo <[email protected]>; Laszlo Ersek <[email protected]>; > Zeng, Star <[email protected]> > Subject: RE: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently > in SerialRead > > Thanks for the comments. > > EFI_TIMER_PERIOD_MICROSECONDS is used for timer event according to its > definition, and its unit is 100ns. > But the unit of mSerialIoMode.Timeout and gBS->Stall() is 1us. > > +1 may cause more polling of SerialPortPoll(). How about keeping using > ++10? :) > > > Thanks, > Star > -----Original Message----- > From: Ni, Ruiyu > Sent: Friday, August 4, 2017 5:13 PM > To: Zeng, Star <[email protected]>; [email protected] > Cc: Heyi Guo <[email protected]>; Laszlo Ersek <[email protected]> > Subject: RE: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently > in SerialRead > > Star, > 3 minor comments below. > > Thanks/Ray > > > -----Original Message----- > > From: Zeng, Star > > Sent: Friday, August 4, 2017 4:29 PM > > To: [email protected] > > Cc: Zeng, Star <[email protected]>; Heyi Guo <[email protected]>; > > Ni, Ruiyu <[email protected]>; Laszlo Ersek <[email protected]> > > Subject: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently > > in SerialRead > > > > https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html > > reported the timeout processing in SerialRead is not consistent. > > > > Since SerialPortPoll only checks the status of serial port and returns > > immediately, and SerialPortRead does not really implement a time out > > mechanism and will always wait for enough input, it will cause below > > results: > > 1. If there is no serial input at all, this interface will return > > timeout immediately without any waiting; 2. If there is A characters > > in serial port FIFO, and caller requires > > A+1 characters, it will wait until a new input is coming and timeout > > will not really occur. > > > > This patch is to update SerialRead() to check SerialPortPoll() and > > read data through SerialPortRead() one byte by one byte, and check > > timeout against mSerialIoMode.Timeout if no input. > > > > Cc: Heyi Guo <[email protected]> > > Cc: Ruiyu Ni <[email protected]> > > Cc: Laszlo Ersek <[email protected]> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Star Zeng <[email protected]> > > --- > > MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c > > b/MdeModulePkg/Universal/SerialDxe/SerialIo.c > > index d2383e56dd8f..43d33dba0c2a 100644 > > --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c > > +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c > > @@ -465,11 +465,25 @@ SerialRead ( > > ) > > { > > UINTN Count; > > + UINTN TimeOut; > > > > Count = 0; > > > > - if (SerialPortPoll ()) { > > - Count = SerialPortRead (Buffer, *BufferSize); > > + while (Count < *BufferSize) { > > + TimeOut = 0; > > + while (TimeOut < mSerialIoMode.Timeout) { > > + if (SerialPortPoll ()) { > > + break; > > + } > > + gBS->Stall (10); > 1. can you use EFI_TIMER_PERIOD_MICROSECONDS(1)? > > > + TimeOut += 10; > 2. TImeOut++? > > > + } > > + if (TimeOut >= mSerialIoMode.Timeout) { > 3. if (TimeOut == ...) { ? > > > + break; > > + } > > + SerialPortRead (Buffer, 1); > > + Count++; > > + Buffer = (VOID *) ((UINT8 *) Buffer + 1); > > } > > > > if (Count != *BufferSize) { > > -- > > 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
