Re: [PATCH v2 1/1] EmbeddedPkg/Drivers: add virtual keyboard driver

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Mar 08, 2018 at 09:34:48PM +0800, Haojian Zhuang wrote:
> This driver is used to simulate a keyboard. For example, user
> could read GPIO setting or data from RAM address. If the value
> matches the expected pattern, it could trigger a key pressed
> event.
> 
> User needs to implement hooks of PLATFORM_VIRTUAL_KBD_PROTOCOL.
> There're 4 hooks in this protocol.
> Register(): Quote the interface that user needs. For example, user
> needs to locate GPIO protocol if he wants to simulate a GPIO value
> as a key.
> Reset(): Do the initialization before reading value.
> Query(): Read value. If the value matches the expected pattern,
> trigger a key pressed event.
> Clear(): Clean the value if necessary.
>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  EmbeddedPkg/EmbeddedPkg.dec                                   |    1 +
>  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf |   60 +
>  EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.h        |  154 +++
>  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.h      |  544 +++++++++
>  EmbeddedPkg/Include/Protocol/PlatformVirtualKeyboard.h        |   65 ++
>  EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.c        |  188 ++++
>  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c      | 1149 ++++++++++++++++++++
>  7 files changed, 2161 insertions(+)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index e48ce2e95bf7..eb135340b173 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -80,6 +80,7 @@ [Protocols.common]
>    gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 0x54, 0x17, 0xc7,  0x0b, 0x44 }}
>    gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 0x08, 0x12, 0x54, 0x7a, 0xc2 }}
>    gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
> +  gPlatformVirtualKeyboardProtocolGuid = { 0x0e3606d2, 0x1dc3, 0x4e6f, { 0xbe, 0x65, 0x39, 0x49, 0x82, 0xa2, 0x65, 0x47 }}

I will move that line up one step before pushing.

>  
>  [Ppis]
>    gEdkiiEmbeddedGpioPpiGuid = { 0x21c3b115, 0x4e0b, 0x470c, { 0x85, 0xc7, 0xe1, 0x05, 0xa5, 0x75, 0xc9, 0x7b }}
> diff --git a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
> new file mode 100644
> index 000000000000..deb54dab6592
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
> @@ -0,0 +1,60 @@
> +## @file
> +# Virtual Keyboard driver.
> +#
> +# Copyright (c) 2018, Linaro Ltd. All rights reserved.<BR>
> +#
> +# This program and the accompanying materials
> +# are licensed and made available under the terms and conditions
> +# of the BSD License which accompanies this distribution.  The
> +# full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = VirtualKeyboardDxe
> +  FILE_GUID                      = 88079b18-b42b-44aa-a6f2-b83911075e89
> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = InitializeVirtualKeyboard
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 IPF ARM AARCH64
> +#
> +#  DRIVER_BINDING                = gVirtualKeyboardDriverBinding
> +#  COMPONENT_NAME                = gVirtualKeyboardComponentName
> +#
> +
> +[Sources.common]
> +  ComponentName.c
> +  VirtualKeyboard.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

MdeModulePkg before MdePkg.

> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  IoLib
> +  ReportStatusCodeLib
> +  UefiDriverEntryPoint
> +  UefiBootServicesTableLib

UefiBoot... before UefiDriver...

> +  UefiLib
> +
> +[Protocols]
> +  gEfiDriverBindingProtocolGuid
> +  gEfiSimpleTextInProtocolGuid
> +  gEfiSimpleTextInputExProtocolGuid
> +  gPlatformVirtualKeyboardProtocolGuid
> +
> +[Depex]
> +  TRUE

OK, so, I am reasonably convinced that this driver carries more across
from the Csm BiosKeyboard than is required (what about all of those
Stalls), and it still contains misleading comments about the 8042 (PC
keyboard controller). But I don't have time to go through it all with
a comb right now, and it sort of blocks some of the really nice
Arm BootManager improvements, and it is a useful addition ... so I am
going to push it for now (with the changes mentioned above
implemented).

I would appreciate if we could get back to this later in the year
though.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 1df5fb2d83

/
    Leif