From bb2fead71a8a9e9e37cd355adff8886fbf52ec19 Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Sat, 8 Mar 2025 18:35:46 -0800 Subject: [PATCH] Add FAQ to docs --- docs/cpuif/customizing.rst | 2 +- docs/faq.rst | 117 +++++++++++++++++++++++++++++++++++++ docs/index.rst | 1 + docs/limitations.rst | 8 +-- 4 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 docs/faq.rst diff --git a/docs/cpuif/customizing.rst b/docs/cpuif/customizing.rst index a354cbc..dc806c5 100644 --- a/docs/cpuif/customizing.rst +++ b/docs/cpuif/customizing.rst @@ -91,7 +91,7 @@ The easiest way to add your cpuif is via the TOML config file. See the :ref:`peakrdl_cfg` section for more details. Via a package's entry point definition --------------------------------------- +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If you are publishing a collection of PeakRDL plugins as an installable Python package, you can advertise them to PeakRDL using an entry point. This advertises your custom CPUIF class to the PeakRDL-regblock tool as a plugin diff --git a/docs/faq.rst b/docs/faq.rst new file mode 100644 index 0000000..a6ccbdd --- /dev/null +++ b/docs/faq.rst @@ -0,0 +1,117 @@ +Frequently Asked Questions +========================== + +Why isn't there an option for a flat non-struct hardware interface? +------------------------------------------------------------------- +SystemRDL is inherently a very hierarchical language. +For small register blocks, flattening the hardware interface may be acceptable, +but this ends up scaling very poorly as the design becomes larger and has more +complex hierarchy. +Using struct compositions for the hardware interface has the benefit of +preserving conceptual hierarchy and arrays exactly as defined in the original +SystemRDL. + +How do I know I connected everything? Structs are harder to review +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Initially this can be daunting, but fortunately the tool has an option to generate a +flattened hardware interface report upon export. Try using the ``--hwif-report`` +command line option when exporting. This is the easiest way to quickly +understand the structure of the hardware interface. + + + +Why does the tool generate un-packed structs? I prefer packed structs. +---------------------------------------------------------------------- +Packed structs are great when describing vectors that have bit-level structure. +In this tool, the use of un-packed structs is intentional since the hardware +interface is not something that is meant to be bit-accessible. In the case of +the hardware interface structs, using a packed struct is semantically inappropriate. + +... Then how can I initialize the struct? +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +We get this request most often because designers want to initialize the ``hwif_in`` +struct with a simple assignment: + +.. code:: systemverilog + + always_comb begin + hwif_in = '0; + end + +Of course since the struct actually is **unpacked**, this will result in a +compile error which usually leads to the inappropriate assumption that it ought +to be packed. (See this amusing blog post about `X/Y problems `_) + +If your goal is to initialize the packed struct, fortunately SystemVerilog already +has syntax to do this: + +.. code:: systemverilog + + always_comb begin + hwif_in = '{default: '0}; + end + +This is lesser-known syntax, but still very well supported by synthesis +tools, and is the recommended way to handle this. + +... Why are unpacked structs preferred? +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +In the case of the hardware interface ports, unpacked structs help prevent +mistakes that are very easy to make. +Consider the following situation - a designer has a field that sets the following +properties: ``sw=rw; hw=rw; we;``, and wants to assign the hardware input value, +so they erroneously do the following assignment in Verilog: + +.. code:: systemverilog + + assign hwif_in.my_register.my_field = ; + +This is actually a bug since the ``my_field`` member is actually a struct that +has two members: ``we`` and ``next``. If this were a packed struct, this would +silently compile and you would potentially have a bug that may not be noticed +(depending on how thorough the test campaign is). +With an unpacked struct, this gets flagged immediately as a compile error since +the assignment is invalid. + +The designer may have simply forgotten that the field is an aggregate of multiple +members and intended to do the following: + +.. code:: systemverilog + + assign hwif.my_register.my_field.next = ; + assign hwif.my_register.my_field.we = ; + + +The generated output does not match our organization's coding style +------------------------------------------------------------------- +SystemVerilog coding styles vary wildly, and unfortunately there is little +consensus on this topic within the digital design community. + +The output generated by PeakRDL-regblock strives to be as human-readable as possible, +and follow consistent indentation and styling. We do our best to use the most +widely accepted coding style, but since this is a very opinionated space, it is +impossible to satisfy everyone. + +In general, we strive to follow the +`SystemVerilog style guide by lowRISC `_, +but may deviate in some areas if not practical or would impose excessive complexity on the code generator. + + +The lint tool I am using is flagging violations in generated code +----------------------------------------------------------------- +Code linting tools are a great way to check for user-error, flag inconsistencies, +and enforce best-practices within an organization. In many cases, linter tools +may be configured to also enforce stylistic preferences. +Unfortunately just like coding styles, lint rules can often be more +opinionated than practical. + +In general, we will not address lint violations unless they flag actual +structural issues or semantically dangerous code. +Stylistic violations that pose no actual danger to the correctness of the design +will rarely be addressed, especially if the change would add unreasonable +complexity to the tool. + +If you encounter a lint violation, please carefully review and consider waiving +it if it does not pose an actual danger. If you still believe it is a problem, +please let us know by `submitting an issue `_ +that describes the problem. diff --git a/docs/index.rst b/docs/index.rst index c0f773c..b3bc11a 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -62,6 +62,7 @@ Links hwif configuring limitations + faq licensing api diff --git a/docs/limitations.rst b/docs/limitations.rst index 994ca24..2ae08b9 100644 --- a/docs/limitations.rst +++ b/docs/limitations.rst @@ -1,9 +1,9 @@ -Known Issues & Limitations -========================== +Known Limitations +================= Not all SystemRDL features are supported by this exporter. For a listing of -supported properties, see the appropriate property listing page in the following -sections. +supported properties, see the appropriate property listing page in the sections +that follow. Alias Registers