Hi Laszlo,
On 18/10/17 13:11, Laszlo Ersek wrote:
On 10/18/17 12:19, Julien Grall wrote:
After commit 91cc526b15 "MdeModulePkg/SerialDxe: Fix not able to change
serial attributes", serial is initialized using the reset method that
will call SetAttributes.
However, SetAttributes may not be supported by the driver and will
return an error (i.e RETURN_UNSUPPORTED) that will be propagate and lead
to UEFI failing to get the console setup.
For instance, this is the case when using the Xen console driver.
Fix it by instropecting the result and return RETURN_SUCCESS when the
driver report it is not supported (i.e RETURN_UNSUPPORTED).
Contributed-under: Tianocore Contribution Agreement 1.1
Signed-off-by: Julien Grall <julien.gr...@linaro.org>
---
MdeModulePkg/Universal/SerialDxe/SerialIo.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index ebcd927263..4253e0b8ea 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -238,6 +238,12 @@ SerialReset (
(UINT8) This->Mode->DataBits,
(EFI_STOP_BITS_TYPE) This->Mode->StopBits
);
+ //
+ // The serial device may not support SetAttributes.
+ // Set the status to RETURN_SUCCESS to prevent later failure.
+ //
+ if ( Status == RETURN_UNSUPPORTED )
The extra spaces within the parens are Xen coding style, not edk2 coding
style; please remove them.
Sorry for that, I will fix the coding style in the next version.
[...]
I suggest waiting for feedback from Star & Eric. Dependent on their
response, your patch could be good enough (once you fix up the coding
style issues). Or else, they could agree with me that the return value
mapping of SerialSetAttributes() should be corrected first (2), and then
your patch should be please adapted as well (3).
Thank you for the detailed explanation. I will wait a couple of days
more for feedbacks and then send a new version.
Bonus comment:
(4) The propagation of the SerialPortInitialize() retval in
SerialReset() looks correct, thankfully. (Both callee and caller are
expected to return one of *_SUCCESS and *_DEVICE_ERROR.)
Cheers,
--
Julien Grall
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
