Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read to clear registers should have default hw_precedence #24

Open
jigarsavla opened this issue Feb 4, 2021 · 1 comment
Open

Read to clear registers should have default hw_precedence #24

jigarsavla opened this issue Feb 4, 2021 · 1 comment

Comments

@jigarsavla
Copy link

Hi Scott,

Looks like when we define a read to clear register, it's defaulting to sw_precedence.

These registers should rather default to hw_precedence.

Thanks and keep up the awesome work!
-jigar

@jigarsavla jigarsavla changed the title Read to clear registers should have default hw_precedence Read to clear registers should have default hw_precedence Feb 4, 2021
@berendo
Copy link

berendo commented Feb 4, 2021

To add to @jigarsavla's description, with the default software precedence, if a hardware write occurs on the same cycle that a read does, the set field value (say, one bit flag) is neither returned in the read value, nor captured in the field state. The complete loss of the set value seems to imply that software precedence can never be a valid setting for a RWC field. It might be better to give it special treatment in the Verilog code generator, as appears to be the case for counters, etc.

Here's a snippet of Verilog for a misbehaving register field (rcv_data_valid):

  always_comb begin
    l2d_sbus_csr_core_sbus_result_status_r = 32'b0;
    ...
    l2d_sbus_csr_core_sbus_result_status_r [1]  = rg_sbus_csr_core_sbus_result_status_rcv_data_valid;
    ...
  end
  always_comb begin
    ...
    reg_sbus_csr_core_sbus_result_status_rcv_data_valid_next = rg_sbus_csr_core_sbus_result_status_rcv_data_valid;
    ...
    if (h2l_sbus_csr_core_sbus_result_status_rcv_data_valid_we) reg_sbus_csr_core_sbus_result_status_rcv_data_valid_next = h2l_sbus_csr_core_sbus_result_status_rcv_data_valid_w;
    ...
    if (d2l_sbus_csr_core_sbus_result_status_re) reg_sbus_csr_core_sbus_result_status_rcv_data_valid_next = 1'b0;
  end
  always_ff @ (posedge clk) begin
    rg_sbus_csr_core_sbus_result_status_rcv_data_valid <=  reg_sbus_csr_core_sbus_result_status_rcv_data_valid_next;
    ...
  end

Note that the if (d2l_sbus_csr_core_sbus_result_status_re) ... statement is the last one in the "next state" always_comb block.

sdnellen added a commit that referenced this issue Feb 8, 2021
#24).

Update allows hardware changes to take effect when occuring in same
cycle as rclr/rset (this was already the case for counter fields).  If
previous behavior is needed, 'precedence=hw' should be added to the
rclr/rset field.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants