public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: Heyi Guo <heyi.guo@linaro.org>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Ni, Ruiyu" <ruiyu.ni@intel.com>,
	"Laszlo Ersek (lersek@redhat.com)" <lersek@redhat.com>
Cc: "Zeng, Star" <star.zeng@intel.com>
Subject: Re: MdeModulePkg/SerialDxe: Inconsistent timeout processing in SerialRead
Date: Tue, 18 Jul 2017 08:39:03 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B8F41C6@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <c5202325-c665-7951-f426-727a13d8b161@linaro.org>

Gary,
SerialDxe is a wrapper of SerialPortLib to produce serial io protocol.
If let SerialDxe to handle the TimeOut, seemingly it can only check SerialPortPoll() and read data by SerialPortRead() one byte by one byte, for example like below.
Do you have any proposed patch?

Ray and Laszlo,
Do you have any comments?


====================
debf0d1b85e7a30defd29838abb20a44dd9ec69b
 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..b05603d7f3b5 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);
+      TimeOut += 10;
+    }
+    if (TimeOut >= mSerialIoMode.TimeOut) {
+      break;
+    }
+    SerialPortRead (Buffer, 1);
+    Count++;
+    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
   }
 
   if (Count != *BufferSize) {
====================


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo
Sent: Monday, July 17, 2017 12:23 PM
To: edk2-devel@lists.01.org
Subject: [edk2] MdeModulePkg/SerialDxe: Inconsistent timeout processing in SerialRead

Hi Folks,

In SerialRead function in MdeModulePkg/Universal/SerialDxe/SerialIo.c,
it seems 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.

As SerialPortLib is a simple library implementation, I think it is better to improve SerialIoDxe driver instead of SerialPortLib.

Please let me know your comments about this.

Thanks and regards,

Gary (Heyi Guo)

EFI_STATUS
EFIAPI
SerialRead (
   IN EFI_SERIAL_IO_PROTOCOL *This,
   IN OUT UINTN              *BufferSize,
   OUT VOID                  *Buffer
   )
{
   UINTN Count;

   Count = 0;

   if (SerialPortPoll ()) {
     Count = SerialPortRead (Buffer, *BufferSize);
   }

   if (Count != *BufferSize) {
     *BufferSize = Count;
     return EFI_TIMEOUT;
   }

   return EFI_SUCCESS;
}

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


  parent reply	other threads:[~2017-07-18  8:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-17  4:23 MdeModulePkg/SerialDxe: Inconsistent timeout processing in SerialRead Heyi Guo
2017-07-18  8:13 ` Heyi Guo
2017-07-18  8:39 ` Zeng, Star [this message]
2017-07-18  8:58   ` Ni, Ruiyu
2017-07-18 10:59   ` Heyi Guo
2017-08-04  8:02     ` Heyi Guo
2017-08-04  8:05       ` Zeng, Star
2017-08-04  8:08         ` Heyi Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103B8F41C6@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox