From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"Rhodes, Sean" <sean@starlabs.systems>,
"Gao, Liming" <gaoliming@byosoft.com.cn>
Cc: "Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the MaxPacketLength to real world values
Date: Wed, 7 Dec 2022 02:23:41 +0000 [thread overview]
Message-ID: <DM6PR11MB4025FDA4205FA0FD11086CA7CA1A9@DM6PR11MB4025.namprd11.prod.outlook.com> (raw)
In-Reply-To: <04f0fa8e36a7ff57ed6ab14ebe284bce35680262.1670231907.git.sean@starlabs.systems>
[-- Attachment #1: Type: text/plain, Size: 3706 bytes --]
I have concern for this patch.
It will make the implementation of XhcControlTransfer() no longer following the UEFI specification requirements on the EFI_USB2_HC_PROTOCOL.ControlTransfer() service:
Section 17.1.7 of UEFI spec Release 2.10:
EFI_INVALID_PARAMETER is returned if one of the following conditions is satisfied:
...
* MaximumPacketLength is not valid. If DeviceSpeed is EFI_USB_SPEED_LOW, then MaximumPacketLength
must be 8. If DeviceSpeed is EFI_USB_SPEED_FULL or EFI_USB_SPEED_HIGH, then MaximumPac-
ketLength must be 8, 16, 32, or 64. If DeviceSpeed is EFI_USB_SPEED_SUPER, then MaximumPacketLength
must be 512.
My take is that it will need a UEFI spec update to integrate this proposed change.
Sorry Liming, could you help on the process of raising opens with regard to the UEFI specification? Thanks in advance.
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Rhodes
> Sent: Monday, December 5, 2022 5:18 PM
> To: devel@edk2.groups.io
> Cc: Rhodes, Sean <sean@starlabs.systems>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the
> MaxPacketLength to real world values
>
> Adjusts the requirements for the MaxPacketLength to match what is seen on
> real world devices that do not follow the USB specification.
>
> This fixes enumeration on the multiple USB 3 devices made by SanDisk,
> Integral, Kingston and other generic brands.
>
> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Signed-off-by: Sean Rhodes <sean@starlabs.systems<mailto:sean@starlabs.systems>>
> ---
> MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 62535cad54..043b7d4cea 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -906,19 +906,16 @@ XhcControlTransfer (
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((MaximumPacketLength != 8) && (MaximumPacketLength != 16) &&
>
> - (MaximumPacketLength != 32) && (MaximumPacketLength != 64) &&
>
> - (MaximumPacketLength != 512)
>
> - )
>
> - {
>
> + // Check for valid maximum packet size
>
> + if ((DeviceSpeed == EFI_USB_SPEED_SUPER) && (MaximumPacketLength >
> 1024)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((DeviceSpeed == EFI_USB_SPEED_LOW) && (MaximumPacketLength !=
> 8)) {
>
> + if ((DeviceSpeed == EFI_USB_SPEED_HIGH) && (MaximumPacketLength >
> 512)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((DeviceSpeed == EFI_USB_SPEED_SUPER) && (MaximumPacketLength !=
> 512)) {
>
> + if ((DeviceSpeed == EFI_USB_SPEED_FULL) && (MaximumPacketLength >
> 64)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> --
> 2.37.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#96951): https://edk2.groups.io/g/devel/message/96951
> Mute This Topic: https://groups.io/mt/95465401/1768737
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-=-=-=-=
>
[-- Attachment #2: Type: text/html, Size: 10017 bytes --]
next prev parent reply other threads:[~2022-12-07 2:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-05 9:18 [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Sean Rhodes
2022-12-05 9:18 ` [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV Sean Rhodes
2022-12-05 9:18 ` [PATCH 3/4] MdeModulePkg/BmBoot: Skip removable media if it is not present Sean Rhodes
2022-12-07 2:23 ` [edk2-devel] " Wu, Hao A
2022-12-05 9:18 ` [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the MaxPacketLength to real world values Sean Rhodes
2022-12-07 2:23 ` Wu, Hao A [this message]
[not found] ` <172DDB13FC08F1B6.13790@groups.io>
2022-12-05 9:19 ` [edk2-devel] " Sean Rhodes
2022-12-08 7:06 ` [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices ian.chiu
2022-12-09 20:45 ` Sean Rhodes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB4025FDA4205FA0FD11086CA7CA1A9@DM6PR11MB4025.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox