On Fri, Jan 15, 2016 at 06:05:03PM +0100, Laszlo Ersek wrote:
> snipping context liberally...
Snipping clear and very detailed (and useful) explanation.
> (4) My opinion:
>
> - The new driver's ReceiveFifoDepth setting, on init and on Reset, is
> correct. (Conforms to the spec.)
>
> - The Timeout defaults seem wrong however: 1 million usecs should be set.
>
> - Whatever *uses* the Serial IO Protocol directly, should explicitly
> set some ReceiveFifoDepth (with the SetAttributes() member
> function), such that the depth accommodates the longest escape
> sequence that the client wishes to handle. I believe this
> requirement falls on the TerminalDxe driver, doesn't it?
>
> In any case, the explicitly configured FIFO depth can be "zero",
> apparently, for selecting the device's "default" FIFO depth. (See
> the documentation of SetAttributes()).
Yes, that was why I agreed with that change in Ard's patch, but after
your explanation I agree that your proposed solution is the correct
location to change that.
Now, SetAttributes also lets you set default timeout by specifying 0,
but I don't think that would make much sense in this case- the
terminal input scan rate is a property of the terminal, not the
device.
> (BTW, the SetAttributes() spec also explains the "everything can be
> rounded down" comment from PL011UartInitializePort(): "The nearest
> FIFO size supported by the serial device will be selected without
> exceeding the ReceiveFifoDepth parameter.")
>
> Ryan, can you please test the following patch?
>
> > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> > b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> > index 6fde3b2..e1a6527 100644
> > --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> > @@ -806,7 +806,7 @@ TerminalDriverBindingStart (
> > Status = TerminalDevice->SerialIo->SetAttributes (
> > TerminalDevice->SerialIo,
> > Mode->BaudRate,
> > - Mode->ReceiveFifoDepth,
> > + 0, // device's default FIFO depth.
> > (UINT32) SerialInTimeOut,
> > (EFI_PARITY_TYPE) (Mode->Parity),
> > (UINT8) Mode->DataBits,
> > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> > b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> > index 3be877b..ffdc8f2 100644
> > --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> > @@ -547,7 +547,7 @@ TerminalConInTimerHandler (
> > Status = SerialIo->SetAttributes (
> > SerialIo,
> > Mode->BaudRate,
> > - Mode->ReceiveFifoDepth,
> > + 0, // device's default FIFO depth.
> > (UINT32) SerialInTimeOut,
> > (EFI_PARITY_TYPE) (Mode->Parity),
> > (UINT8) Mode->DataBits,
So I'm happy with the above fix.
Thanks!
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
