Hi Prince,
There's some high-level changes I suggest to the patch series:
1. This patch series mixes changes across packages in single patches
which should be avoided.
2. Some of the patches include unrelated changes that would make git
revert difficult.
2.a. Example: In patch #1, one change is to add
gBoardModulePkgTokenSpaceGuid. This is a pre-requisite for
adding the PCDs to BoardModulePkg.dec but still an isolated
change. If someone were to add more commits in the future
that rely upon gBoardModulePkgTokenSpaceGuid and then wanted
to revert the PS/2 keyboard change it would be difficult due to
the coupling.
2.b. Example: In patch #3, a change is made that affects all
driver consumers in BoardModulePkg/LegacySioDxe but the commit
includes changes for enabling the driver in SimicsOpenBoardPkg.
The change to enable PS/2 keyboard/mouse in SimicsOpenBoardPkg
could not easily be reverted without affecting the overall driver.
3. The order of changes leaves some earlier commits incomplete.
3.a. For example, patch #1 exposes a PCD interface in BoardModulePkg
that is effectively not used until later patches so using the
PCDs at that point in the commit log would be misleading.
As far as I can tell, there's a high-level order to do the following:
1. Define gBoardModulePkgTokenSpaceGuid in BoardModulePkg.dec
2. Remove the LegacySioDxe driver from SimicsOpenBoardPkg
3. Add the LegacySioDxe driver to BoardModulePkg
4. Add new PCDs to BoardModulePkg for usage with LegacySioDxe
5. Update relevant boards in KabylakeOpenBoardPkg to use the LegacySioDxe
driver and PCDs from BoardModulePkg, remove the now redundant PCD
gKabylakeOpenBoardPkgTokenSpaceGuid.PcdPs2KbMsEnable in
KabylakeOpenBoardPkg/OpenBoardPkg.dec, configure the relevant PCDs
in BoardModulePkg in the board DSC files to properly use the
LegacySioDxe driver.
6. Add the Ps2KbcLib changes in KabylakeOpenBoardPkg as currently done
in V1 patches #4 and #5.
7. Do the same as #5 for SimicsOpenBoardPkg.
8. Explicitly set gBoardModulePkgTokenSpaceGuid.PcdPs2KbMsEnable
in WhiskeylakeOpenBoardPkg/WhiskeylakeURvp.
Thanks,
Michael
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Agyeman, Prince
> Sent: Friday, November 1, 2019 12:51 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [edk2-platforms] [PATCH 0/5] Enable Ps2 keyboard
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2228
>
> This patch series enables BIOS Ps2 keyboard in GalagoPro3
>
>
> What was done:
> Patch 0001 adds PCDs to BoardModulePkg that will enable/disable, describe
> Super I/O , Ps2 keyboard/mouse, uart1 and uart2 com ports
>
> Patch 0002 moves the generic Super I/O driver from SimicsOpenBoardPkg to
> BoardModulePkg in order for it to be shared. This driver publishes the
> gEfiSioProtocolGuid consumed by edk2's
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe
> driver to enable Ps2 keyboard functions in BIOS
>
> Patch 0003 adds PCDs defined in patch 0001 to enable/disable devices in the
> Super I/O driver added in patch 0002
>
> Patch 0004 adds a Null Ps2 Library that adds Ps2 keyboard device path to
> ConIn and ConInDev
>
> Patch 0005 enables Ps2 keyboard in BIOS by setting Ps2 keyboard related
> PCDs Prince Agyeman (5):
> Platform/Intel: Add gBoardModulePkgTokenSpaceGuid
> Platform/Intel: Move Sio Dxe Driver
> BoardModulePkg: Added Pcds Sio Driver
> KabylakeOpenBoardPkg: Add Ps2 keyboard Null Library
> KabylakeOpenBoardPkg: Add Ps2 Keyboard Support
>
> .../Intel/BoardModulePkg/BoardModulePkg.dec | 25 +++
> .../Intel/BoardModulePkg/BoardModulePkg.dsc | 1 +
> .../LegacySioDxe/ComponentName.c | 0
> .../LegacySioDxe/ComponentName.h | 0
> .../LegacySioDxe/LegacySioDxe.inf | 18 +-
> .../LegacySioDxe/Register.h | 0
> .../LegacySioDxe/SioChip.c | 71 +++++-
> .../LegacySioDxe/SioChip.h | 18 +-
> .../LegacySioDxe/SioDriver.c | 42 +++-
> .../LegacySioDxe/SioDriver.h | 1 -
> .../LegacySioDxe/SioService.c | 0
> .../LegacySioDxe/SioService.h | 0
> .../BoardAcpiLib/DxeBoardAcpiTableLib.inf | 3 +-
> .../DxeMultiBoardAcpiSupportLib.inf | 3 +-
> .../GalagoPro3/Library/Ps2KbcLib/Ps2KbcLib.c | 202 ++++++++++++++++++
> .../GalagoPro3/Library/Ps2KbcLib/Ps2KbcLib.h | 65 ++++++
> .../Library/Ps2KbcLib/Ps2KbcLib.inf | 39 ++++
> .../GalagoPro3/OpenBoardPkg.dsc | 7 +
> .../GalagoPro3/OpenBoardPkg.fdf | 2 +
> .../GalagoPro3/OpenBoardPkgPcd.dsc | 6 +
> .../BoardAcpiLib/DxeBoardAcpiTableLib.inf | 3 +-
> .../DxeMultiBoardAcpiSupportLib.inf | 3 +-
> .../KabylakeRvp3/OpenBoardPkgPcd.dsc | 5 +
> .../KabylakeOpenBoardPkg/OpenBoardPkg.dec | 2 -
> .../BoardX58Ich10/OpenBoardPkg.dsc | 2 +-
> .../BoardX58Ich10/OpenBoardPkg.fdf | 2 +-
> .../BoardX58Ich10/OpenBoardPkgPcd.dsc | 6 +
> .../WhiskeylakeOpenBoardPkg/OpenBoardPkg.dec | 1 -
> .../WhiskeylakeURvp/OpenBoardPkgPcd.dsc | 5 +
> 29 files changed, 499 insertions(+), 33 deletions(-) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/ComponentName.c (100%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/ComponentName.h (100%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/LegacySioDxe.inf (63%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/Register.h (100%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/SioChip.c (75%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/SioChip.h (90%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/SioDriver.c (88%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/SioDriver.h (95%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/SioService.c (100%) rename
> Platform/Intel/{SimicsOpenBoardPkg =>
> BoardModulePkg}/LegacySioDxe/SioService.h (100%) create mode 100644
> Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2K
> bcLib.c
> create mode 100644
> Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2K
> bcLib.h
> create mode 100644
> Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/Ps2KbcLib/Ps2K
> bcLib.inf
>
> --
> 2.19.1.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49939): https://edk2.groups.io/g/devel/message/49939
Mute This Topic: https://groups.io/mt/40479678/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
