Re: [edk2-devel] [PATCH edk2-platforms v3 4/4] SbsaQemu: disable XHCI in DSDT if not present

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 18 Oct 2023 at 07:56, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> W dniu 18.10.2023 o 05:28, Nhi Pham pisze:
> > Hi Marcin,
> >
> > There is a nitpicking below.
> >
> > Other than, it looks good to me.
> >
> > Acked-by: Nhi Pham <nhi@os.amperecomputing.com>
> >
> >> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> >> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> >> index fd849ca1594b..cf6e534ca3a0 100644
> >> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> >> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> >> @@ -10,6 +10,7 @@
> >>   #include <IndustryStandard/AcpiAml.h>
> >>   #include <IndustryStandard/IoRemappingTable.h>
> >>   #include <IndustryStandard/SbsaQemuAcpi.h>
> >> +#include <IndustryStandard/SbsaQemuPlatformVersion.h>
> >>   #include <Library/AcpiLib.h>
> >>   #include <Library/ArmLib.h>
> >>   #include <Library/BaseMemoryLib.h>
> >> @@ -682,6 +683,63 @@ AddGtdtTable (
> >>     return Status;
> >>   }
> >> +EFI_STATUS
> >> +DisableXhciOnOlderPlatVer (
> >> +  VOID
> >> +  )
> >> +{
> >> +  EFI_STATUS            Status;
> >> +  EFI_ACPI_SDT_PROTOCOL                       *AcpiSdtProtocol;
> >> +  EFI_ACPI_DESCRIPTION_HEADER                 *Table;
> >> +  UINTN                                       TableKey;
> >> +  UINTN                                       TableIndex;
> >> +  EFI_ACPI_HANDLE                             TableHandle;
> >> +
> >> +  Status = EFI_SUCCESS;
> >> +
> >> +  if ( PLATFORM_VERSION_LESS_THAN(0, 3) ) {
> >> +    DEBUG ((DEBUG_ERROR, "Platform Version < 0.3 - disabling XHCI\n"));
> >> +    Status = gBS->LocateProtocol (
> >> +                    &gEfiAcpiSdtProtocolGuid,
> >> +                    NULL,
> >> +                    (VOID **)&AcpiSdtProtocol
> >> +                    );
> >> +    if (EFI_ERROR (Status)) {
> >> +      DEBUG ((DEBUG_ERROR, "Unable to locate ACPI table protocol\n"));
> >> +      return Status;
> >> +    }
> >> +
> >> +    TableIndex = 0;
> >> +    Status = AcpiLocateTableBySignature (
> >> +               AcpiSdtProtocol,
> >> +
> >> EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
> >> +               &TableIndex,
> >> +               &Table,
> >> +               &TableKey
> >> +               );
> >> +    if (EFI_ERROR (Status)) {
> >> +      DEBUG ((DEBUG_ERROR, "ACPI DSDT table not found!\n"));
> >> +      ASSERT_EFI_ERROR (Status);
> >> +      return Status;
> >> +    }
> >> +
> >> +    Status = AcpiSdtProtocol->OpenSdt (TableKey, &TableHandle);
> >> +    if (EFI_ERROR (Status)) {
> >> +      ASSERT_EFI_ERROR (Status);
> >> +      AcpiSdtProtocol->Close (TableHandle);
> >> +      return Status;
> >> +    }
> >> +
> >> +    AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle,
> >> "\\_SB.USB0.XHCI", 0x0);
> >> +
> >> +    AcpiSdtProtocol->Close (TableHandle);
> >> +    AcpiUpdateChecksum ((UINT8 *)Table, Table->Length);
> >> +  }
> >> +
> >> +  return Status;
> >> +}
> >> +
> >> +
> >>   EFI_STATUS
> >>   EFIAPI
> >>   InitializeSbsaQemuAcpiDxe (
> >> @@ -738,5 +796,7 @@ InitializeSbsaQemuAcpiDxe (
> >>       DEBUG ((DEBUG_ERROR, "Failed to add GTDT table\n"));
> >>     }
> >> +  Status = DisableXhciOnOlderPlatVer();
> >
> > Nit: EDK2 Coding Style says that you need a space before (.
>
> Ah, right. forgot to crucify the source.
>
> > Is it necessary to handle the result of Status?
>
> EDK2 is full of handling Status on touching ACPI tables. So I followed.
>

Can you just do 'return DisableXhciOnOlderPlatVer();' instead?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109715): https://edk2.groups.io/g/devel/message/109715
Mute This Topic: https://groups.io/mt/102017316/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-