tgingold-cern / cheby

GNU General Public License v3.0
7 stars 4 forks source link

Add separate write mask port for (read-)write wire fields #34

Closed lorenzschmid closed 7 months ago

lorenzschmid commented 7 months ago

Write masks were added in #16. Similar code between register fields and wire fields was used. Since register fields are assigned in a sequential process, using the same signal name on the right side of the assignment (i.e., the current value) and on the left side (i.e. the new value) is permitted. This is not valid for wires where such a code creates an infinite loop. Instead, when using write masks with read-write wire fields, the input wire value has to be used on the right side of the assignment (i.e., the current value), while the output wire value has to be used on the left side (i.e. the new value).

I.e., having the following cheby file:

memory-map:
  ...
  x-hdl:
    wmask: True
  children:
  ...
  - reg:
      name: wire_rw
      type: unsigned
      width: 32
      access: rw
      preset: 0x00000000
      x-hdl:
        type: wire

Previously, the following VHDL code was generated:

wire_rw_o <= (wire_rw_o and not wr_sel_d0) or (wr_dat_d0 and wr_sel_d0);

As one can see, both sides of the assignment contain the wire_rw_o (_o suffix) signal while the actual input of the wire field, wire_rw_i (_i suffix), is not used. Hence, one has an infinite loop and the input wire signal is erroneously not used to apply the mask.

Originally, a fix proposed here corrected this behavior and generates the following VHDL code:

wire_rw_o <= (wire_rw_i and not wr_sel_d0) or (wr_dat_d0 and wr_sel_d0);

Finally, it was decided to add a dedicated write mask port for all wire fields to which one can write (and if the mask feature is enabled).

tgingold-cern commented 7 months ago

Probably there should be an extra output for the mask, as inputs may be completely unrelated to outputs.

lorenzschmid commented 7 months ago

So instead of applying the write mask to the new data and the input wire, you would propose to simply output write data and write mask as two separate signals?

tgingold-cern commented 7 months ago

Yes. With wire, the user has full control.

If you need the masking as you have implemented it here, there are two choices:

Does it make sense ?

lorenzschmid commented 7 months ago

Thanks for your input. I agree with you that having a separate mask output for write/read-write wires is the cleanest approach, which gives the user full control. I will update this PR.

lorenzschmid commented 7 months ago

I updated the PR by adding write mask ports for wire fieldsto which one can write. These ports have a resolution of a single bit. I also updated the tests accordingly to verify the new behavior.