| Message ID | 20171129172823.2906-3-julien.grall@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not supported | expand |
On 11/29/17 18:28, Julien Grall wrote: > SerialSetAttributes is meant to match the behavior of the function > EFI_SERIAL_IO_PROTOCOL.SetAttributes() in the UEFI spec (v2.7). This > means the function can only return: > - EFI_SUCCESS > - EFI_INVALID_PARAMETER > - EFI_DEVICE_ERROR > > However the function SerialPortSetAttributes may also validly return > EFI_UNSUPPORTED. For instance this is the case of the Xen Console > driver. > > EFI_UNSUPPORTED could be also interpreted as "One or more of the attributes > has an unsupported value". So return EFI_INVALID_PARAMETER in that case. > > Lastly, to prevent another return slipping in the future, all the errors > but EFI_INVALID_PARAMETERR and EFI_UNSUPPORTED will return "EFI_INVALID_PARAMETERR" has a typo here. Other than that, series Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > EFI_DEVICE_ERROR. > > Contributed-under: Tianocore Contribution Agreement 1.1 > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Reviewed-by: Star Zeng <star.zeng@intel.com> > > --- > > Star, I found a prototype for SetAttributes in SerialIo.c as well > and updated the description there. I decided to keep the Reviewed-by > even with that change. Let me know whether it is fine for you. > > Changes in v3: > - Add description of EFI_INVALID_PARAMETER above the prototypes > for SetAttributes as well. > - Add Star reviewed-by > --- > MdeModulePkg/Universal/SerialDxe/SerialIo.c | 14 +++++++++----- > MdePkg/Include/Protocol/SerialIo.h | 5 +++-- > 2 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c > index 19fc889c25..ee10ec7e05 100644 > --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c > +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c > @@ -66,8 +66,9 @@ SerialReset ( > value of DefaultStopBits will use the device's default number of > stop bits. > > - @retval EFI_SUCCESS The device was reset. > - @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. > + @retval EFI_SUCCESS The device was reset. > + @retval EFI_INVALID_PARAMETER One or more attributes has an unsupported value. > + @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. > > **/ > EFI_STATUS > @@ -264,8 +265,9 @@ SerialReset ( > value of DefaultStopBits will use the device's default number of > stop bits. > > - @retval EFI_SUCCESS The device was reset. > - @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. > + @retval EFI_SUCCESS The device was reset. > + @retval EFI_INVALID_PARAMETER One or more attributes has an unsupported value. > + @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. > > **/ > EFI_STATUS > @@ -323,8 +325,10 @@ SerialSetAttributes ( > DataBits = OriginalDataBits; > StopBits = OriginalStopBits; > Status = EFI_SUCCESS; > + } else if (Status == EFI_INVALID_PARAMETER || Status == EFI_UNSUPPORTED) { > + return EFI_INVALID_PARAMETER; > } else { > - return Status; > + return EFI_DEVICE_ERROR; > } > } > > diff --git a/MdePkg/Include/Protocol/SerialIo.h b/MdePkg/Include/Protocol/SerialIo.h > index 84cb34364d..1263dc4fe9 100644 > --- a/MdePkg/Include/Protocol/SerialIo.h > +++ b/MdePkg/Include/Protocol/SerialIo.h > @@ -125,8 +125,9 @@ EFI_STATUS > value of DefaultStopBits will use the device's default number of > stop bits. > > - @retval EFI_SUCCESS The device was reset. > - @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. > + @retval EFI_SUCCESS The device was reset. > + @retval EFI_INVALID_PARAMETER One or more attributes has an unsupported value. > + @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. > > **/ > typedef > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c index 19fc889c25..ee10ec7e05 100644 --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c @@ -66,8 +66,9 @@ SerialReset ( value of DefaultStopBits will use the device's default number of stop bits. - @retval EFI_SUCCESS The device was reset. - @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. + @retval EFI_SUCCESS The device was reset. + @retval EFI_INVALID_PARAMETER One or more attributes has an unsupported value. + @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. **/ EFI_STATUS @@ -264,8 +265,9 @@ SerialReset ( value of DefaultStopBits will use the device's default number of stop bits. - @retval EFI_SUCCESS The device was reset. - @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. + @retval EFI_SUCCESS The device was reset. + @retval EFI_INVALID_PARAMETER One or more attributes has an unsupported value. + @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. **/ EFI_STATUS @@ -323,8 +325,10 @@ SerialSetAttributes ( DataBits = OriginalDataBits; StopBits = OriginalStopBits; Status = EFI_SUCCESS; + } else if (Status == EFI_INVALID_PARAMETER || Status == EFI_UNSUPPORTED) { + return EFI_INVALID_PARAMETER; } else { - return Status; + return EFI_DEVICE_ERROR; } } diff --git a/MdePkg/Include/Protocol/SerialIo.h b/MdePkg/Include/Protocol/SerialIo.h index 84cb34364d..1263dc4fe9 100644 --- a/MdePkg/Include/Protocol/SerialIo.h +++ b/MdePkg/Include/Protocol/SerialIo.h @@ -125,8 +125,9 @@ EFI_STATUS value of DefaultStopBits will use the device's default number of stop bits. - @retval EFI_SUCCESS The device was reset. - @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. + @retval EFI_SUCCESS The device was reset. + @retval EFI_INVALID_PARAMETER One or more attributes has an unsupported value. + @retval EFI_DEVICE_ERROR The serial device is not functioning correctly. **/ typedef
SerialSetAttributes is meant to match the behavior of the function EFI_SERIAL_IO_PROTOCOL.SetAttributes() in the UEFI spec (v2.7). This means the function can only return: - EFI_SUCCESS - EFI_INVALID_PARAMETER - EFI_DEVICE_ERROR However the function SerialPortSetAttributes may also validly return EFI_UNSUPPORTED. For instance this is the case of the Xen Console driver. EFI_UNSUPPORTED could be also interpreted as "One or more of the attributes has an unsupported value". So return EFI_INVALID_PARAMETER in that case. Lastly, to prevent another return slipping in the future, all the errors but EFI_INVALID_PARAMETERR and EFI_UNSUPPORTED will return EFI_DEVICE_ERROR. Contributed-under: Tianocore Contribution Agreement 1.1 Signed-off-by: Julien Grall <julien.grall@linaro.org> Reviewed-by: Star Zeng <star.zeng@intel.com> --- Star, I found a prototype for SetAttributes in SerialIo.c as well and updated the description there. I decided to keep the Reviewed-by even with that change. Let me know whether it is fine for you. Changes in v3: - Add description of EFI_INVALID_PARAMETER above the prototypes for SetAttributes as well. - Add Star reviewed-by --- MdeModulePkg/Universal/SerialDxe/SerialIo.c | 14 +++++++++----- MdePkg/Include/Protocol/SerialIo.h | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel