search

lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.52k stars 751 forks source link

[clkmgr] D3 Signoff #22482

Closed andreaskurth closed 1 month ago

moidx commented 3 months ago

Moving to M5 as we are not expecting any further RTL changes as part of M4.

matutem commented 2 months ago

I updated the description to indicate the items in D3 I am able to sign-off at this point.

matutem commented 1 month ago

clkmgr was declared D2S on 18772a9 on Mar 9, 2022. and the current state is at 49fd66a73ae0c3e5f47077f32ba0b7120ed61d30.

The clkmgr code location changed from hw/ip/clkmgr to hw/ip_templates/clkmgr when it was generated by ipgen near commit 94e6ba7

Analysis by checklist item:

NEW_FEATURES_D3

5967df933a814f72d5212bd43d13a0041a3aaf9d [hw] Align HW IP block versions for Earlgrey-PROD ba71a6df80a034020352ccb07a93907cf86d7695 [ipgen,clkmgr] Delete old files 30d7e787c753caaa03fe68a4a70da1bbcbc1d96f Add the project name to the copyright header f575e4b7dc75ec811c5bd868a96b1763ae875476 Make .core files pass FuseSoC 2 schema validator 209e4878a086e03baee9ba4e89641dd701a458ba [ipgen,clkmgr] Generate clkmgr files with ipgen 7615b8fc6add84d799e372462394b05646b55c81 [ipgen,clkmgr] Change templates to use plain dictionaries 6311398984d4eff6ba659ff80f0642329ff8616a [ipgen,clkmgr] Fix core files 09314be4a9b31bbe753cd0085f6ccd48920d1281 [ipgen,clkmgr] Turn a few more files into templates and fix paths d28e33707d4d4cdd0376781cba38dc168867aefc [ipgen,clkmgr] Move template files and remove unused files ab570a2934871fac178c8ae963355677c0d6aba9 [ipgen,clkmgr] Copy all hw/ip/clkmgr files to ip_templates

7c97490c2485fbb5f282d4b1bbc2cdd9586affe7 [clkmgr] Insert hookup buffers on all root clock inputs ab4b36f6b92617ecd67219df8dc1728a0029cae7 [ipgen,rstmgr] Fix paths to rstmgr in md files c721c51c133b75a9c429e7b04eba98b685b3605e [rtl, prim] Add 'commit' functionality to prim_count 61a237e1973d6a29d377c2be871139553c7d344f [util/reggen] reverse order of substruct generation de31bdf1c26be2714ca787c6041a56eb56eb7d1e [reggen] Remove the devmode input af221615474d5ed9bfe6a6bc1f23a3f57fff0a18 [sival,chip_testplans] Handle alert_handler enables tests 3f88a5512095201c1432d5e4bfb22789f35b25b1 [pwrmgr,ipgen] Generate pwrmgr ip_autogen files with topgen 897b51edc45419e94d07ed56effedfc116840315 [sival,clkgr] Add features in clkmgr.hjson.tpl 1b16ca2122d956f4b1f909ba2e996b01ff22001e [reggen] Add mubi support SWAccess that sets/clears a reg 7688e714e847ce2672a7f43fc6e9d0c0f53b5730 [reggen] Add initial support for version and cip_id hjson fields e47df29f3e12a48c62f381a240c8cebf9658b91a [misc] Use lc_tx_t testing functions at endpoints d98b18f186eda61028748a9f9f276cad76dc1058 [rtl/clkmgr] Fix verilint warning 1790498aed071f409b97a0b55c3f8d381a0bb93e [clkmgr] Align signal name with actual usage 2608b59314c2a1ce746e915c55778980bea9853a [reggen] Move prj metadata into main hjson 10d6e41577f6602010c342cf8575cd71f62029f6 [prim_mubi*_sync] Remove clk/rst tie-offs to improve coverage 2b1388ad4b25d9767b18bf7c83e9a6ecbffb7b18 [clkmgr] Add MUBI Synchronizer to Idle e063764dd06f0c61dad3f992cee794d13679c716 [tlul] Add access latency and ifetch to adapter_reg 8202f9f45697287e4d19588745b71c1b067bda73 [CLOCK group] Fixed some unclear clock groups raised in #5305 3fef5173c38588a050a1e7bc062e90719b40a8a0 [sensor_ctrl/clkmgr] update both to use mubi ast_init_done 3452553ef943b7ca107e4e81dbc8a95454c97520 [prim] Assertion update for prim_reg_cdc 0794cfc4a8c1a5cbcd8d94ae3d8664215185afbf [prim_count] This reworks the primitive to make it more generic e7d338784f00bbda505686edf9e8f86161fb25e7 [clkmgr/pwrmgr] Support synchronous resets for lc domain 6f306d3dc604b35c495efdc5332863754565ebdc [RTL] Resolve enum assignment violation 29f760ca206ba7e6f163934acb294e6e40f75e72 [top] Auto generate 4b19b4589da9f75413b4b5203e0f874e16e2c2fb [clkmgr] swap most clkmgr resets to lc root 9a89d46ff17514a0757d46f74b91bd6bea9027d3 [clkmgr] Minor documentation updates eba99e7b938c28e375a4b195e39103f3b9b4b45d [clkmgr] Allow transactional clock groups with just 1 clock f446683d7e76c511ff214a059141b9b6ebb64611 [clkmgr] Fix cdc issues from reg status 68c142b027cd4cca9e16d93e82b69d346dff3d6b [clkmgr] Split clkmgr resets f44a7a7de1a1b0d67827a66b6b3b7739156e9f76 [clkmgr] refactoring measurement clock for readability 3619bb889beebc8e2c14fdd68a9e9f2d3328bf42 [clkmgr] Handle measurement control on calibration lost 6349970862f4c99db7ab7b3b5b2f0a27135bb371 [clkmgr] De-couple idle count from mubi 2e3fe6a51cb52d5f8e6257d44cbc7e19590915b3 [mubi/lc_ctrl] Change MUBI / lc_tx_t encodings 208d34a3e1eda12a07f2c9a246ddccc381792fd6 [reggen] Add spurious WE check to autogen'd regfile 14d44ba1e417776302dccb72f0b5aa53310df812 [clkmgr] External ack readback for external clock switch c62e71d1cb8b09d9fe96f54ecea38dc08bfc70a9 [clkmgr] Connect prim count failure to right alert c83256698562dd4d5ab0fa24e69759139f79b49b [reggen] Move shadow register error aggregation into reg_top e5ec3c5707609d92985b1702e5e711e458afdefd [reggen] Don't generate reg_busy_sel if there are no async regs 75c69734f2e69f0db70d7fefeb0144ec860477ae [clkmgr] Fix build with Xcelium 8fc2a76136cab25c61f3dc75fae48c58e19f8609 [clkmgr] Add missing items from d2s 0d07362907d5dee4337566e149063a7b06b96a80 [clkmgr] Combined shadow error registers into a single field 25d6a536deba189a0551c12fc8f42b1e95b393b9 [clkmgr] clarify documentation for clkmgr

