address feedback from Nathanael

Author: Aaron-Hartwig

hdl/ip/vhd/i2c/common/BUCK

@@ -0,0 +1,19 @@
+load("//tools:hdl.bzl", "vhdl_unit", "vunit_sim")
+
+vhdl_unit(
+    name = "i2c_common_pkg",
+    srcs = ["i2c_common_pkg.vhd"],
+    visibility = ['PUBLIC']
+)
+
+vhdl_unit(
+    name = "i2c_cmd_vc",
+    srcs = ["sims/i2c_cmd_vc.vhd", "sims/i2c_cmd_vc_pkg.vhd"],
+    visibility = ['PUBLIC']
+)
+
+vhdl_unit(
+    name = "i2c_target_vc",
+    srcs = ["sims/i2c_target_vc.vhd", "sims/i2c_target_vc_pkg.vhd"],
+    visibility = ['PUBLIC']
+)

hdl/ip/vhd/i2c/i2c_common_pkg.vhd hdl/ip/vhd/i2c/common/i2c_common_pkg.vhd

@@ -6,7 +6,6 @@
 
 library ieee;
 use ieee.std_logic_1164.all;
-use ieee.numeric_std.all;
 
 package i2c_common_pkg is
 
@@ -48,7 +47,7 @@ package i2c_common_pkg is
         op      : op_t;
         addr    : std_logic_vector(6 downto 0);
         reg     : std_logic_vector(7 downto 0);
-        len     : unsigned(7 downto 0);
+        len     : std_logic_vector(7 downto 0);
     end record;
     constant CMD_RESET  : cmd_t := (READ, (others => '0'), (others => '0'), (others => '0'));
 

hdl/ip/vhd/i2c/sims/i2c_cmd_vc.vhd hdl/ip/vhd/i2c/common/sims/i2c_cmd_vc.vhd

@@ -70,11 +70,15 @@ package body i2c_cmd_vc_pkg is
         -- breaking down our type since we can't push enums in VUnit
         is_read     := false when cmd.op = WRITE else true;
         is_random   := true when cmd.op = RANDOM_READ else false;
-        push(msg, is_read);
-        push(msg, is_random);
-        push(msg, cmd.addr);
-        push(msg, cmd.reg);
-        push(msg, cmd.len);
+
+        -- We break down cmd_t into it's primitive types to push them into the message where we will
+        -- pop them off when we receive it in order to reconstruct a cmd_t.
+        -- TODO: implement push/pop for our custom type so we wouldn't have to do this?
+        push(msg, is_read);     -- boolean
+        push(msg, is_random);   -- boolean
+        push(msg, cmd.addr);    -- std_logic_vector
+        push(msg, cmd.reg);     -- std_logic_vector
+        push(msg, cmd.len);     -- std_logic_vector
         send(net, i2c_cmd_vc.p_actor, msg);
     end;
 

hdl/ip/vhd/i2c/sims/i2c_cmd_vc_pkg.vhd hdl/ip/vhd/i2c/common/sims/i2c_cmd_vc_pkg.vhd

@@ -16,11 +16,11 @@ use vunit_lib.sync_pkg.all;
 
 use work.tristate_if_pkg.all;
 
-use work.i2c_peripheral_pkg.all;
+use work.i2c_target_vc_pkg.all;
 
-entity i2c_peripheral is
+entity i2c_target_vc is
     generic (
-        i2c_peripheral_vc   : i2c_peripheral_t
+        i2c_target_vc   : i2c_target_vc_t
     );
     port (
         -- Tri-state signals to I2C interface
@@ -29,7 +29,7 @@ entity i2c_peripheral is
     );
 end entity;
 
