-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MdePkg, MdeModulePkg, ShellPkg: Drop efi 1.10 usb hc protocol #10761
base: master
Are you sure you want to change the base?
MdePkg, MdeModulePkg, ShellPkg: Drop efi 1.10 usb hc protocol #10761
Conversation
EFI_USB_HC_PROTOCOL was never defined in a released UEFI specification, so drop use of it in UsbBusDxe in preparation of deleting the definitions based on something allegedly supported in confidential EFI 1.10. Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
Protocol/UsbHostController.h described an ancient protocol never part of a public specification ... and was in fact not needed. So drop the reference. Also drop some text from a function documentation header incorrectly implying use of the UsbHcProtocol. Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
EFI_USB_HC_PROTOCOL was never defined in a released UEFI specification, so drop use of it in ShellPkg in preparation of deleting the definitions based on something allegedly supported in confidential EFI 1.10. Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
It seems the platform |
I did point this out in the PR description. What I'm more interested in is whether there are tons of non-public platforms using this not-even-legacy protocol. |
On that note, do the other QuarkPlatformPkg maintainers have any comments? |
To my knowledge, most x86 SoCs expose their USB host controllers as PCIe Root Complex Integrated Endpoints that implement the XHCI standard. For that reason, the standard XhciDxe from MdeModulePkg works out-of-box without any SoC specific programming. As far as SoCs that need a bespoke USB host controller driver, that seems to be more common in the embedded space to me. Quark is an interesting exception as it is an x86 SoC designed for the embedded space. I think you will find more of these type of drivers in the ARM ecosystem. |
AFAIK, the first ever ARM prototype to run EDK2 was the BeagleBoard, which implements a EHCI high speed controller that relies on transaction translation to support FS/LS support, and so it does not rely on UhciDxe, only on EhciDxe. I think it is safe to say that we can drop support for EFI_USB_HC_PROTOCOL once we're willing to retire QuarkSoc from edk2-platforms. |
The Quark platform can be dropped from edk2-platforms. Please move forward with these changes. |
*MaxSpeed = EFI_USB_SPEED_FULL; | ||
*Is64BitCapable = (UINT8)FALSE; | ||
if (UsbBus->Usb2Hc == NULL) { | ||
return EFI_INVALID_PARAMETER; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop these NULL checks? This code should never be reached if the OpenProtocol() call that populated the Usb2Hc field failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, good point. It wasn't a "has this been initialized" test, it was a "is this protocol version 1 or 2" test. Will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(will do this when back from holiday on Monday)
Description
This PR deletes the EFI_USB_HC_PROTOCOL, as discussed in https://edk2.groups.io/g/devel/message/121115.
As stated there, EFI_USB_HC_PROTOCOL existed in the pre-public EFI 1.10 specification,
but never made it into a UEFI release.
This is the sledgehammer approach - if it turns out there are real-life users of the
protocol, we may need to do something with a bit more finesse. Or this may be an
opportunity to nudge them to rewrite code to the protocol covered by a specification.
and deletes code accommodating it in UsbBusDxe, UhciDxe, and ShellPkg.
in edk2-platforms.
How This Was Tested
Build tested only.
This needs to be tested on out-of-tree platforms, so if the approach is deemed acceptable, I would request merging shortly after stable tag is made. As always, reverts are cheap, but I
think getting data on whether the interface is used at all would be valuable.
Integration Instructions
N/A