From: "Chiu, Chasel" <chasel.chiu@intel.com>
To: Benjamin Doron <benjamin.doron00@gmail.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL
Date: Sat, 11 Sep 2021 00:23:48 +0000 [thread overview]
Message-ID: <BN9PR11MB5483C39A0453CCF0EDA69985E6D79@BN9PR11MB5483.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210906025530.279219-1-benjamin.doron00@gmail.com>
Push: https://github.com/tianocore/edk2-platforms/commit/f1add54453dddb2028c87445c234e746b62d7a12
Thanks,
Chasel
> -----Original Message-----
> From: Benjamin Doron <benjamin.doron00@gmail.com>
> Sent: Monday, September 6, 2021 10:55 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>
> Subject: [edk2-platforms][PATCH v4 1/2]
> KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL
>
> 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/Bo
> ardEcLib.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/Libr
> +++ ary/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/BoardEcL
> ib/EcCommands.c
> b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcL
> ib/EcCommands.c
> index 09b2b5ee9180..ea8a8ae11e4d 100644
> ---
> a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcL
> ib/EcCommands.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
> +++ dEcLib/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
prev parent reply other threads:[~2021-09-11 0:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-06 2:55 [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL Benjamin Doron
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 [this message]
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=BN9PR11MB5483C39A0453CCF0EDA69985E6D79@BN9PR11MB5483.namprd11.prod.outlook.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