pulp-platform / riscv-dbg

RISC-V Debug Support for our PULP RISC-V Cores
Other
219 stars 72 forks source link

Pushing zero into FIFO for SBData0/1 read when sberror != 0 or sbbusyerror !=0 #92

Closed Silabs-ArjanB closed 3 years ago

Silabs-ArjanB commented 3 years ago

What should happen upon a SBData0/1 read when sberror != 0 or sbbusyerror !=0 ?

The FIFO will be pushed no matter if there is an sberror, sbbusyerror or not:

assign resp_queue_push = dmi_req_valid_i & dmi_req_ready_o; However the data that will be pushed (i.e. resp_queue_data) is not set according to the DMI address in case sbbusy_i == 1 :

        dm::SBData0: begin
          // access while the SBA was busy
          if (sbbusy_i) begin
            sbcs_d.sbbusyerror = 1'b1;
          end else begin
            sbdata_read_valid_o = (sbcs_q.sberror == '0);
            resp_queue_data = sbdata_q[31:0];
          end
        end
        dm::SBData1: begin
          // access while the SBA was busy
          if (sbbusy_i) begin
            sbcs_d.sbbusyerror = 1'b1;
          end else begin
            resp_queue_data = sbdata_q[63:32];
          end
        end

(Apart from that it seems that 'sbbusy_i' above should really be 'sbbusy_i || sbcs_q.sbbusyerror') I would think/expect that the resp_queue_data assignments should be done unconditionally instead of in the 'else branches' as is now happening (which will results in pushing 32'b0 into the i_fifo FIFO).

Could you please comment on whether you think the current behavior is correct and/or whether it would be better to move above resp_queue_data assignments outside the else clauses?