Pedro,

It sounds good to me.  I am exposing our formerly proprietary code without much refactoring.  But I see no reason to have CRC related code in multiple libraries.

Regards,
Isaac

From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Thursday, March 10, 2022 3:19 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Oram, Isaac W <isaac.w.oram@intel.com>
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
Subject: Re: [edk2-devel][edk2-platforms][PATCH V1 3/9] WhitleyOpenBoardPkg/BaseCrcLib: Add library for CRC16

Hi,

I've just noticed this patch adds CRC16, which I've already added to my Ext4Pkg (https://github.com/tianocore/edk2-platforms/blob/master/Features/Ext4Pkg/Ext4Dxe/Crc16.c).
I suggest we add CRC16 (and possibly CRC32C, which I already have in my package as well) to MdePkg, as to de-duplicate code which might be useful in other places.
What do you think? If it sounds good to you, I'll open a bugzilla and work on that.

Best regards,
Pedro

On Thu, Mar 10, 2022 at 10:41 PM Oram, Isaac W <isaac.w.oram@intel.com<mailto:isaac.w.oram@intel.com>> wrote:
Core only supports CRC32, this library adds CRC16 support.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>>
Cc: Chasel Chiu <chasel.chiu@intel.com<mailto:chasel.chiu@intel.com>>
Signed-off-by: Isaac Oram <isaac.w.oram@intel.com<mailto:isaac.w.oram@intel.com>>
---
 Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h          | 42 ++++++++++++
 Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c   | 71 ++++++++++++++++++++
 Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf | 23 +++++++
 Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc                   |  1 +
 4 files changed, 137 insertions(+)

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h b/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
new file mode 100644
index 0000000000..7ca3b7cabb
--- /dev/null
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
@@ -0,0 +1,42 @@
+/** @file
+  Interface header file for the CRC library class.
+
+  @copyright
+  Copyright 2016 - 2018 Intel Corporation. <BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef _CRC_LIB_H_
+#define _CRC_LIB_H_
+
+#include <Uefi.h>
+
+/**
+  Calculate a 16-bit CRC.
+
+  The algorithm used is MSB-first form of the ITU-T Recommendation V.41, which
+  uses an initial value of 0x0000 and a polynomial of 0x1021. It is the same
+  algorithm used by XMODEM.
+
+  The output CRC location is not updated until the calculation is finished, so
+  it is possible to pass a structure as the data, and the CRC field of the same
+  structure as the output location for the calculated CRC. The CRC field should
+  be set to zero before calling this function. Once the CRC field is updated by
+  this function, running it again over the structure produces a CRC of zero.
+
+  @param[in]  Data              A pointer to the target data.
+  @param[in]  DataSize          The target data size.
+  @param[out] CrcOut            A pointer to the return location of the CRC.
+
+  @retval EFI_SUCCESS           The CRC was calculated successfully.
+  @retval EFI_INVALID_PARAMETER A null pointer was provided.
+**/
+EFI_STATUS
+CalculateCrc16 (
+  IN  VOID    *Data,
+  IN  UINTN   DataSize,
+  OUT UINT16  *CrcOut
+  );
+
+#endif  // _CRC_LIB_H_
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
new file mode 100644
index 0000000000..3e8fa402ad
--- /dev/null
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
@@ -0,0 +1,71 @@
+/** @file
+  Base implementation of the CRC library class.
+
+  @copyright
+  Copyright 2016 - 2018 Intel Corporation. <BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h>
+#include <Library/CrcLib.h>
+
+/**
+  Calculate a 16-bit CRC.
+
+  The algorithm used is MSB-first form of the ITU-T Recommendation V.41, which
+  uses an initial value of 0x0000 and a polynomial of 0x1021. It is the same
+  algorithm used by XMODEM.
+
+  The output CRC location is not updated until the calculation is finished, so
+  it is possible to pass a structure as the data, and the CRC field of the same
+  structure as the output location for the calculated CRC. The CRC field should
+  be set to zero before calling this function. Once the CRC field is updated by
+  this function, running it again over the structure produces a CRC of zero.
+
+  @param[in]  Data              A pointer to the target data.
+  @param[in]  DataSize          The target data size.
+  @param[out] CrcOut            A pointer to the return location of the CRC.
+
+  @retval EFI_SUCCESS           The CRC was calculated successfully.
+  @retval EFI_INVALID_PARAMETER A null pointer was provided.
+**/
+EFI_STATUS
+CalculateCrc16 (
+  IN  VOID    *Data,
+  IN  UINTN   DataSize,
+  OUT UINT16  *CrcOut
+  )
+{
+  UINT32  Crc;
+  UINTN   Index;
+  UINT8   *Byte;
+
+  if (Data == NULL || CrcOut == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Crc = 0x0000;
+  for (Byte = (UINT8 *) Data; Byte < (UINT8 *) Data + DataSize; Byte++) {
+    //
+    // XOR the next data byte into the CRC.
+    //
+    Crc ^= (UINT16) *Byte << 8;
+    //
+    // Shift out eight bits, feeding back based on the polynomial whenever a
+    // 1 is shifted out of bit 15.
+    //
+    for (Index = 0; Index < 8; Index++) {
+      Crc <<= 1;
+      if (Crc & BIT16) {
+        Crc ^= 0x1021;
+      }
+    }
+  }
+
+  //
+  // Mask and return the 16-bit CRC.
+  //
+  *CrcOut = (UINT16) (Crc & 0xFFFF);
+  return EFI_SUCCESS;
+}
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf
new file mode 100644
index 0000000000..6b404e1259
--- /dev/null
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf
@@ -0,0 +1,23 @@
+## @file
+# Base implementation of the CRC library class.
+#
+# @copyright
+# Copyright 2016 Intel Corporation. <BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                   = 0x00010019
+  BASE_NAME                     = BaseCrcLib
+  FILE_GUID                     = F3BE9A28-78A2-4B02-AB26-D27EE85D9256
+  MODULE_TYPE                   = BASE
+  VERSION_STRING                = 1.0
+  LIBRARY_CLASS                 = CrcLib
+
+[Sources]
+  BaseCrcLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  WhitleyOpenBoardPkg/PlatformPkg.dec
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
index e78a104004..9cdb5bc2f6 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
+++ b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
@@ -618,6 +618,7 @@
   PciSegmentInfoLib|$(PLATFORM_PKG)/Pci/Library/PciSegmentInfoLibSimple/PciSegmentInfoLibSimple.inf
   PlatformOpromPolicyLib|$(RP_PKG)/Library/PlatformOpromPolicyLibNull/PlatformOpromPolicyLibNull.inf
   VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
+  CrcLib|WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf

 [LibraryClasses.Common.SEC, LibraryClasses.Common.PEI_CORE, LibraryClasses.Common.PEIM]
   FspWrapperApiLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFspWrapperApiLib.inf
--
2.27.0.windows.1







--
Pedro Falcato