[v2] Provide XHCI USB controller only for newer hardware

[edk2-devel] [PATCH edk2-platforms v2 0/4] Provide XHCI USB controller only for newer hardware

Posted by Marcin Juszkiewicz 6 months, 3 weeks ago

Platform version 0.3 introduced XHCI USB controller instead of EHCI one.
But we did it in a way that there is no in-EDK2 check for platform
version (XHCI is always given).

This behaviour works with Linux as it complains about being unable to
initialize EHCI and goes on. Free/Net/Open BSD systems hang in such
situation.

Now we check are we on Platform Version 0.3 at least and then initialize
XHCI controller. Otherwise we disable it's node in DSDT table.

I checked several ways to handle the situation. 

SSDT overlay enabling with LocateAndInstallAcpiFromFvConfitional() was
first one but this function had only DBG2 and FACP tables.

Then looked at trying to detect XHCI from _STA method. But this is
sysbus device which is there or not without way to discover it.

Next attempt was to have variable in DSDT and to use it in _STA. Too
much trouble.

Then looked again at code from
Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiDsdt.c and noticed
UpdateStatusMethodObject() function. Copied some code and it worked.

Booting OpenBSD reminded me to update table checksum.

---
Changes in v2:
- XHCI initialized only on PlatVer 0.3+
- XHCI disabled in DSDT for older platforms
- no SSDT overlays for EHCI/XHCI
- no EHCI at all (it does not work anyway)
- no Pcd renaming

---
Marcin Juszkiewicz (4):
      SbsaQemu: introduce macro to compare platform version
      SbsaQemu: add AcpiLib
      SbsaQemu: initialize XHCI only if it exists
      SbsaQemu: disable XHCI in DSDT if not present

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc                  |  2 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf      |  4 +
 .../IndustryStandard/SbsaQemuPlatformVersion.h       | 25 ++++++
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c        | 93 ++++++++++++++++++++
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c        | 47 +++++-----
 5 files changed, 149 insertions(+), 22 deletions(-)
---
base-commit: 06f6274d56422084281886fad447ab117fd0e487
change-id: 20231013-ehci-xhci-fix-c529356a7a8f

Best regards,
-- 
Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109625): https://edk2.groups.io/g/devel/message/109625
Mute This Topic: https://groups.io/mt/101992601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 0/4] Provide XHCI USB controller only for newer hardware

Posted by Nhi Pham via groups.io 6 months, 2 weeks ago

Hi Marcin,

On 10/16/2023 5:06 PM, Marcin Juszkiewicz wrote:
> Platform version 0.3 introduced XHCI USB controller instead of EHCI one.
> But we did it in a way that there is no in-EDK2 check for platform
> version (XHCI is always given).
> 
> This behaviour works with Linux as it complains about being unable to
> initialize EHCI and goes on. Free/Net/Open BSD systems hang in such
> situation.
> 
> Now we check are we on Platform Version 0.3 at least and then initialize
> XHCI controller. Otherwise we disable it's node in DSDT table.
> 
> I checked several ways to handle the situation.
> 
> SSDT overlay enabling with LocateAndInstallAcpiFromFvConfitional() was
> first one but this function had only DBG2 and FACP tables.
> 
> Then looked at trying to detect XHCI from _STA method. But this is
> sysbus device which is there or not without way to discover it.
> 
> Next attempt was to have variable in DSDT and to use it in _STA. Too
> much trouble.
> 
> Then looked again at code from
> Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiDsdt.c and noticed
> UpdateStatusMethodObject() function. Copied some code and it worked.

Could you please check whether we can replace the 
UpdateStatusMethodObject() function with the 
AcpiAmlObjectUpdateInteger() function provided by AcpiLib?

Regards,
Nhi
> 
> Booting OpenBSD reminded me to update table checksum.
> 
> ---
> Changes in v2:
> - XHCI initialized only on PlatVer 0.3+
> - XHCI disabled in DSDT for older platforms
> - no SSDT overlays for EHCI/XHCI
> - no EHCI at all (it does not work anyway)
> - no Pcd renaming
> 
> ---
> Marcin Juszkiewicz (4):
>        SbsaQemu: introduce macro to compare platform version
>        SbsaQemu: add AcpiLib
>        SbsaQemu: initialize XHCI only if it exists
>        SbsaQemu: disable XHCI in DSDT if not present
> 
>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc                  |  2 +
>   .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf      |  4 +
>   .../IndustryStandard/SbsaQemuPlatformVersion.h       | 25 ++++++
>   .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c        | 93 ++++++++++++++++++++
>   .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c        | 47 +++++-----
>   5 files changed, 149 insertions(+), 22 deletions(-)
> ---
> base-commit: 06f6274d56422084281886fad447ab117fd0e487
> change-id: 20231013-ehci-xhci-fix-c529356a7a8f
> 
> Best regards,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109675): https://edk2.groups.io/g/devel/message/109675
Mute This Topic: https://groups.io/mt/101992601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 0/4] Provide XHCI USB controller only for newer hardware

Posted by Marcin Juszkiewicz 6 months, 2 weeks ago

W dniu 17.10.2023 o 13:46, Nhi Pham pisze:
>>
>> Then looked again at code from
>> Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiDsdt.c and noticed
>> UpdateStatusMethodObject() function. Copied some code and it worked.
> 
> Could you please check whether we can replace the 
> UpdateStatusMethodObject() function with the 
> AcpiAmlObjectUpdateInteger() function provided by AcpiLib?

AcpiAmlObjectUpdateInteger() finds \\_SB.USB0._STA, checks Buffer[0] and
quits because it is AML_METHOD_OP while it only works for AML_NAME_OP.

So I went a way I had in meantime. Added AML variable into DSDT and then
have _STA method return it's value (as I set it to either 0xF or 0x0).

DSDT.asl snippet:
----------------------------------------------------------------------
     // USB XHCI Host Controller
     Device (USB0) {
         Name (_HID, "PNP0D10")      // _HID: Hardware ID
         Name (_UID, 0x00)            // _UID: Unique ID
         Name (_CCA, 0x01)            // _CCA: Cache Coherency Attribute
         Name (XHCI, 0xF)            // will be set using AcpiLib
         Method (_STA) {
           Return (XHCI)
         }
----------------------------------------------------------------------

And then:

     // UpdateStatusMethodObject (AcpiSdtProtocol, TableHandle, "\\_SB.USB0._STA", 0x0);
     AcpiAmlObjectUpdateInteger (AcpiSdtProtocol, TableHandle, "\\_SB.USB0.XHCI", 0x0);


With this change code works as expected. Thanks!

Will send v3.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109680): https://edk2.groups.io/g/devel/message/109680
Mute This Topic: https://groups.io/mt/101992601/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-