From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f49.google.com (mail-ot1-f49.google.com [209.85.210.49]) by mx.groups.io with SMTP id smtpd.web08.5080.1611819072192460017 for ; Wed, 27 Jan 2021 23:31:12 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@9elements.com header.s=google header.b=LIHxs8hF; spf=pass (domain: 9elements.com, ip: 209.85.210.49, mailfrom: patrick.rudolph@9elements.com) Received: by mail-ot1-f49.google.com with SMTP id v1so4307205ott.10 for ; Wed, 27 Jan 2021 23:31:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VLNtZadLp9okxHypUHPICIcVAjLECHoQ5E6qzFc9rBI=; b=LIHxs8hF+qXc8leey1JGO7YpPe5vK6t1RCbfNr7Ktxuqu+TZkKEZT+oBhxHeQX8I9C 31NC2Q2dwz1vVRVMh7PiPMhGczNgn/vYoli/3xfN/8ATZ9HNv9b9cCBVwW7VZ0JcSvrj 8UDvqicgm4jUvCgT/e6Wc4ELtIMMr28CbLHLq5ne1BwoU3UAad5/g/IzqZfG0gQZFhv/ Q8tSZMOscIcLjB+4pJJgt9YJ+gJGT/aQQUudaxOtJuVqFm9044iIWSEQzcTEMq0HyBa5 0aSsREZ4fkgGAYICd5/8w60w6XQ5s2UAquw0FIikQKqB7SJu8ae+Xqja0b8SvBbHjEoV 6L2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VLNtZadLp9okxHypUHPICIcVAjLECHoQ5E6qzFc9rBI=; b=CGy/JRfrIT0lZ3EUOGd4GUC7bt7zZCs7y+PHyWHCQXmRQq9nlnBPD1aBMPycm5h78N a9c33Po1U+lTVGU1kJ5Onz7w4k0InbpMB4/5su+WyH4JEYMpPcGd9pDTBf3p4LdWJ4TO rHckPRWtgE7Yw8DjJl/GgB5nHQbscTpB6GAszCwdehHQ8CEWRd8X1iscV68u1WLqVHVy caliNg8BX9TCxe0hbi+VKBs4pr+ava0LL/CMHMD4fMfPTuvobme2g8HIg1fD4SUJPBB4 xnA1BEDwRs8OhIrHrVI/4dEsp7Ft1PKqTaEp6MW0zYto7P3Dph/ig1EkC56D9VhcPVHR CJ2w== X-Gm-Message-State: AOAM531kLuon8tQsemTxr5HFG226zVCCzzFSvbyjp0t4r2+UNun2zEaA 5DT2g1cOteGs0Y6hUN3viI9JfFHF419xh/9+CoQ18A== X-Google-Smtp-Source: ABdhPJyCG+V5te5CC5ycRQeWDuPh/lMXSvJegXyKdgLy3RQ6ydJKLvVwrmABUx2PlEXVpQTPisYUgHFL2ZPxVzu0tR4= X-Received: by 2002:a9d:75d1:: with SMTP id c17mr10647468otl.78.1611819071496; Wed, 27 Jan 2021 23:31:11 -0800 (PST) MIME-Version: 1.0 References: <20210120155900.3343123-1-patrick.rudolph@9elements.com> <20210120155900.3343123-2-patrick.rudolph@9elements.com> In-Reply-To: From: "Patrick Rudolph" Date: Thu, 28 Jan 2021 08:31:00 +0100 Message-ID: Subject: Re: [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting To: "Wu, Hao A" Cc: "devel@edk2.groups.io" , "Wang, Jian J" , "Ni, Ray" Content-Type: text/plain; charset="UTF-8" Hello, Until now no regressions have been observed. I'll provide a full test description using a wide variety of platforms soon. Kind Regards, Patrick Rudolph On Thu, Jan 28, 2021 at 3:14 AM Wu, Hao A wrote: > > > -----Original Message----- > > From: Patrick Rudolph > > Sent: Wednesday, January 20, 2021 11:59 PM > > To: devel@edk2.groups.io > > Cc: Wang, Jian J ; Wu, Hao A ; > > Ni, Ray > > Subject: [PATCH 2/2] MdeModulePkg/Usb/Keyboard.c: don't request > > protocol before setting > > > > From: Matt DeVillier > > > > No need to check the interface protocol then conditionally setting, just set it > > to BOOT_PROTOCOL and check for error. > > > > This is what Linux does for HID devices as some don't follow the USB spec. > > One example is the Aspeed BMC HID keyboard device, which adds a massive > > boot delay without this patch as it doesn't respond to 'GetProtocolRequest'. > > > I am okay with the code change. > > But since the current logic is here for a long time, I am not sure whether > other USB keyboard device will have functional/performance impact for this > change. > > Besides the above mentioned Aspeed BMC HID keyboard device (which has a > performance issue for the Get_Protocol request that the patch is going to > drop), could you at least try one USB keyboard device that works fine with > the origin code logic to see if there is any side effect for this patch? > > Thanks in advance. > > Best Regards, > Hao Wu > > > > > > Signed-off-by: Matt DeVillier > > Signed-off-by: Patrick Rudolph > > --- > > MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++++++++++++------- > > - > > 1 file changed, 17 insertions(+), 11 deletions(-) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > index 77e20b203f..5914174b4d 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > @@ -801,7 +801,6 @@ InitUSBKeyboard ( > > IN OUT USB_KB_DEV *UsbKeyboardDevice ) {- UINT8 Protocol; > > EFI_STATUS Status; REPORT_STATUS_CODE_WITH_DEVICE_PATH (@@ > > -814,21 +813,28 @@ InitUSBKeyboard ( > > InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA)); > > InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof > > (EFI_KEY_DATA)); - UsbGetProtocolRequest (- UsbKeyboardDevice- > > >UsbIo,- UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,- > > &Protocol- ); // // Set boot protocol for the USB Keyboard. // This driver > > only supports boot protocol. //- if (Protocol != BOOT_PROTOCOL) {- > > UsbSetProtocolRequest (- UsbKeyboardDevice->UsbIo,- > > UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,- > > BOOT_PROTOCOL+ Status = UsbSetProtocolRequest (+ > > UsbKeyboardDevice->UsbIo,+ UsbKeyboardDevice- > > >InterfaceDescriptor.InterfaceNumber,+ BOOT_PROTOCOL+ );+ > > if (EFI_ERROR (Status)) {+ //+ // If protocol could not be set here, it > > means+ // the keyboard interface has some errors and could+ // not be > > initialized+ //+ REPORT_STATUS_CODE_WITH_DEVICE_PATH (+ > > EFI_ERROR_CODE | EFI_ERROR_MINOR,+ (EFI_PERIPHERAL_KEYBOARD | > > EFI_P_EC_INTERFACE_ERROR),+ UsbKeyboardDevice->DevicePath );++ > > return EFI_DEVICE_ERROR; } UsbKeyboardDevice->CtrlOn = FALSE;-- > > 2.26.2 >