Most of these are dv changes, minor enhancements, or bug fixes. While the following RTL change is a bug fix, it is a major change:

https://github.com/lowRISC/opentitan/commit/4b19b4589da9f75413b4b5203e0f874e16e2c2fb [clkmgr] swap most clkmgr resets to lc root

Prior to this change clkmgr was reset with POR only, and after it the only pieces of the block that are reset by POR are clock dividers and clock status. This means all CSRs are affected by all resets.

The following changes affect CSRs:

TODO_COMPLETE

There are no TODOs in the clkmgr code.

LINT_COMPLETE

There are no lint errors or warnings in clkmgr since 2024-07-22.

There are two lint exclusions in clkmgr related to the prim_clock_gating instantiation using . for its port connections. It is obvious by inspection that the ports in both the generated prim_clock_gating and prim_generic_clock_gating are named the same way and are connected, so . is okay.

Another lint waiver for prim_clock_gating about possibly having empty parameters is strange since all parameters have default values, and neither parameter is even used in this module.

The CLOCK_MUX lint waiver is due to the need for handling the step_down functionality. However, the mux used for the real chip is a library dependent mux specialized for clocks, and is carefully designed for the clocking infrastructure.

CDC_COMPLETE

The CDC checking flow is in flux across the project at the moment, so this item can't really be signed off right now. This was waived for the last tapeout because there was no block-level CDC flow. The best approach might be to waive it again.

RDC_COMPLETE

I do not believe that OpenTitan currently has a block-level RDC flow. As with the last tapeout, this requirement will need to be waived.

REVIEW_RTL

I did an RTL review and found no salient issues.

REVIEW_SW_CHANGE

No software-visible change since this was at D2S.

REVIEW_SW_ERRATA

Not applicable.

andreaskurth commented 1 month ago

Similar to rstmgr, it would be valuable if I could check the RTL diff since the previous signoff.

I tried git diff 18772a998406c1d59f094450607d392604e641b4:hw/ip/clkmgr/rtl HEAD:hw/top_earlgrey/ip_autogen/clkmgr/rtl but that has 5k lines .. did we really change so much? The only major change that you listed above is 4b19b4589da9f75413b4b5203e0f874e16e2c2fb, which seems to change less than 100 lines in clkmgr RTL.

matutem commented 1 month ago

Along the same line of the pwrmgr and rstmgr, for clkmgr the right comparison is between the generated files under hw/top_earlgrey/ip/clkmgr/rtl and the current files. I also updated the comment above to call out the changes affectig CSRs.

andreaskurth commented 1 month ago

Thanks for the details, @matutem. I think this is ready for signoff except that the lint waiver file will have to be approved by TC.

andreaskurth commented 1 month ago

clkmgr.waiver approved by TC