docs/regmap: General fixes for SystemVerilog regmap package generation

analogdevicesinc / hdl

HDL libraries and projects

https://wiki.analog.com/resources/fpga/docs/hdl

Other

1.53k stars 1.52k forks source link

docs/regmap: General fixes for SystemVerilog regmap package generation #1495

Closed IstvanZsSzekely closed 1 month ago

IstvanZsSzekely commented 1 month ago

PR Description

Updated regmap files to solve some issues with the regmap generation. Used with this branch from doctools to generate the regmaps.

PR Type

[x] Bug fix (change that fixes an issue)
[ ] New feature (change that adds new functionality)
[ ] Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

[ ] I have followed the code style guidelines
[x] I have performed a self-review of changes
[ ] I have compiled all hdl projects and libraries affected by this PR
[ ] I have tested in hardware affected projects, at least on relevant boards
[ ] I have commented my code, at least hard-to-understand parts
[x] I have signed off all commits from this PR
[x] I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
[x] I have not introduced new Warnings/Critical Warnings on compilation
[ ] I have added new hdl testbenches or updated existing ones

gastmaier commented 1 month ago

My only concern is that i thought 0xXXXXXXXX meant unknown reset value for Read and doesn't matter for Write. And also the formulas should be systemverilog expressions? Because it would be too troublesome creating a "Human readable math to systemverilog" converter.

gastmaier commented 1 month ago

Regardless what you choose, can you document at https://github.com/analogdevicesinc/hdl/blob/main/docs/user_guide/ip_cores/creating_new_ip.rst?plain=1#L24 http://analogdevicesinc.github.io/hdl/user_guide/ip_cores/creating_new_ip.html#register-map so we reefer to it later on?

IstvanZsSzekely commented 1 month ago

My only concern is that i thought 0xXXXXXXXX meant unknown reset value for Read and doesn't matter for Write.

No default value for the field means that the value is unknown when it is read, so the default value in the SystemVerilog regmap package is set to 'hXXXXXXXX, which is practically the same. I found out that 0xXXXXXXXX is not handled by the HDL parser, and many of our regmap files don't have default values for fields. This is why I removed them.

IstvanZsSzekely commented 1 month ago

Regardless what you choose, can you document at https://github.com/analogdevicesinc/hdl/blob/main/docs/user_guide/ip_cores/creating_new_ip.rst?plain=1#L24 http://analogdevicesinc.github.io/hdl/user_guide/ip_cores/creating_new_ip.html#register-map so we reefer to it later on?

I'll update these pages.

IstvanZsSzekely commented 1 month ago

And also the formulas should be systemverilog expressions? Because it would be too troublesome creating a "Human readable math to systemverilog" converter.

This is something that I personally forgot. I'll update the all of the regmaps, where the default value has to be calculated and update the package generator as well.