Address PR feedback

hdl/ip/vhd/i2c/io_expanders/PCA9506ish/pca9506_pkg.vhd

@@ -30,6 +30,13 @@ package pca9506_pkg is
         reg_mask: io_type
     ) return std_logic;
 
+    -- PCA9506 defines a category as a set of contiguous registers of the
+    -- same function for each of the  5 ports.  When auto-incrementing is
+    -- enabled, the pointer will wrap around to the first register of the
+    -- category when it reaches the last register of the category.
+    -- Because there  are 5 ports, there are 5 registers in each category
+    -- so we can't just use a power of 2 that wraps and we wrote this 
+    -- function.
     function category_wrapping_increment(
         pointer: std_logic_vector(5 downto 0) -- register number
     ) return std_logic_vector;

hdl/ip/vhd/i2c/muxes/PCA9545ish/pca9545ish_function.vhd

@@ -29,7 +29,7 @@ entity pca9545ish_function is
         reset : in std_logic;
         mux_reset: in std_logic;
         mux_sel : out std_logic_vector(1 downto 0);
-        other_mux_selected : in std_logic;
+        allowed_to_enable : in std_logic;
         -- instruction interface
         inst_data : in std_logic_vector(7 downto 0);
         inst_valid : in std_logic;
@@ -50,10 +50,10 @@ end entity;
 architecture rtl of pca9545ish_function is
     function is_valid(
         data : std_logic_vector(7 downto 0);
-        other_mux_selected : std_logic) return boolean is
+        allowed_to_enable : std_logic) return boolean is
     begin
-        -- if another mux is selected, only writes of 0 are allowed
-        if other_mux_selected = '1' and data /= "00000000" then
+        -- allow only writes of 0 even when we're not allowed to enable
+        if allowed_to_enable = '0' and data /= 0 then
             return false;
         end if;
         -- only allow clear and one-hot bits 0-2
@@ -77,7 +77,7 @@ architecture rtl of pca9545ish_function is
 begin
 
     inst_ready <= '1';  -- never block writes
-    resp_valid <= is_our_transaction; -- never block reads
+    resp_valid <= '1'; -- never block reads
     resp_data <= pack(control_reg);  -- Only one register to read so hand it back always
 
     -- register block
@@ -91,7 +91,7 @@ begin
             if mux_reset then
                 control_reg <= rec_reset;
             elsif inst_valid = '1' and inst_ready = '1' 
-                    and is_valid(inst_data, other_mux_selected) 
+                    and is_valid(inst_data, allowed_to_enable) 
                     and is_valid_write(txn_header)
                     and is_our_transaction = '1' then
                 control_reg <= unpack(inst_data);
@@ -128,7 +128,7 @@ begin
             if in_ack_phase_last = '1' and in_ack_phase = '0' then
                 ack_next <= '0';
             -- Ack on valid data when we're in the middle of a transaction (and we're being addressed)
-            elsif inst_valid = '1' and inst_ready = '1' and is_valid(inst_data, other_mux_selected) and is_our_transaction = '1' then
+            elsif inst_valid = '1' and inst_ready = '1' and is_valid(inst_data, allowed_to_enable) and is_our_transaction = '1' then
                 ack_next <= '1';
             -- Ack on the address byte when we're in the start of a transaction and we're being addressed
             elsif txn_header.tgt_addr = i2c_addr and txn_header.valid = '1' and  is_our_transaction = '0' then

hdl/ip/vhd/i2c/muxes/PCA9545ish/pca9545ish_top.vhd

@@ -40,7 +40,7 @@ entity pca9545ish_top is
 
         -- Signal to reset the mux-state out of band if desired.
         mux_reset: in std_logic;
-        other_mux_selected: in std_logic := '0';
+        allowed_to_enable: in std_logic;
         -- I2C bus mux endpoint for control
         -- Does not support clock-stretching
         scl : in std_logic;
@@ -103,7 +103,7 @@ begin
         reset => reset,
         mux_reset => mux_reset,
         mux_sel => mux_sel,
-        other_mux_selected => other_mux_selected,
+        allowed_to_enable => allowed_to_enable,
         inst_data => inst_data,
         inst_valid => inst_valid,
         inst_ready => inst_ready,

hdl/ip/vhd/i2c/muxes/PCA9545ish/sims/i2c_pca9545ish_th.vhd

@@ -41,7 +41,7 @@ architecture th of i2c_pca9545ish_th is
     signal mux_reset : std_logic := '0';
     signal mux0_sel : std_logic_vector(1 downto 0);
     signal mux1_sel : std_logic_vector(1 downto 0);
-    signal other_mux_selected : std_logic_vector(1 downto 0);
+    signal allowed_to_enable : std_logic_vector(1 downto 0);
 
 begin
 
@@ -62,8 +62,9 @@ begin
     i2c_bus_scl <= i2c_bus_scl_o when i2c_bus_scl_oe = '1' else 'H';
     i2c_bus_sda <= i2c_bus_sda_o when i2c_bus_sda_oe = '1' else 'H';
 
-    other_mux_selected(0) <= '1' when mux1_sel /= "11" else '0';
-    other_mux_selected(1) <= '1' when mux0_sel /= "11" else '0';
+    -- "11" is de-selected
+    allowed_to_enable(0) <= '1' when mux1_sel = "11" else '0';
+    allowed_to_enable(1) <= '1' when mux0_sel = "11" else '0';
 
     DUT0: entity work.pca9545ish_top
      generic map(
@@ -74,7 +75,7 @@ begin
         clk => clk,
         reset => reset,
         mux_reset => mux_reset,
-        other_mux_selected => other_mux_selected(0),
+        allowed_to_enable => allowed_to_enable(0),
         scl => tgt_scl(0),
         scl_o => tgt_scl_o(0),
         scl_oe => tgt_scl_oe(0),
@@ -93,7 +94,7 @@ begin
         clk => clk,
         reset => reset,
         mux_reset => mux_reset,
-        other_mux_selected => other_mux_selected(1),
+        allowed_to_enable => allowed_to_enable(1),
         scl => tgt_scl(1),
         scl_o => tgt_scl_o(1),
         scl_oe => tgt_scl_oe(1),