Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] ArmPlatformPkg/PL011UartLib : Fix SetControl() SCT conformance

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Samer,

Please find my response marked [SAMI].

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer El-Haj-Mahmoud via groups.io
Sent: 05 January 2021 08:47 PM
To: devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>
Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] ArmPlatformPkg/PL011UartLib : Fix SetControl() SCT conformance

Gentle reminder

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer
> El-Haj-Mahmoud via groups.io
> Sent: Sunday, December 20, 2020 2:23 PM
> To: devel@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel
> <Ard.Biesheuvel@arm.com>
> Subject: [edk2-devel] [edk2-platform][PATCH v1 1/1]
> ArmPlatformPkg/PL011UartLib : Fix SetControl() SCT conformance
>
> REF: https://github.com/pftf/RPi4/issues/87
>
> The PL011UartLib SetControl() is failing the SCT test for
> SerialIoBBTestConformance (00605CBC-3965-4B61-A254-2B2B723172EA),
> which is trying to set bits that are not supported per UEFI spec.
>
> Add proper argument check for valid bits, and confirm that test passes.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> ---
>  ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 15
> +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> index 3c58a0f39acb..ec6056f5fdfd 100644
> --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> @@ -269,6 +269,21 @@ PL011UartSetControl (
>  {
>
>    UINT32  Bits;
>
>
>
> +
>
> +  //
>
> +  // Per UEFI spec, the control bits that can be set are :
>
> +  //     EFI_SERIAL_DATA_TERMINAL_READY
>
> +  //     EFI_SERIAL_REQUEST_TO_SEND
>
> +  //     EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE
>
> +  //     EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE
>
> +  //     EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE
>
> +  //
>
> +  if ((Control & (~(EFI_SERIAL_REQUEST_TO_SEND |
> EFI_SERIAL_DATA_TERMINAL_READY |
>
> +                    EFI_SERIAL_HARDWARE_LOOPBACK_ENABLE |
> EFI_SERIAL_SOFTWARE_LOOPBACK_ENABLE |
>
> +                    EFI_SERIAL_HARDWARE_FLOW_CONTROL_ENABLE))) != 0) {
>
> +    return EFI_UNSUPPORTED;
>
> +  }
>
> +
[SAMI] mInvalidControlBits appears to be filtering the software loopback enable control as it is not supported.
Would it be possible to initialise mInvalidControlBits with the control bits above instead, please?
[/SAMI]
>
>    if ((Control & mInvalidControlBits) != 0) {
>
>      return RETURN_UNSUPPORTED;
>
>    }
>
> --
> 2.25.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#69271): https://edk2.groups.io/g/devel/message/69271
> Mute This Topic: https://groups.io/mt/79110273/1945644
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
> mahmoud@arm.com]
> -=-=-=-=-=-=
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.