fix vendorid and marchid #152
Conversation
| @@ -555,10 +555,10 @@ Machine Architecture ID (marchid) | |||
|
|
|||
| CSR Address: ``0xF12`` | |||
|
|
|||
| Reset Value: ``0x0000_0016`` | |||
| Reset Value: ``CSR_MARCHID_VALUE`` | |||
|
|
|||
| Use the ``CSR_MARCHID_VALUE`` parameter in :file:`rtl/cve2_pkg.sv` to change the fixed value. | |||
There was a problem hiding this comment.
I guess this should now be the MARCHID parameter of the package
There was a problem hiding this comment.
yes but MARCHID_VALUE is equal to MARCHID, see https://github.com/davideschiavone/cve2/blob/fix12/rtl/cve2_pkg.sv#L564
There was a problem hiding this comment.
right, but would you think a user should change MARCHID or CSR_MARCHID_VALUE definition? To me it seems they should use MARCHID
There was a problem hiding this comment.
MARCHID and MARCHID_VALUE are the same value, and it's a read only register. If it is easier for the user to read "35" or MARCHID I can change it - let me know
There was a problem hiding this comment.
my point is, if an integrator wants a different value for MARCHID, this will be done like:
module cve2_top import cve2_pkg::*; #(
parameter MARCHID = 32'd99,
...
)
You essentially added a parameter, which is good, but it was not reflected in the documentation. There shouldn't be a need to modify inside the pkg anymore
There was a problem hiding this comment.
Oh I see, I can move it to localparam , what do you think?
There was a problem hiding this comment.
i think its cool as a parameter, Somebody who instantiates the core might want to have their own value, then this is an easy way
There was a problem hiding this comment.
Ok then as soon as you approve the PR I merge :) thanks
No description provided.