* [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL
@ 2021-09-06 2:55 Benjamin Doron
2021-09-06 2:55 ` [edk2-platforms][PATCH v4 2/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift Benjamin Doron
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Benjamin Doron @ 2021-09-06 2:55 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [edk2-platforms][PATCH v4 2/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift
2021-09-06 2:55 [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL Benjamin Doron
@ 2021-09-06 2:55 ` 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
2 siblings, 2 replies; 6+ messages in thread
From: Benjamin Doron @ 2021-09-06 2:55 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone
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..4bce51886e3a 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 vendor's 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL
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:04 ` Chiu, Chasel
2021-09-11 0:23 ` Chiu, Chasel
2 siblings, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2021-09-06 9:04 UTC (permalink / raw)
To: Benjamin Doron, devel@edk2.groups.io; +Cc: Desimone, Nathaniel L
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-platforms][PATCH v4 2/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift
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
1 sibling, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2021-09-06 9:05 UTC (permalink / raw)
To: Benjamin Doron, devel@edk2.groups.io; +Cc: Desimone, Nathaniel L
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Benjamin Doron <benjamin.doron00@gmail.com>
> Sent: Monday, September 6, 2021 10:56 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 2/2]
> KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift
>
> 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/BoardInitLi
> b/DxeBoardInitLib.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInit
> Lib/DxeBoardInitLib.c
> b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInit
> Lib/DxeBoardInitLib.c
> index 906b2d265092..4bce51886e3a 100644
> ---
> a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInit
> Lib/DxeBoardInitLib.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
> +++ dInitLib/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 vendor's 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL
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:04 ` [edk2-platforms][PATCH v4 1/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/BoardEcLib: Check for NULL Chiu, Chasel
@ 2021-09-11 0:23 ` Chiu, Chasel
2 siblings, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2021-09-11 0:23 UTC (permalink / raw)
To: Benjamin Doron, devel@edk2.groups.io; +Cc: Desimone, Nathaniel L
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-platforms][PATCH v4 2/2] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift
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
1 sibling, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2021-09-11 0:24 UTC (permalink / raw)
To: Benjamin Doron, devel@edk2.groups.io; +Cc: Desimone, Nathaniel L
Pushed: https://github.com/tianocore/edk2-platforms/commit/057f23a2022c7efb9f99c5c523e09125cb66756b
Thanks,
Chasel
> -----Original Message-----
> From: Benjamin Doron <benjamin.doron00@gmail.com>
> Sent: Monday, September 6, 2021 10:56 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 2/2]
> KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Fix byte shift
>
> 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/BoardInitLi
> b/DxeBoardInitLib.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInit
> Lib/DxeBoardInitLib.c
> b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInit
> Lib/DxeBoardInitLib.c
> index 906b2d265092..4bce51886e3a 100644
> ---
> a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInit
> Lib/DxeBoardInitLib.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
> +++ dInitLib/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 vendor's 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-11 0:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox