public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Benjamin Doron" <benjamin.doron00@gmail.com>
To: devel@edk2.groups.io
Cc: Chasel Chiu <chasel.chiu@intel.com>,
	Nate DeSimone <nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL
Date: Sun,  5 Sep 2021 22:55:29 -0400	[thread overview]
Message-ID: <20210906025530.279219-1-benjamin.doron00@gmail.com> (raw)

Check that data pointers are not NULL and update the documented return
values. 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    | 40 +++++------
 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c | 70 ++++++++++++--------
 2 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
index 2e7e0573900a..8bb4cccb8f19 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  Data is NULL
+  @retval    EFI_DEVICE_ERROR       Command error
+  @retval    EFI_TIMEOUT            Command timeout
 **/
 EFI_STATUS
 EcCmd90Read (
@@ -44,11 +45,12 @@ EcCmd91Write (
 /**
   Query the EC status.
 
-  @param[out] Status           EC status byte
+  @param[out] Status                EC status byte
 
-  @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  Data is NULL
+  @retval    EFI_DEVICE_ERROR       Command error
+  @retval    EFI_TIMEOUT            Command timeout
 **/
 EFI_STATUS
 EcCmd94Query (
@@ -58,12 +60,8 @@ EcCmd94Query (
 /**
   Reads a byte of EC (index) RAM.
 
-  @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
+  @param[in]  Address               Address to read
+  @param[out] Data                  Data received
 **/
 VOID
 EcIdxRead (
@@ -74,12 +72,8 @@ EcIdxRead (
 /**
   Writes a byte of EC (index) RAM.
 
-  @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
+  @param[in] Address          Address to read
+  @param[in] Data             Data received
 **/
 VOID
 EcIdxWrite (
@@ -91,10 +85,8 @@ EcIdxWrite (
   Read EC analog-digital converter.
   TODO: Check if ADC is valid.
 
+  @param[in]  Adc
   @param[out] DataBuffer
-
-  @retval     EFI_SUCCESS       Command success
-  @retval     EFI_DEVICE_ERROR  Command error
 **/
 VOID
 ReadEcAdcConverter (
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
index 09b2b5ee9180..ea8a8ae11e4d 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 command 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.
+ * Reversing vendor's protocols:
+ * - Only read and write are used.
+ * - Query, ACPI "CMDB" processing and command 58 are unused.
+ * - Equivalent KbcPeim is an 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  Data is NULL
+  @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__));
@@ -110,11 +119,12 @@ EcCmd91Write (
 /**
   Query the EC status.
 
-  @param[out] Status           EC status byte
+  @param[out] Status                EC status byte
 
-  @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  Data is NULL
+  @retval    EFI_DEVICE_ERROR       Command error
+  @retval    EFI_TIMEOUT            Command timeout
 **/
 EFI_STATUS
 EcCmd94Query (
@@ -123,6 +133,10 @@ EcCmd94Query (
 {
   EFI_STATUS                 Status;
 
+  if (Data == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   Status = SendEcCommand (0x94);
   if (EFI_ERROR (Status)) {
     DEBUG((DEBUG_ERROR, "%a(): SendEcCommand(0x94) failed!\n", __FUNCTION__));
@@ -140,11 +154,8 @@ EcCmd94Query (
 /**
   Reads a byte of EC (index) RAM.
 
-  @param[in]  Address          Address to read
-  @param[out] Data             Data received
-
-  @retval    EFI_SUCCESS       Command success
-  @retval    EFI_DEVICE_ERROR  Command error
+  @param[in]  Address               Address to read
+  @param[out] Data                  Data received
 **/
 VOID
 EcIdxRead (
@@ -152,6 +163,10 @@ EcIdxRead (
   OUT UINT8                  *Data
   )
 {
+  if (Data == NULL) {
+    return;
+  }
+
   IoWrite8 (EC_INDEX_IO_HIGH_ADDR_PORT, Address >> 8);
   IoWrite8 (EC_INDEX_IO_LOW_ADDR_PORT, Address);
   *Data = IoRead8 (EC_INDEX_IO_DATA_PORT);
@@ -160,11 +175,8 @@ EcIdxRead (
 /**
   Writes a byte of EC (index) RAM.
 
-  @param[in]  Address          Address to read
-  @param[out] Data             Data received
-
-  @retval    EFI_SUCCESS       Command success
-  @retval    EFI_DEVICE_ERROR  Command error
+  @param[in] Address          Address to read
+  @param[in] Data             Data received
 **/
 VOID
 EcIdxWrite (
@@ -181,10 +193,8 @@ EcIdxWrite (
   Read EC analog-digital converter.
   TODO: Check if ADC is valid.
 
+  @param[in]  Adc
   @param[out] DataBuffer
-
-  @retval     EFI_SUCCESS       Command success
-  @retval     EFI_DEVICE_ERROR  Command error
 **/
 VOID
 ReadEcAdcConverter (
@@ -195,6 +205,10 @@ ReadEcAdcConverter (
   UINT8            AdcConvertersEnabled;  // Contains some ADCs and some DACs
   UINT8            IdxData;
 
+  if (DataBuffer == NULL) {
+    return;
+  }
+
   // Backup enabled ADCs
   EcIdxRead (0xff15, &AdcConvertersEnabled);  // ADDAEN
 
-- 
2.31.1


             reply	other threads:[~2021-09-06  2:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06  2:55 Benjamin Doron [this message]
2021-09-06  2:55 ` [edk2-platforms][PATCH v4 2/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift Benjamin Doron
2021-09-06  9:05   ` Chiu, Chasel
2021-09-11  0:24   ` Chiu, Chasel
2021-09-06  9:04 ` [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL Chiu, Chasel
2021-09-11  0:23 ` Chiu, Chasel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210906025530.279219-1-benjamin.doron00@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox