Re: MdeModulePkg/SerialDxe: Inconsistent timeout processing in SerialRead

[Heyi Guo <heyi.guo@linaro.org>]

Hi Star,

The patch works well on our platform. Would you merge it to EDK2 main 
stream or do you like me to post it to the mailing-list?

Thanks and regards,

Gary (Heyi Guo)


在 7/18/2017 6:59 PM, Heyi Guo 写道:
> 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
>