main/i2c: Use magic value to enable/disable PEC

Signed-off-by: Adam Wujek <dev_public@wujek.eu>

main/i2c: Use magic value to enable/disable PEC
Signed-off-by: Adam Wujek <dev_public@wujek.eu>
7e909d6a · Adam Wujek · 835d20b6 · 7e909d6a · 7e909d6a · 7e909d6a
Commit 7e909d6a authored Jul 25, 2022 by Adam Wujek
7 changed files
--- a/common/include/i2c_slave.h
+++ b/common/include/i2c_slave.h
@@ -29,6 +29,9 @@ typedef void(*fp_t)(void);
 /* Defines of Magic numbers */
 #define UC_RESET_MAGIC    0x5A
 #define BOOT_NEW_FW_MAGIC 0xAD
+#define PEC_ON_MAGIC      0x37
+#define PEC_OFF_MAGIC     0x0F
 // QUERY command returned data format
 enum QUERY_RET_VALS {

--- a/doc/source/firmware.rst
+++ b/doc/source/firmware.rst
@@ -676,8 +676,30 @@ USE_PEC
 | Data length: **1**
 The SMBus specification indicates that a device's PEC support could be
-enabled or disabled at will. Using this command with a zero byte
+enabled or disabled at will.
-disables PEC; any non-zero value enables it. The command itself is
+The read gives the information whether the PEC is enabled.
+   +--------+---------------------------+
+   | Value  | Meaning                   |
+   +--------+---------------------------+
+   | 0x00   | PEC disabled              |
+   +--------+---------------------------+
+   | 0x01   | PEC enabled               |
+   +--------+---------------------------+
+To change the state of the PEC, the proper magic value has to be written to
+this register. All other values are silently ignored.
+   +--------+---------------------------+
+   | Value  | Meaning                   |
+   +--------+---------------------------+
+   | 0x0F   | Disable PEC               |
+   +--------+---------------------------+
+   | 0x37   | Enable PEC                |
+   +--------+---------------------------+
+The command itself is
 used without a PEC byte appended, no matter whether the function is
 enabled or not.

--- a/main_fw/src/i2c_impl.c
+++ b/main_fw/src/i2c_impl.c
@@ -360,7 +360,10 @@ void __xMR get_pec(void)
 void __xMR set_pec(void)
 {
-        use_pec = !!use_pec_tmp;
+        if (use_pec_tmp == PEC_ON_MAGIC)
+                use_pec = 1;
+        if (use_pec_tmp == PEC_OFF_MAGIC)
+                use_pec = 0;
 }
 void read_status_b(void)

--- a/pytest/conftest.py
+++ b/pytest/conftest.py
@@ -18,11 +18,16 @@ MONIMOD_UPTIME_ADDR = 0xd7
 MONIMOD_UPTIME_SIZE = 4
 MONIMOD_TMR_ADDR = 0xd8
 MONIMOD_TMR_SIZE = 4
+MONIMOD_PEC_ON_MAGIC  = 0x37
+MONIMOD_PEC_OFF_MAGIC = 0x0F
 class monimod:
    FW_UNKNOWN = 0
    FW_BOOTLOADER = 1
    FW_MAIN = 2
+    PEC_ON_MAGIC  = 0x37
+    PEC_OFF_MAGIC = 0x0F
    cmd_ids = {
        "PAGE"               : 0x00,
@@ -134,7 +139,11 @@ class monimod:
        # the linux driver.
        if self.bus.pec:
            self.bus.pec = False
-        self.pmbus_write_byte_cmd(self.cmd_ids["USE_PEC"], pec_val)
+        if pec_val:
+            pec_magic = self.PEC_ON_MAGIC
+        else:
+            pec_magic = self.PEC_OFF_MAGIC
+        self.pmbus_write_byte_cmd(self.cmd_ids["USE_PEC"], pec_magic)
        # Turn on PEC in Linux i2c driver if PEC was enabled in Monimod
        if pec_val:
            self.bus.pec = True

--- a/pytest/test-00-basic/test_fw_main.py
+++ b/pytest/test-00-basic/test_fw_main.py
@@ -291,7 +291,7 @@ class TestFwMainRegValues(object):
        assert data_new_read != data_old
        assert data_old_read == data_old
-    @pytest.mark.parametrize("cmds", ["USE_PEC", "TC_ONOFF",])
+    @pytest.mark.parametrize("cmds", ["TC_ONOFF",])
    def test_on_off(self, monimod_bus, use_pec, cmds):
        '''Check functionality to off then on by the write to the register.
           A the end restore the old value'''
@@ -300,13 +300,10 @@ class TestFwMainRegValues(object):
        # For PEC testing disable PEC in the Linux i2c driver;
        # Setting PEC in Monimod is possible only with PEC disabled in
        # the driver.
-        if cmds == "USE_PEC" and PEC_ON == pec_map[use_pec]:
-            monimod_bus.bus.pec = False
        old_val = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids[cmds])
        # set to off
        monimod_bus.pmbus_write_byte_cmd(monimod_bus.cmd_ids[cmds], off_val)
-        # let the driver to not use PEC
        off_val_read = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids[cmds])
        assert off_val == off_val_read