-architecture model of i2c_peripheral is
+architecture model of i2c_target_vc is
 
     type state_t is (
         IDLE,
@@ -94,20 +94,20 @@ begin
 
             when START =>
                 event_msg   := new_msg(got_start);
-                send(net, i2c_peripheral_vc.p_actor, event_msg);
+                send(net, i2c_target_vc.p_actor, event_msg);
                 state       <= GET_START_BYTE;
 
             when GET_START_BYTE =>
                 wait on rx_done;
-                if rx_data(7 downto 1) = address(i2c_peripheral_vc) then
+                if rx_data(7 downto 1) = address(i2c_target_vc) then
                     state       <= SEND_ACK;
                     is_read     := rx_data(0) = '1';
                     event_msg   := new_msg(address_matched);
-                    send(net, i2c_peripheral_vc.p_actor, event_msg);
+                    send(net, i2c_target_vc.p_actor, event_msg);
                 else
                     state       <= SEND_NACK;
                     event_msg   := new_msg(address_different);
-                    send(net, i2c_peripheral_vc.p_actor, event_msg);
+                    send(net, i2c_target_vc.p_actor, event_msg);
                 end if;
 
             when GET_BYTE =>
@@ -126,9 +126,9 @@ begin
                     end if;
 
                     if addr_set then
-                        write_word(memory(i2c_peripheral_vc), to_integer(reg_addr_v), rx_data);
+                        write_word(memory(i2c_target_vc), to_integer(reg_addr_v), rx_data);
                         event_msg   := new_msg(got_byte);
-                        send(net, i2c_peripheral_vc.p_actor, event_msg);
+                        send(net, i2c_target_vc.p_actor, event_msg);
                         addr_incr   <= TRUE;
                     else
                         addr_set    <= TRUE;
@@ -160,7 +160,7 @@ begin
             when GET_STOP =>
                 wait until (stop_condition or stop_during_write);
                 event_msg           := new_msg(got_stop);
-                send(net, i2c_peripheral_vc.p_actor, event_msg);
+                send(net, i2c_target_vc.p_actor, event_msg);
                 state               <= IDLE;
                 addr_set            <= FALSE;
                 addr_incr           <= FALSE;
@@ -212,7 +212,7 @@ begin
             tx_bit_count    <= to_unsigned(1, tx_bit_count'length);
         elsif state = SEND_BYTE then
             if tx_bit_count = 0 then
-                txd := read_word(i2c_peripheral_vc.p_buffer.p_memory_ref, natural(to_integer(reg_addr)), 1);
+                txd := read_word(i2c_target_vc.p_buffer.p_memory_ref, natural(to_integer(reg_addr)), 1);
             else
                 txd := '1' & tx_data(7 downto 1);
             end if;

hdl/ip/vhd/i2c/sims/i2c_peripheral.vhd hdl/ip/vhd/i2c/common/sims/i2c_target_vc.vhd

@@ -6,14 +6,14 @@
 
 library ieee;
 use ieee.std_logic_1164.all;
-use ieee.numeric_std.all;
+use ieee.numeric_std_unsigned.all;
 
 library vunit_lib;
     context vunit_lib.vunit_context;
     context vunit_lib.com_context;
     context vunit_lib.vc_context;
 
-package i2c_peripheral_pkg is
+package i2c_target_vc_pkg is
 
     -- Message definitions
     constant got_start          : msg_type_t := new_msg_type("got_start");
@@ -24,7 +24,7 @@ package i2c_peripheral_pkg is
     constant got_byte           : msg_type_t := new_msg_type("got_byte");
     constant got_stop           : msg_type_t := new_msg_type("got_stop");
 
-    type i2c_peripheral_t is record
+    type i2c_target_vc_t is record
         -- private
         p_actor     : actor_t;
         p_buffer    : buffer_t;
@@ -33,51 +33,51 @@ package i2c_peripheral_pkg is
         P_address   : std_logic_vector(6 downto 0);
     end record;
 
-    constant i2c_peripheral_vc_logger : logger_t := get_logger("work:i2c_peripheral_vc");
+    constant i2c_target_vc_logger : logger_t := get_logger("work:i2c_peripheral_vc");
 
-    impure function new_i2c_peripheral_vc (
-        name    : string;
+    impure function new_i2c_target_vc (
+        name    : string    := "I2C_TARGET_VC";
         address : std_logic_vector(6 downto 0) := b"1010101";
-        logger  : logger_t := i2c_peripheral_vc_logger
-    ) return i2c_peripheral_t;
+        logger  : logger_t  := i2c_target_vc_logger
+    ) return i2c_target_vc_t;
 
-    impure function address (i2c_periph: i2c_peripheral_t) return std_logic_vector;
-    impure function buf (i2c_periph: i2c_peripheral_t) return buffer_t;
-    impure function memory (i2c_periph: i2c_peripheral_t) return memory_t;
+    impure function address (i2c_periph: i2c_target_vc_t) return std_logic_vector;
+    impure function buf (i2c_periph: i2c_target_vc_t) return buffer_t;
+    impure function memory (i2c_periph: i2c_target_vc_t) return memory_t;
 
     procedure expect_message (
         signal net              : inout network_t;
-        constant vc             : i2c_peripheral_t;
+        constant vc             : i2c_target_vc_t;
         constant expected_msg   : msg_type_t;
     );
 
     procedure expect_stop (
         signal net  : inout network_t;
-        constant vc : i2c_peripheral_t;
+        constant vc : i2c_target_vc_t;
     );
 
     procedure start_byte_ack (
         signal net      : inout network_t;
-        constant vc     : i2c_peripheral_t;
+        constant vc     : i2c_target_vc_t;
         variable ack    : out boolean;
     );
 
     procedure check_written_byte (
         signal net      : inout network_t;
-        constant vc     : i2c_peripheral_t;
+        constant vc     : i2c_target_vc_t;
         variable data   : std_logic_vector;
-        variable addr   : unsigned;
+        variable addr   : std_logic_vector;
     );
 
 end package;
 
-package body i2c_peripheral_pkg is
+package body i2c_target_vc_pkg is
 
-    impure function new_i2c_peripheral_vc (
-        name    : string;
+    impure function new_i2c_target_vc (
+        name    : string    := "I2C_TARGET_VC";
         address : std_logic_vector(6 downto 0) := b"1010101";
-        logger  : logger_t := i2c_peripheral_vc_logger
-    ) return i2c_peripheral_t is
+        logger  : logger_t  := i2c_target_vc_logger
+    ) return i2c_target_vc_t is
         variable buf : buffer_t;
     begin
         -- I2C can address 256 bytes, so construct an internal buffer to reflect that
@@ -91,24 +91,24 @@ package body i2c_peripheral_pkg is
         );
     end;
 
-    impure function address (i2c_periph: i2c_peripheral_t) return std_logic_vector is
+    impure function address (i2c_periph: i2c_target_vc_t) return std_logic_vector is
     begin
         return i2c_periph.p_address;
     end function;
 
-    impure function buf (i2c_periph: i2c_peripheral_t) return buffer_t is
+    impure function buf (i2c_periph: i2c_target_vc_t) return buffer_t is
     begin
         return i2c_periph.p_buffer;
     end function;
 
-    impure function memory (i2c_periph: i2c_peripheral_t) return memory_t is
+    impure function memory (i2c_periph: i2c_target_vc_t) return memory_t is
     begin
         return i2c_periph.p_buffer.p_memory_ref;
     end function;
 
     procedure expect_message (
         signal net              : inout network_t;
-        constant vc             : i2c_peripheral_t;
+        constant vc             : i2c_target_vc_t;
         constant expected_msg   : msg_type_t;
     ) is
         variable msg        : msg_t;
@@ -121,15 +121,15 @@ package body i2c_peripheral_pkg is
 
     procedure expect_stop (
         signal net      : inout network_t;
-        constant vc     : i2c_peripheral_t;
+        constant vc     : i2c_target_vc_t;
     ) is
     begin
         expect_message(net, vc, got_stop);
     end procedure;
 
     procedure start_byte_ack (
         signal net      : inout network_t;
-        constant vc     : i2c_peripheral_t;
+        constant vc     : i2c_target_vc_t;
         variable ack    : out boolean;
     ) is
         variable msg    : msg_t;
@@ -149,9 +149,9 @@ package body i2c_peripheral_pkg is
 
     procedure check_written_byte (
         signal net      : inout network_t;
-        constant vc     : i2c_peripheral_t;
+        constant vc     : i2c_target_vc_t;
         variable data   : std_logic_vector;
-        variable addr   : unsigned;
+        variable addr   : std_logic_vector;
     ) is
         variable msg    : msg_t;
     begin

hdl/ip/vhd/i2c/sims/i2c_peripheral_pkg.vhd hdl/ip/vhd/i2c/common/sims/i2c_target_vc_pkg.vhd

@@ -1,21 +1,20 @@
 load("//tools:hdl.bzl", "vhdl_unit", "vunit_sim")
-load("//tools:hdl.bzl", "vhdl_unit", "vunit_sim")
 load("//tools:rdl.bzl", "rdl_file")
 
 rdl_file(
-    name = "i2c_core_regs_pkg",
-    src = "i2c_core_regs.rdl",
-    outputs = ["i2c_core_regs_pkg.vhd", "i2c_core_regs.html"],
+    name = "i2c_ctrl_regs_pkg",
+    src = "regs/i2c_ctrl_regs.rdl",
+    outputs = ["i2c_ctrl_regs_pkg.vhd", "i2c_ctrl_regs.html"],
     visibility = ['PUBLIC']
 )
 
 vhdl_unit(
-    name = "i2c_txn_layer",
+    name = "i2c_ctrl_txn_layer",
     srcs = glob([
         "link_layer/*.vhd",
-        "txn_layer/*.vhd",
-        "i2c_common_pkg.vhd"]),
+        "txn_layer/*.vhd",]),
     deps = [
+        "//hdl/ip/vhd/i2c/common:i2c_common_pkg",
         "//hdl/ip/vhd/common:countdown",
         "//hdl/ip/vhd/common:strobe",
         "//hdl/ip/vhd/common:streaming_if_pkg",
@@ -28,10 +27,12 @@ vhdl_unit(
 )
 
 vunit_sim(
-    name = "i2c_txn_layer_tb",
-    srcs = glob(["sims/txn_layer/*.vhd", "sims/*.vhd"]),
+    name = "i2c_ctrl_txn_layer_tb",
+    srcs = glob(["txn_layer/sims/*.vhd"]),
     deps = [
-        ":i2c_txn_layer",
+        ":i2c_ctrl_txn_layer",
+        "//hdl/ip/vhd/i2c/common:i2c_cmd_vc",
+        "//hdl/ip/vhd/i2c/common:i2c_target_vc",
         "//hdl/ip/vhd/vunit_components:basic_stream"
     ],
     visibility = ['PUBLIC'],

hdl/ip/vhd/i2c/BUCK hdl/ip/vhd/i2c/controller/BUCK

@@ -11,7 +11,7 @@ use work.tristate_if_pkg;
 
 use work.i2c_common_pkg;
 
-entity i2c_core is
+entity i2c_ctrl is
     generic (
         CLK_PER_NS  : positive;
         MODE        : mode_t;
@@ -28,25 +28,25 @@ entity i2c_core is
     );
 end entity;
 
-architecture rtl of i2c_core is
+architecture rtl of i2c_ctrl is
 
 begin
 
-    i2c_txn_layer_inst: entity work.i2c_txn_layer
-     generic map(
-        CLK_PER_NS => CLK_PER_NS,
-        MODE => MODE
-    )
-     port map(
-        clk => clk,
-        reset => reset,
-        scl_if => scl_if,
-        sda_if => sda_if,
-        cmd => cmd,
-        cmd_valid => cmd_valid,
-        core_ready => core_ready,
-        tx_st_if => tx_st_if,
-        rx_st_if => rx_st_if
-    );
+    i2c_ctrl_txn_layer_inst: entity work.i2c_ctrl_txn_layer
+        generic map(
+            CLK_PER_NS  => CLK_PER_NS,
+            MODE        => MODE
+        )
+        port map(
+            clk         => clk,
+            reset       => reset,
+            scl_if      => scl_if,
+            sda_if      => sda_if,
+            cmd         => cmd,
+            cmd_valid   => cmd_valid,
+            core_ready  => core_ready,
+            tx_st_if    => tx_st_if,
+            rx_st_if    => rx_st_if
+        );
 
 end architecture;