From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-x22e.google.com (mail-pg0-x22e.google.com [IPv6:2607:f8b0:400e:c05::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B7DC721CB0318 for ; Tue, 18 Jul 2017 03:57:54 -0700 (PDT) Received: by mail-pg0-x22e.google.com with SMTP id u5so10734592pgq.3 for ; Tue, 18 Jul 2017 03:59:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding; bh=vgZWimBQ9qW9mATJEVuie+I1IpuncUyMy/tEY8oxRY0=; b=OkahB1jvKWNPbkDKLx2d1lGcAJHmfO/pDU8mn9qVeD7M9q6p9rEX/jlFnv1Dzsoeyh CqQW5kVxtwyuKR+FajYp1DQeA+/OrpKP282xNJUdPmNmoKhYJGaVz19BiJzI6duDrSfr zFSakV5oeJj+/cOBUxpH4I1g0uUQct+YjteQ4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=vgZWimBQ9qW9mATJEVuie+I1IpuncUyMy/tEY8oxRY0=; b=ORbhkYwpsXvzcdAwtHi4jJgr7q2+m0vX0fvM2AGQ4EsHVKlg1y7GiZi5E2SDHHa1Pk zzUQCYfk44Rr6K6f7M2RcU8Md73mnmOIRC+othhNk0XdN5MaJ2m9dlDy/MaCRDauv81A 8O4wmgWF8XMypnHpUSJ//qtPTmic3ccJfoyJe9jnkvJrnv2HUDTeiARsCZ0qjpaIoymn LwsFMQrN1/82GWSWvFPXoAP7SuI8Apt4A72RagpERUWmUpMsDGXFuTfHfMfQd7u2Xfl7 ojfRSUicu3osCGHfbZEDmvTPWU3wJgJrfAmu7cucgzArCbOW5ukraGQq+h1fJORfu2jN nP9w== X-Gm-Message-State: AIVw112KFo6Zlf4DELqM/05p2EEWJ/j5QQjX+Ue+frkYfmNa9MjjZHaj y1oN+K4SoFZH6pysTs3www== X-Received: by 10.101.86.8 with SMTP id l8mr959909pgs.271.1500375588165; Tue, 18 Jul 2017 03:59:48 -0700 (PDT) Received: from [10.229.36.134] ([119.145.15.121]) by smtp.gmail.com with ESMTPSA id o1sm3825093pgq.10.2017.07.18.03.59.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 18 Jul 2017 03:59:47 -0700 (PDT) To: "Zeng, Star" , "edk2-devel@lists.01.org" , "Ni, Ruiyu" , "Laszlo Ersek (lersek@redhat.com)" References: <0C09AFA07DD0434D9E2A0C6AEB0483103B8F41C6@shsmsx102.ccr.corp.intel.com> From: Heyi Guo Message-ID: <65b2b345-941e-710e-60f6-65b341c750e6@linaro.org> Date: Tue, 18 Jul 2017 18:59:44 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B8F41C6@shsmsx102.ccr.corp.intel.com> Subject: Re: MdeModulePkg/SerialDxe: Inconsistent timeout processing in SerialRead X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Jul 2017 10:57:54 -0000 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Star, I think your proposed patch is fine. Thanks. Gary (Heyi Guo) 在 7/18/2017 4:39 PM, Zeng, Star 写道: > 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