@@ -318,11 +315,44 @@ class TestFwMainRegValues(object):
        # restore and verify the old value
        monimod_bus.pmbus_write_byte_cmd(monimod_bus.cmd_ids[cmds], old_val)
        old_val_read = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids[cmds])
+        assert old_val == old_val_read
+    def test_on_off_pec(self, monimod_bus, use_pec):
+        '''Check functionality to off then on by the write to the register.
+           A the end restore the old value'''
+        off_val = 0
+        on_val = 1
+        # For PEC testing disable PEC in the Linux i2c driver;
+        # Setting PEC in Monimod is possible only with PEC disabled in
+        # the driver.
+        if PEC_ON == pec_map[use_pec]:
+            monimod_bus.bus.pec = False
+        old_val = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids["USE_PEC"])
+        # set to off
+        monimod_bus.pmbus_write_byte_cmd(monimod_bus.cmd_ids["USE_PEC"], monimod_bus.PEC_OFF_MAGIC)
+        # let the driver to not use PEC
+        off_val_read = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids["USE_PEC"])
+        assert off_val == off_val_read
+        # set to on
+        monimod_bus.pmbus_write_byte_cmd(monimod_bus.cmd_ids["USE_PEC"], monimod_bus.PEC_ON_MAGIC)
+        on_val_read = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids["USE_PEC"])
+        assert on_val == on_val_read
+        # restore and verify the old value
+        if old_val == off_val:
+            old_val_magic = monimod_bus.PEC_OFF_MAGIC
+        if old_val == on_val:
+            old_val_magic = monimod_bus.PEC_ON_MAGIC
+        monimod_bus.pmbus_write_byte_cmd(monimod_bus.cmd_ids["USE_PEC"], old_val_magic)
+        old_val_read = monimod_bus.pmbus_read_byte_cmd(monimod_bus.cmd_ids["USE_PEC"])
        # Restore PEC usage for Linux i2c driver
-        if cmds == "USE_PEC" and PEC_ON == pec_map[use_pec]:
+        if PEC_ON == pec_map[use_pec]:
            monimod_bus.bus.pec = old_val
        assert old_val == old_val_read
 @pytest.mark.check_uptime
 class TestFwMainOther(object):
    def test_switch_to_bootloader(self, monimod_bus, use_pec):

--- a/test_master/include/i2c_master_mod_fns.h
+++ b/test_master/include/i2c_master_mod_fns.h
@@ -16,6 +16,9 @@
 #define ADDR_TEMP_MATRIX_ROW    0xE1
 #define ADDR_TC_ONOFF           0xE2
+#define PEC_ON_MAGIC      0x37
+#define PEC_OFF_MAGIC     0x0F
 int32_t i2c_m_sync_blk_cmd_write_and_read(struct i2c_m_sync_desc *i2c, uint8_t cmd, uint8_t *wr_buffer, uint8_t wr_length, uint8_t *rd_buffer, uint8_t *rd_length);
 int32_t i2c_m_sync_my_cmd_write(struct i2c_m_sync_desc *i2c, uint8_t cmd, uint8_t *buffer, uint8_t length);

--- a/test_master/src/i2c_master_mod_fns.c
+++ b/test_master/src/i2c_master_mod_fns.c
@@ -483,7 +483,7 @@ int32_t i2c_m_sync_blk_cmd_write(struct i2c_m_sync_desc *i2c, uint8_t cmd, uint8
 int32_t enable_pec(struct i2c_m_sync_desc *i2c)
 {
-        uint8_t wr_pec = 1;
+        uint8_t wr_pec = PEC_ON_MAGIC;
        uint32_t ret;
        use_pec = 0;
@@ -498,7 +498,7 @@ int32_t enable_pec(struct i2c_m_sync_desc *i2c)
 int32_t disable_pec(struct i2c_m_sync_desc *i2c)
 {
-        uint8_t wr_pec = 0;
+        uint8_t wr_pec = PEC_OFF_MAGIC;
        uint32_t ret;
        use_pec = 0;