Re: [edk2-platforms] [PATCH 4/5] KabylakeOpenBoardPkg: Add Ps2 keyboard Null Library

["Agyeman, Prince" <prince.agyeman@intel.com>]

Hi Chasel, 

I will fix the descriptions and typos as suggested in my v2 patch series.

Thanks for the feedback!

Prince 

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: Monday, November 4, 2019 1:04 AM
To: Agyeman, Prince <prince.agyeman@intel.com>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: RE: [edk2-platforms] [PATCH 4/5] KabylakeOpenBoardPkg: Add Ps2 keyboard Null Library


Hi Prince,

I added 3 questions below inline, please help to check them. Thanks!


> -----Original Message-----
> From: Agyeman, Prince <prince.agyeman@intel.com>
> Sent: Saturday, November 2, 2019 3:51 AM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>
> Subject: [edk2-platforms] [PATCH 4/5] KabylakeOpenBoardPkg: Add Ps2 
> keyboard Null Library
> 
> Added GalagoPro3 board ps2 keyboard library that adds ps2 device path 
> to ConIn and ConInDev Uefi variables
> 
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> 
> Signed-off-by: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../GalagoPro3/Library/Ps2KbcLib/Ps2KbcLib.c  | 202
> ++++++++++++++++++  .../GalagoPro3/Library/Ps2KbcLib/Ps2KbcLib.h  |
> 65 ++++++
>  .../Library/Ps2KbcLib/Ps2KbcLib.inf           |  39 ++++
>  3 files changed, 306 insertions(+)
>  create mode 100644
> Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2Kb
> cLib.c
>  create mode 100644
> Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2Kb
> cLib.h
>  create mode 100644
> Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2Kb
> cLib.inf
> 
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2
> K
> bcLib.c
> b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2
> KbcLib.c
> new file mode 100644
> index 0000000000..ac1563d3cd
> --- /dev/null
> +++
> b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/P
> +++ s2KbcLib.c
> @@ -0,0 +1,202 @@
> +/** @file
> +  Main file for NULL named library for Ps2 keyboard controller librarr.

I think this is not really a NULL library right? It actually does a lot real functionality.
Also the typo "librarr"


> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "Ps2KbcLib.h"
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED PLATFORM_KEYBOARD_DEVICE_PATH 
> +gKeyboardDevicePath = {
> +  gPciRootBridge,
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_PCI_DP,
> +      {
> +        (UINT8) (sizeof (PCI_DEVICE_PATH)),
> +        (UINT8) ((sizeof (PCI_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    0, // Function, patched in EnumPs2Keyboard
> +    0  // Device, patched in EnumPs2Keyboard
> +  },
> +  {
> +    {
> +      ACPI_DEVICE_PATH,
> +      ACPI_DP,
> +      {
> +        (UINT8) (sizeof (ACPI_HID_DEVICE_PATH)),
> +        (UINT8) ((sizeof (ACPI_HID_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    EISA_PNP_ID(0x0303),
> +    0
> +  },
> +  gEndEntire
> +};
> +
> +/**
> +  Check if PS2 keyboard is conntected, by sending ECHO command.
> +  @retval                       TRUE if connected FALSE otherwise
> +**/
> +BOOLEAN
> +DetectPs2Keyboard (
> +  VOID
> +  )
> +{
> +  UINT32                TimeOut;
> +  UINT32                RegEmptied;
> +  UINT8                 Data;
> +  UINT32                SumTimeOut;
> +  BOOLEAN               FoundPs2Kbc;
> +
> +  TimeOut     = 0;
> +  RegEmptied  = 0;
> +  FoundPs2Kbc       = FALSE;
> +
> +  //
> +  // Wait for input buffer empty
> +  //
> +  for (TimeOut = 0; TimeOut < PS2_KEYBOARD_TIMEOUT; TimeOut += 30) {
> +    if ((IoRead8 (KEYBOARD_8042_STATUS_REGISTER) & 0x02) == 0) {
> +      FoundPs2Kbc = TRUE;
> +      break;
> +    }
> +    MicroSecondDelay (30);
> +  }
> +
> +  if (FoundPs2Kbc == FALSE) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Send echo command
> +  //
> +  IoWrite8 (KEYBOARD_8042_DATA_REGISTER,
> KBC_INPBUF_VIA60_KBECHO);
> +
> +  //
> +  // Init variables
> +  //
> +  FoundPs2Kbc       = FALSE;
> +  TimeOut     = 0;
> +  SumTimeOut  = 0;
> +  Data = 0;
> +
> +  //
> +  // Read from 8042 (multiple times if needed)  // until the expected 
> + value appears  // use SumTimeOut to control the iteration  //  while
> + (1) {
> +    //
> +    // Perform a read
> +    //
> +    for (TimeOut = 0; TimeOut < PS2_KEYBOARD_TIMEOUT; TimeOut += 30)
> {
> +      if (IoRead8 (KEYBOARD_8042_STATUS_REGISTER) & 0x01) {
> +        Data = IoRead8 (KEYBOARD_8042_DATA_REGISTER);
> +        break;
> +      }
> +      MicroSecondDelay (30);
> +    }
> +
> +    SumTimeOut += TimeOut;
> +
> +    if (Data == KBC_INPBUF_VIA60_KBECHO) {
> +      FoundPs2Kbc = TRUE;
> +      break;
> +    }
> +
> +    if (SumTimeOut >= PS2_KEYBOARD_WAITFORVALUE_TIMEOUT) {
> +      break;
> +    }
> +  }
> +  return FoundPs2Kbc;
> +}
> +
> +/**
> +  Check if PS2 keyboard is conntected. If the result of first time is
> +  error, it will retry again.
> +  @retval                       TRUE if connected FALSE otherwise
> +**/
> +BOOLEAN
> +IsPs2KeyboardConnected (
> +  VOID
> +  )
> +{
> +  BOOLEAN Result;
> +  Result = DetectPs2Keyboard ();
> +
> +  if (Result == FALSE) {
> +    //
> +    // If there is no ps2 keyboard detected for the 1st time, retry again.
> +    //
> +    Result = DetectPs2Keyboard ();
> +  }
> +  return Result;
> +}
> +
> +
> +/**
> +  Updates the ConIn variable with Ps2 Keyboard device path,
> +  if it doesn't already exists in ConIn and ConInDev **/ VOID 
> +AddPs2Keyboard (
> +  VOID
> +  )
> +{
> +  SIO_PCI_ISA_BRIDGE_DEVICE_INFO *SioIsaInfo;
> +
> +  DEBUG ((DEBUG_INFO, "[AddPs2Keyboard]\n"));
> +
> +  SioIsaInfo = (SIO_PCI_ISA_BRIDGE_DEVICE_INFO*) FixedPcdGetPtr 
> + (PcdSuperIoPciIsaBridgeDevice);
> +
> +  //
> +  // patch IsaBridge device and and function  // 
> + gKeyboardDevicePath.IsaBridge.Device = SioIsaInfo->Device; 
> + gKeyboardDevicePath.IsaBridge.Function = SioIsaInfo->Funtion;
> +
> +  //
> +  // Append Ps2 Keyboard into "ConIn"
> +  //
> +  EfiBootManagerUpdateConsoleVariable (ConIn,
> (EFI_DEVICE_PATH_PROTOCOL
> + *) &gKeyboardDevicePath, NULL);
> +
> +  //
> +  // Append Ps2 Keyboard into "ConInDev"
> +  //
> +  EfiBootManagerUpdateConsoleVariable (ConInDev, 
> +(EFI_DEVICE_PATH_PROTOCOL *) &gKeyboardDevicePath, NULL); }
> +
> +
> +/**
> +  Constructor for the Ps2 keyboard controller library.
> +
> +  @param ImageHandle    the image handle of the process
> +  @param SystemTable    the EFI System Table pointer
> +
> +  @retval EFI_SUCCESS        the shell command handlers were
> installed sucessfully
> +  @retval EFI_UNSUPPORTED    the shell level required was not found.
> +**/
> +EFI_STATUS
> +EFIAPI
> +Ps2KbcLibConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  UINT8                               Ps2KbMsEnable;
> +
> +  Ps2KbMsEnable = PcdGet8 (PcdPs2KbMsEnable);
> +
> +  if (Ps2KbMsEnable == 0x1
> +    && IsPs2KeyboardConnected())
> +  {
> +      // add ps2 device path to ConIn and ConInDev
> +      AddPs2Keyboard ();
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2
> K
> bcLib.h
> b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2
> KbcLib.h
> new file mode 100644
> index 0000000000..60bba122d6
> --- /dev/null
> +++
> b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/P
> +++ s2KbcLib.h
> @@ -0,0 +1,65 @@
> +/** @file
> +  Header file for NULL named library for Ps2 keyboard controller library.

Same question here, is this really NULL library instance?



> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _PS2_KBC_LIB_H
> +#define _PS2_KBC_LIB_H
> +
> +#include <Uefi.h>
> +#include <Library/UefiLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/TimerLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +
> +//
> +// Below is the platform console device path // typedef struct {
> +  ACPI_HID_DEVICE_PATH      PciRootBridge;
> +  PCI_DEVICE_PATH           IsaBridge;
> +  ACPI_HID_DEVICE_PATH      Keyboard;
> +  EFI_DEVICE_PATH_PROTOCOL  End;
> +} PLATFORM_KEYBOARD_DEVICE_PATH;
> +
> +typedef struct {
> +  UINT8 Segment;
> +  UINT8 Bus;
> +  UINT8 Device;
> +  UINT8 Funtion;
> +} SIO_PCI_ISA_BRIDGE_DEVICE_INFO;
> +
> +#define gPciRootBridge \
> +  { \
> +    { \
> +      ACPI_DEVICE_PATH, \
> +      ACPI_DP, \
> +      { \
> +        (UINT8) (sizeof (ACPI_HID_DEVICE_PATH)), \
> +        (UINT8) ((sizeof (ACPI_HID_DEVICE_PATH)) >> 8) \
> +      }, \
> +    }, \
> +    EISA_PNP_ID (0x0A03), \
> +    0 \
> +  }
> +
> +#define gEndEntire \
> +  { \
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, { 
> +END_DEVICE_PATH_LENGTH, 0 } \
> +  }
> +
> +#define KBC_INPBUF_VIA60_KBECHO             0xEE
> +#define KEYBOARD_8042_DATA_REGISTER         0x60
> +#define KEYBOARD_8042_STATUS_REGISTER       0x64
> +
> +#define PS2_KEYBOARD_TIMEOUT                65536   // 0.07s
> +#define PS2_KEYBOARD_WAITFORVALUE_TIMEOUT   1000000 // 1s
> +#define PS2_KEYBOARD_KBEN                   0xF4
> +#define PS2_KEYBOARD_CMDECHO_ACK            0xFA
> +
> +#endif
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2
> K
> bcLib.inf
> b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2
> KbcLib.inf
> new file mode 100644
> index 0000000000..7ab1b628e3
> --- /dev/null
> +++
> b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/P
> +++ s2KbcLib.inf
> @@ -0,0 +1,39 @@
> +## @file
> +# Component information file for PEI GalagoPro3 Board Init Pre-Mem 
> +Library # # Copyright (c) 2019, Intel Corporation. All rights

This library is for PS2 initialization, not "Board Init Pre-Mem".


> +reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # ## 
> +[Defines]
> +  INF_VERSION                    = 0x00010006
> +  BASE_NAME                      = Ps2KbcLib
> +  FILE_GUID                      =
> E94EA52E-E84C-42E7-B863-EA1327EFA265
> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.2
> +  LIBRARY_CLASS                  = NULL|UEFI_DRIVER
> +  CONSTRUCTOR                    = Ps2KbcLibConstructor
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  KabylakeOpenBoardPkg/OpenBoardPkg.dec
> +  BoardModulePkg/BoardModulePkg.dec
> +
> +[Sources]
> +  Ps2KbcLib.c
> +  Ps2KbcLib.h
> +
> +[LibraryClasses]
> +  DevicePathLib
> +  DebugLib
> +  IoLib
> +  UefiDriverEntryPoint
> +  UefiBootManagerLib
> +  UefiLib
> +  TimerLib
> +
> +[Pcd]
> +  gBoardModulePkgTokenSpaceGuid.PcdPs2KbMsEnable
> +  gBoardModulePkgTokenSpaceGuid.PcdSuperIoPciIsaBridgeDevice
> --
> 2.19.1.windows.1