PMICDRV-179: Add ability to control CONFIG_CRC

Add support for controlling the CONFIG_CRC register protection mechanism. A new property (`configCrcEnable`) is added to the Pmic_CoreCfg_t and Pmic_CoreHandle_t which controls this functionality. Additionally, new APIs are added to allow users to control this feature outside of initialization contexts: - Pmic_configCrcEnable(Pmic_CoreHandle_t *, bool) - Pmic_configCrcDisable(Pmic_CoreHandle_t *) Signed-off-by: Michael Leonard <[email protected]>
TexasInstruments · Jun 27, 2024 · 8a89fdc · 8a89fdc
1 parent a5502e0
commit 8a89fdc
Show file tree

Hide file tree

Showing 7 changed files with 181 additions and 38 deletions.
diff --git a/README.md b/README.md
@@ -140,20 +140,22 @@ Pmic_CoreHandle_t PmicHandle;
 
 Pmic_CoreCfg_t coreCfg = {
     .validParams = (
-        PMIC_CFG_DEVICE_TYPE_VALID_SHIFT   |
-        PMIC_CFG_COMM_MODE_VALID_SHIFT     |
-        PMIC_CFG_CRC_ENABLE_VALID_SHIFT    |
-        PMIC_CFG_SLAVEADDR_VALID_SHIFT     |
-        PMIC_CFG_COMM_HANDLE_VALID_SHIFT   |
-        PMIC_CFG_COMM_IO_RD_VALID_SHIFT    |
-        PMIC_CFG_COMM_IO_WR_VALID_SHIFT    |
-        PMIC_CFG_CRITSEC_START_VALID_SHIFT |
+        PMIC_CFG_DEVICE_TYPE_VALID_SHIFT    |
+        PMIC_CFG_COMM_MODE_VALID_SHIFT      |
+        PMIC_CFG_CRC_ENABLE_VALID_SHIFT     |
+        PMIC_CFG_CFG_CRC_ENABLE_VALID_SHIFT |
+        PMIC_CFG_SLAVEADDR_VALID_SHIFT      |
+        PMIC_CFG_COMM_HANDLE_VALID_SHIFT    |
+        PMIC_CFG_COMM_IO_RD_VALID_SHIFT     |
+        PMIC_CFG_COMM_IO_WR_VALID_SHIFT     |
+        PMIC_CFG_CRITSEC_START_VALID_SHIFT  |
         PMIC_CFG_CRITSEC_STOP_VALID_SHIFT
     ),
     .instType = PMIC_MAIN_INST,
     .pmicDeviceType = PMIC_DEV_COACH_LP8772X,
     .commMode = PMIC_INTF_I2C_SINGLE,
     .crcEnable = PMIC_ENABLE,
+    .configCrcEnable = PMIC_ENABLE,
     .slaveAddr = <Device I2C Address>,
     .pCommHandle = &commHandle,
     .pFnPmicCommIoRead = PmicCommIoRead,

diff --git a/include/pmic.h b/include/pmic.h
@@ -198,16 +198,6 @@ extern "C" {
 #define PMIC_I2C_FORCED_HS_MODE (1U)
 /** @} */
 
-/**
- * @anchor Pmic_CrcEnableCfg
- * @name PMIC CRC Enable/Disable Configuration
- *
- *  @{
- */
-#define PMIC_CRC_DISABLE    (0U)
-#define PMIC_CRC_ENABLE     (1U)
-/** @} */
-
 /**
  * @anchor Pmic_ValidParamCfg
  * @name  PMIC Config Structure Param Bits
@@ -216,20 +206,21 @@ extern "C" {
  *
  * @{
  */
-#define PMIC_CFG_DEVICE_TYPE_VALID   (0U)
-#define PMIC_CFG_COMM_MODE_VALID     (1U)
-#define PMIC_CFG_SLAVEADDR_VALID     (2U)
-#define PMIC_CFG_QASLAVEADDR_VALID   (3U)
-#define PMIC_CFG_NVMSLAVEADDR_VALID  (4U)
-#define PMIC_CFG_COMM_HANDLE_VALID   (5U)
-#define PMIC_CFG_QACOMM_HANDLE_VALID (6U)
-#define PMIC_CFG_COMM_IO_RD_VALID    (7U)
-#define PMIC_CFG_COMM_IO_WR_VALID    (8U)
-#define PMIC_CFG_CRITSEC_START_VALID (9U)
-#define PMIC_CFG_CRITSEC_STOP_VALID  (10U)
-#define PMIC_CFG_I2C1_SPEED_VALID    (11U)
-#define PMIC_CFG_I2C2_SPEED_VALID    (12U)
-#define PMIC_CFG_CRC_ENABLE_VALID    (13U)
+#define PMIC_CFG_DEVICE_TYPE_VALID    (0U)
+#define PMIC_CFG_COMM_MODE_VALID      (1U)
+#define PMIC_CFG_SLAVEADDR_VALID      (2U)
+#define PMIC_CFG_QASLAVEADDR_VALID    (3U)
+#define PMIC_CFG_NVMSLAVEADDR_VALID   (4U)
+#define PMIC_CFG_COMM_HANDLE_VALID    (5U)
+#define PMIC_CFG_QACOMM_HANDLE_VALID  (6U)
+#define PMIC_CFG_COMM_IO_RD_VALID     (7U)
+#define PMIC_CFG_COMM_IO_WR_VALID     (8U)
+#define PMIC_CFG_CRITSEC_START_VALID  (9U)
+#define PMIC_CFG_CRITSEC_STOP_VALID   (10U)
+#define PMIC_CFG_I2C1_SPEED_VALID     (11U)
+#define PMIC_CFG_I2C2_SPEED_VALID     (12U)
+#define PMIC_CFG_CRC_ENABLE_VALID     (13U)
+#define PMIC_CFG_CFG_CRC_ENABLE_VALID (14U)
 /** @} */
 
 /**
@@ -255,6 +246,7 @@ extern "C" {
 #define PMIC_CFG_I2C1_SPEED_VALID_SHIFT     (1U << PMIC_CFG_I2C1_SPEED_VALID)
 #define PMIC_CFG_I2C2_SPEED_VALID_SHIFT     (1U << PMIC_CFG_I2C2_SPEED_VALID)
 #define PMIC_CFG_CRC_ENABLE_VALID_SHIFT     (1U << PMIC_CFG_CRC_ENABLE_VALID)
+#define PMIC_CFG_CFG_CRC_ENABLE_VALID_SHIFT (1U << PMIC_CFG_CFG_CRC_ENABLE_VALID)
 /** @brief Helper macro to set all `validParams` necessary for configuring I2C
  * based driver. */
 #define PMIC_CFG_ALL_I2C_VALID_SHIFT        (\
@@ -263,6 +255,7 @@ extern "C" {
     PMIC_CFG_SLAVEADDR_VALID_SHIFT      |\
     PMIC_CFG_I2C1_SPEED_VALID_SHIFT     |\
     PMIC_CFG_CRC_ENABLE_VALID_SHIFT     |\
+    PMIC_CFG_CFG_CRC_ENABLE_VALID_SHIFT |\
     PMIC_CFG_COMM_IO_RD_VALID_SHIFT     |\
     PMIC_CFG_COMM_IO_WR_VALID_SHIFT     |\
     PMIC_CFG_COMM_HANDLE_VALID_SHIFT    |\
@@ -342,6 +335,11 @@ extern "C" {
  * the CRC feature will be disabled in HW and no calculations will be performed
  * by the driver.
  *
+ * @param configCrcEnable Controls whether configuration register CRC is enabled
+ * or disabled. If enabled, APIs are provided to enable, disable, and recalcuate
+ * this CRC value. Otherwise, this feature is left disabled and no further user
+ * input is required.
+ *
  * @param pFnPmicCommIoRd Pointer to I2C/SPI Comm LLD Read Function. Valid
  * only when `PMIC_CFG_COMM_IO_RD_VALID` bit of `validParams` is set.
  *
@@ -371,6 +369,7 @@ typedef struct Pmic_CoreCfg_s {
     uint8_t i2c1Speed;
     uint8_t i2c2Speed;
     bool crcEnable;
+    bool configCrcEnable;
     void *pCommHandle;
     void *pQACommHandle;
     int32_t (*pFnPmicCommIoRd)(struct Pmic_CoreHandle_s *handle,

diff --git a/include/pmic_common.h b/include/pmic_common.h
@@ -68,6 +68,7 @@ typedef struct Pmic_CoreHandle_s {
     uint8_t i2c1Speed;
     uint8_t i2c2Speed;
     bool crcEnable;
+    bool configCrcEnable;
     void *pCommHandle;
     void *pQACommHandle;
     int32_t (*pFnPmicCommIoRd)(struct Pmic_CoreHandle_s *pmicCorehandle,

diff --git a/include/pmic_core.h b/include/pmic_core.h
@@ -79,6 +79,16 @@ extern "C" {
 #define PMIC_LOCK_ENABLE  ((bool)true)
 /** @} */
 
+/**
+ * @anchor Pmic_ConfigCrcRecalculate
+ * @name PMIC Config CRC recalculate value
+ *
+ * @{
+ */
+#define PMIC_CFG_CRC_RECALCULATE ((bool)true)
+#define PMIC_CFG_CRC_ENABLE_ONLY ((bool)false)
+/** @} */
+
 /**
  * @brief Write a value to a target scratch pad register on the PMIC.
  *
@@ -130,6 +140,35 @@ int32_t Pmic_setRegLockState(Pmic_CoreHandle_t *handle, bool lockState);
  */
 int32_t Pmic_getRegLockState(Pmic_CoreHandle_t *handle, bool *lockState);
 
+/**
+ * @brief Enables config register CRC.
+ *
+ * @param handle    [IN] Pointer to the PMIC core handle structure.
+ * @param calculate [IN] If true (PMIC_CFG_CRC_RECALCULATE), this API will
+ * calculate the new CONFIG_CRC value and write it to CONFIG_CRC_REG_{1,2}
+ * before enabling CONFIG_CRC. If false (PMIC_CFG_CRC_ENABLE_ONLY), this API
+ * will only enable the CONFIG_CRC feature. See @ref Pmic_ConfigCrcRecalculate.
+ *
+ * @warning The `calculate` parameter is currently unimplemented and does
+ * nothing.
+ *
+ * @return Returns PMIC_ST_SUCCESS if the operation is successful; otherwise,
+ * it returns an appropriate error code. For possible values, see @ref
+ * Pmic_ErrorCodes.
+ */
+int32_t Pmic_configCrcEnable(Pmic_CoreHandle_t *handle, bool calculate);
+
+/**
+ * @brief Disables config register CRC.
+ *
+ * @param handle    [IN] Pointer to the PMIC core handle structure.
+ *
+ * @return Returns PMIC_ST_SUCCESS if the operation is successful; otherwise,
+ * it returns an appropriate error code. For possible values, see @ref
+ * Pmic_ErrorCodes.
+ */
+int32_t Pmic_configCrcDisable(Pmic_CoreHandle_t *handle);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

diff --git a/include/regmap/core.h b/include/regmap/core.h
@@ -55,6 +55,15 @@ extern "C" {
 #define PMIC_REGISTER_LOCK_STATUS_SHIFT (0U)
 #define PMIC_REGISTER_LOCK_STATUS_MASK  (1U << PMIC_REGISTER_LOCK_STATUS_SHIFT)
 
+/* CONFIG_CRC_* register definitions */
+#define PMIC_CONFIG_CRC_CONFIG_REG ((uint8_t)0x60U)
+#define PMIC_CONFIG_CRC_REG_1_REG  ((uint8_t)0x44U)
+#define PMIC_CONFIG_CRC_REG_2_REG  ((uint8_t)0x45U)
+
+#define PMIC_CONFIG_CRC_EN_SHIFT     (0U)
+#define PMIC_CONFIG_CRC_CALC_SHIFT   (1U)
+#define PMIC_CONFIG_CRC_STATUS_SHIFT (2U)
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

diff --git a/src/pmic.c b/src/pmic.c
@@ -140,6 +140,54 @@ static int32_t updateSubSysInfoAndValidateComms(const Pmic_CoreCfg_t *config, Pm
     return status;
 }
 
+static int32_t configureDeviceCrc(const Pmic_CoreCfg_t *config, Pmic_CoreHandle_t *handle) {
+    int32_t status = PMIC_ST_SUCCESS;
+
+    // Determine the initial state of the communications CRC, this must be known
+    // in order to configure the CRC state later as the config CRC must be
+    // disabled and the register space needs to be unlocked
+    if (status == PMIC_ST_SUCCESS) {
+        status = Pmic_ioGetCrcEnableState(handle, &handle->crcEnable);
+    }
+
+    // Unconditionally unlock config register space to ensure comms CRC can be
+    // configured
+    if (status == PMIC_ST_SUCCESS) {
+        status = Pmic_setRegLockState(handle, PMIC_LOCK_DISABLE);
+    }
+
+    // Unconditionally disable config CRC to ensure comms CRC can be configured
+    if (status == PMIC_ST_SUCCESS) {
+        status = Pmic_configCrcDisable(handle);
+    }
+
+    // Update PMIC Comms CRC status, note that this performs communications with
+    // the device to ensure handle `crcEnable` property and HW status are in
+    // sync, and this relies on the handle being fully initialized so it must
+    // come after DRV_INIT_SUCCESS.
+    if (Pmic_validParamStatusCheck(config->validParams, PMIC_CFG_CRC_ENABLE_VALID, status)) {
+        status = Pmic_ioSetCrcEnableState(handle, config->crcEnable);
+    }
+
+    // If the user requested that register CRC be enabled, re-enable it here, if
+    // they listed this as a don't care, or explicitly marked it as disabled,
+    // just leave it disabled.
+    if (Pmic_validParamStatusCheck(config->validParams, PMIC_CFG_CFG_CRC_ENABLE_VALID, status)) {
+        handle->configCrcEnable = config->configCrcEnable;
+
+        if (config->configCrcEnable) {
+            status = Pmic_configCrcEnable(handle, PMIC_CFG_CRC_RECALCULATE);
+        }
+    }
+
+    // Re-lock config register space
+    if (status == PMIC_ST_SUCCESS) {
+        status = Pmic_setRegLockState(handle, PMIC_LOCK_ENABLE);
+    }
+
+    return status;
+}
+
 /* ========================================================================== */
 /*                        Interface Implementations                           */
 /* ========================================================================== */
@@ -186,12 +234,9 @@ int32_t Pmic_init(Pmic_CoreHandle_t *handle, const Pmic_CoreCfg_t *config) {
         handle->drvInitStatus |= DRV_INIT_SUCCESS;
     }
 
-    // Update PMIC Comms CRC status, note that this performs communications with
-    // the device to ensure handle `crcEnable` property and HW status are in
-    // sync, and this relies on the handle being fully initialized so it must
-    // come after DRV_INIT_SUCCESS.
-    if (Pmic_validParamStatusCheck(config->validParams, PMIC_CFG_CRC_ENABLE_VALID, status)) {
-        status = Pmic_ioSetCrcEnableState(handle, config->crcEnable);
+    // Configure CRC for HW and PMIC handle
+    if (status == PMIC_ST_SUCCESS) {
+        status = configureDeviceCrc(config, handle);
     }
 
     return status;

diff --git a/src/pmic_core.c b/src/pmic_core.c
@@ -31,6 +31,7 @@
  *
  *****************************************************************************/
 #include <stdint.h>
+#include <stdbool.h>
 #include <stddef.h>
 
 #include "pmic.h"
@@ -134,3 +135,50 @@ int32_t Pmic_getRegLockState(Pmic_CoreHandle_t *handle, bool *lockState)
 
     return status;
 }
+
+int32_t Pmic_configCrcEnable(Pmic_CoreHandle_t *handle, bool calculate)
+{
+    int32_t status = Pmic_checkPmicCoreHandle(handle);
+    uint8_t regData = 0U;
+
+    if ((status == PMIC_ST_SUCCESS) && (calculate == PMIC_CFG_CRC_RECALCULATE)) {
+        // PMICDRV-180
+        // Intentionally do nothing, to be implemented in a future update
+    }
+
+    // Set the CRC_EN bit (only) and write it to the CONFIG_CRC_CONFIG register
+    if (status == PMIC_ST_SUCCESS) {
+        Pmic_setBitField_b(&regData, PMIC_CONFIG_CRC_EN_SHIFT, PMIC_ENABLE);
+
+        Pmic_criticalSectionStart(handle);
+        status = Pmic_ioTxByte(handle, PMIC_CONFIG_CRC_CONFIG_REG, regData);
+        Pmic_criticalSectionStop(handle);
+    }
+
+    // Update the handle
+    if (status == PMIC_ST_SUCCESS) {
+        handle->configCrcEnable = PMIC_ENABLE;
+    }
+
+    return status;
+}
+
+int32_t Pmic_configCrcDisable(Pmic_CoreHandle_t *handle)
+{
+    int32_t status = Pmic_checkPmicCoreHandle(handle);
+
+    // Write 0x00 to the CONFIG_CRC_CONFIG register in order to disable this
+    // feature as described in the TRM.
+    if (status == PMIC_ST_SUCCESS) {
+        Pmic_criticalSectionStart(handle);
+        status = Pmic_ioTxByte(handle, PMIC_CONFIG_CRC_CONFIG_REG, 0x00U);
+        Pmic_criticalSectionStop(handle);
+    }
+
+    // Update the handle
+    if (status == PMIC_ST_SUCCESS) {
+        handle->configCrcEnable = PMIC_DISABLE;
+    }
+
+    return status;
+}