[edk2-platforms][PATCH v1 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL

[edk2-platforms][PATCH v1 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL

[Benjamin Doron]

Check that Data pointer is not NULL. Also update some notes on this
library.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h    | 11 +++----
 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c | 31 +++++++++++++-------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
index 2e7e0573900a..55c0e1495ced 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
@@ -12,12 +12,13 @@
 /**
   Reads a byte of EC RAM.
 
-  @param[in]  Address          Address to read
-  @param[out] Data             Data received
+  @param[in]  Address               Address to read
+  @param[out] Data                  Data received
 
-  @retval    EFI_SUCCESS       Command success
-  @retval    EFI_DEVICE_ERROR  Command error
-  @retval    EFI_TIMEOUT       Command timeout
+  @retval    EFI_SUCCESS            Command success
+  @retval    EFI_INVALID_PARAMETER  Command success
+  @retval    EFI_DEVICE_ERROR       Command error
+  @retval    EFI_TIMEOUT            Command timeout
 **/
 EFI_STATUS
 EcCmd90Read (
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
index 09b2b5ee9180..98507eab34f1 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
@@ -12,18 +12,22 @@
 #include <Library/EcLib.h>
 #include <Library/IoLib.h>
 
-/* TODO - Implement:
+/* Notes:
+ * - ACPI "CMDB": Writing to this offset is equivalent to sending commands.
+ *   The CMDx bytes contain the parameters.
+ *
+ * TODO - Implement:
  *   - Commands: 0x58, 0xE1 and 0xE2
  *     - 0x51, 0x52: EC flash write?
  *   - ACPI CMDB: 0x63 and 0x64, 0xC7
  *     - 0x0B: Flash write (Boolean argument? Set in offset 0x0B?)
  *
- * NB: Consider that if a vendor's UEFI driver consumes
- *     unimplemented PPI/protocol, the driver is dead code.
+ * Protocols/PPI reversing:
+ * - Only read and write are used.
+ * - EcQuery, ACPI "CMDB" processing and command 58 are unused.
+ * - Equivalent KbcPeim is unused PPI.
  *
- * NOTE: Check protocol use.
- *   - Commands delivered across vendor's modules
- *   - EC writes also control behaviour
+ * NB: Also look for potential EC library
  */
 
 #define EC_INDEX_IO_PORT            0x1200
@@ -34,12 +38,13 @@
 /**
   Reads a byte of EC RAM.
 
-  @param[in]  Address          Address to read
-  @param[out] Data             Data received
+  @param[in]  Address               Address to read
+  @param[out] Data                  Data received
 
-  @retval    EFI_SUCCESS       Command success
-  @retval    EFI_DEVICE_ERROR  Command error
-  @retval    EFI_TIMEOUT       Command timeout
+  @retval    EFI_SUCCESS            Command success
+  @retval    EFI_INVALID_PARAMETER  Command success
+  @retval    EFI_DEVICE_ERROR       Command error
+  @retval    EFI_TIMEOUT            Command timeout
 **/
 EFI_STATUS
 EcCmd90Read (
@@ -49,6 +54,10 @@ EcCmd90Read (
 {
   EFI_STATUS                 Status;
 
+  if (Data == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   Status = SendEcCommand (0x90);
   if (EFI_ERROR (Status)) {
     DEBUG((DEBUG_ERROR, "%a(): SendEcCommand(0x90) failed!\n", __FUNCTION__));
-- 
2.31.1

[edk2-platforms][PATCH v1 2/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift

[Benjamin Doron]

Since the time is sent to the EC byte-by-byte, perform shift by a byte
multiple of bits. Also update some comments.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
index 906b2d265092..d87cb006bf6f 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
@@ -24,7 +24,7 @@ EcSendTime (
 {
   EFI_STATUS  Status;
   EFI_TIME    EfiTime;
-  // TODO: Confirm this is really INTN and not UINTN
+  // Time could be negative (before 2016)
   INTN        EcTime;
   UINT8       EcTimeByte;
   INTN        Index;
@@ -36,7 +36,7 @@ EcSendTime (
     return;
   }
 
-  // Time since year of release?
+  // Time since year of release. Note that "century" is ignored.
   EcTime = ((EfiTime.Year << 26) + (EfiTime.Month << 22) + (EfiTime.Day << 17)
          + (EfiTime.Hour << 12) + (EfiTime.Minute << 6) + (EfiTime.Second)
          /* 16 years */
@@ -45,7 +45,8 @@ EcSendTime (
   DEBUG ((DEBUG_INFO, "EC: reporting present time 0x%x\n", EcTime));
   SendEcCommand (0xE0);
   for (Index = 0; Index < 4; Index++) {
-    EcTimeByte = EcTime >> Index;
+    // Shift bytes
+    EcTimeByte = EcTime >> Index*8;
     DEBUG ((DEBUG_INFO, "EC: Sending 0x%x (iteration %d)\n", EcTimeByte, Index));
     SendEcData (EcTimeByte);
   }
@@ -61,13 +62,14 @@ EcSendTime (
 
 **/
 VOID
-EcInit (
+EcRequestsTime (
   VOID
   )
 {
   UINT8           Dat;
 
-  /* Vendor's UEFI modules "notify" this protocol in RtKbcDriver */
+  /* This is executed as protocol notify in RtKbcDriver when *CommonService protocol
+   * is installed. Effectively, this code could execute from the entrypoint */
   EcCmd90Read (0x79, &Dat);
   if (Dat & BIT0) {
     EcSendTime ();
@@ -86,7 +88,7 @@ BoardInitAfterPciEnumeration (
   VOID
   )
 {
-  EcInit ();
+  EcRequestsTime ();
   return EFI_SUCCESS;
 }
 
-- 
2.31.1