docs/csr/reg: add guide-level documentation.

Also, fix failing doctests.

Author: jfng

amaranth_soc/csr/bus.py

@@ -36,10 +36,8 @@ class Access(enum.Enum):
 
         #: Read-only mode.
         R  = "r"
-
         #: Write-only mode.
         W  = "w"
-
         #: Read/write mode.
         RW = "rw"
 

amaranth_soc/csr/reg.py

@@ -31,9 +31,14 @@ class FieldPort(wiring.PureInterface):
 
     class Access(enum.Enum):
         """Field access mode."""
+
+        #: Read-only mode.
         R  = "r"
+        #: Write-only mode.
         W  = "w"
+        #: Read/write mode.
         RW = "rw"
+        #: Not connected.
         NC = "nc"
 
         def readable(self):
@@ -42,7 +47,7 @@ def readable(self):
             Returns
             -------
             :class:`bool`
-                ``True`` if `self` is equal to :attr:`R` or :attr:`RW`.
+                ``True`` if equal to :attr:`R` or :attr:`RW`.
             """
             return self == self.R or self == self.RW
 
@@ -52,7 +57,7 @@ def writable(self):
             Returns
             -------
             :class:`bool`
-                ``True`` if `self` is equal to :attr:`W` or :attr:`RW`.
+                ``True`` if equal to :attr:`W` or :attr:`RW`.
             """
             return self == self.W or self == self.RW
 
@@ -178,7 +183,7 @@ class Field:
 
     Arguments
     ---------
-    action_cls : :class:`FieldAction` subclass
+    action_cls : subclass of :class:`FieldAction`
         The type of field action to be instantiated by :meth:`Field.create`.
     *args : :class:`tuple`
         Positional arguments passed to ``action_cls.__init__``.
@@ -211,22 +216,16 @@ class FieldAction(wiring.Component):
     Arguments
     ---------
     shape : :ref:`shape-like object <lang-shapelike>`
-        Shape of the field. See :class:`FieldPort.Signature`.
+        Shape of the field.
     access : :class:`FieldPort.Access`
-        Field access mode. See :class:`FieldPort.Signature`.
-    members : iterable of (:class:`str`, :class:`amaranth.lib.wiring.Member`) key/value pairs
-        Signature members. Optional, defaults to ``()``. A :class:`FieldPort.Signature` member
-        named 'port' and oriented as input is always present in addition to these members.
+        Field access mode.
+    members : iterable of (:class:`str`, :class:`amaranth.lib.wiring.Member`) pairs, optional
+        Additional signature members.
 
     Members
     -------
     port : :py:`In(csr.reg.FieldPort.Signature(shape, access))`
         Field port.
-
-    Raises
-    ------
-    :exc:`ValueError`
-        If the key 'port' is used in ``members``.
     """
     def __init__(self, shape, access, members=()):
         members = dict(members)
@@ -247,7 +246,7 @@ class FieldActionMap(Mapping):
     fields : :class:`dict` of :class:`str` to (:class:`Field` or :class:`dict` or :class:`list`)
         Register fields. Fields are instantiated according to their type:
 
-          - a :class:`Field` is instantiated as a :class:`FieldAction` (see :meth:`Field.create`);
+          - a :class:`Field` is instantiated as a :class:`FieldAction`;
           - a :class:`dict` is instantiated as a :class:`FieldActionMap`;
           - a :class:`list` is instantiated as a :class:`FieldActionArray`.
     """
@@ -360,7 +359,7 @@ class FieldActionArray(Sequence):
     fields : :class:`list` of (:class:`Field` or :class:`dict` or :class:`list`)
         Register fields. Fields are instantiated according to their type:
 
-          - a :class:`Field` is instantiated as a :class:`FieldAction` (see :meth:`Field.create`);
+          - a :class:`Field` is instantiated as a :class:`FieldAction`;
           - a :class:`dict` is instantiated as a :class:`FieldActionMap`;
           - a :class:`list` is instantiated as a :class:`FieldActionArray`.
     """
@@ -404,7 +403,7 @@ def __len__(self):
         return len(self._fields)
 
     def flatten(self):
-        """Iterate recursively over the field array.
+        """Recursively iterate over the field array.
 
         Yields
         ------
@@ -423,17 +422,17 @@ def flatten(self):
 
 
 class Register(wiring.Component):
-    _doc_template = """
-    A CSR register.
+    """A CSR register.
 
     Arguments
     ---------
-    fields : :class:`dict` or :class:`list` or :class:`Field`
-        Collection of register fields. If ``None`` (default), a dict is populated from Python
-        :term:`variable annotations <python:variable annotation>`. ``fields`` is used to create
+    fields : :class:`dict` or :class:`list` or :class:`Field`, optional
+        Collection of register fields. If omitted, a dict is populated from Python :term:`variable
+        annotations <python:variable annotation>`. ``fields`` is used to create
         a :class:`FieldActionMap`, :class:`FieldActionArray`, or :class:`FieldAction`,
         depending on its type (:class:`dict`, :class:`list`, or :class:`Field`).
-    {arguments}
+    access : :class:`~.csr.bus.Element.Access`
+        Element access mode.
 
     Members
     -------
@@ -451,11 +450,6 @@ class Register(wiring.Component):
         If ``element.access`` is not writable and at least one field is writable.
     """
 
-    __doc__ = _doc_template.format(arguments="""
-    access : :class:`~.csr.bus.Element.Access`
-        Element access mode.
-    """)
-
     def __init_subclass__(cls, *, access=None, **kwargs):
         if access is not None:
             # TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
@@ -593,22 +587,22 @@ def elaborate(self, platform):
 class Builder:
     """CSR builder.
 
-    A CSR builder can organize a group of :class:`Register`\\ s within an address range and convert
-    it to a :class:`~.memory.MemoryMap` for consumption by other SoC primitives.
+    A CSR builder collects a group of :class:`Register`\\ s within an address range with the goal
+    of producing a :class:`~.memory.MemoryMap` of the resulting layout.
 
     Arguments
     ---------
     addr_width : :class:`int`
         Address width.
     data_width : :class:`int`
         Data width.
-    granularity : :class:`int`
-        Granularity. Optional, defaults to 8 bits.
+    granularity : :class:`int`, optional
+        Granularity. Defaults to 8 bits.
 
     Raises
     ------
     :exc:`ValueError`
-        If ``granularity`` is not a divisor of ``data_width``
+        If ``data_width`` is not a multiple of ``granularity``.
     """
     def __init__(self, *, addr_width, data_width, granularity=8):
         if not isinstance(addr_width, int) or addr_width <= 0:
@@ -668,7 +662,7 @@ def freeze(self):
         self._frozen = True
 
     def add(self, name, reg, *, offset=None):
-        """Add a CSR register.
+        """Add a register.
 
         Arguments
         ---------
@@ -758,7 +752,11 @@ def Index(self, index):
     def as_memory_map(self):
         """Build a memory map.
 
-        .. todo:: explain address/offset conversions
+        If a register was added without an explicit ``offset``, the :ref:`implicit next address
+        <memory-implicit-next-address>` of the memory map is used. Otherwise, the register address
+        is ``offset * granularity // data_width``.
+
+        Registers are added to the memory map in the same order as they were added to the builder.
 
         Returns
         -------
@@ -781,8 +779,6 @@ def as_memory_map(self):
 class Bridge(wiring.Component):
     """CSR bridge.
 
-    .. todo:: extended summary
-
     Arguments
     ---------
     memory_map : :class:`~.memory.MemoryMap`

docs/csr.rst

@@ -1,10 +1,6 @@
 CSR
 ===
 
-.. warning::
-
-   This manual is a work in progress and is seriously incomplete!
-
 .. toctree::
    :maxdepth: 2
 

docs/csr/_images/csr-bus.png

@@ -1,19 +1,76 @@
-CSR field actions
------------------
+CSR fields
+----------
 
-.. warning::
+.. py:module:: amaranth_soc.csr.action
 
-   This manual is a work in progress and is seriously incomplete!
+The :mod:`amaranth_soc.csr.action` module provides built-in :class:`~amaranth_soc.csr.reg.FieldAction` implementations intended for common use cases, which are split in three categories: :ref:`basic fields <csr-action-basic>` for numerical values, :ref:`flag fields <csr-action-flag>` for arrays of bits, and :ref:`reserved fields <csr-action-reserved>` to serve as placeholders for compatibility.
 
-.. py:module:: amaranth_soc.csr.action
+.. _csr-action-basic:
 
-The :mod:`amaranth_soc.csr.action` module provides built-in CSR field actions.
+Basic fields
+============
+
+Such fields are either exclusively writable by a CSR bus initiator (e.g. :class:`W`, :class:`RW`) or the peripheral itself (e.g. :class:`R`). This effectively removes the possibility of a write conflict between a CSR bus initiator and the peripheral.
 
 .. autoclass:: R()
 .. autoclass:: W()
 .. autoclass:: RW()
+
+.. _csr-action-flag:
+
+Flag fields
+===========
+
+Flag fields may be concurrently written by a CSR bus initiator and the peripheral. Each bit of a flag field may be set or cleared independently of others.
+
+Suggested use cases
++++++++++++++++++++
+
+- :class:`RW1C` flags may be used when a peripheral needs to notify the CPU of a given condition, such as an error or a pending interrupt. To acknowledge the notification, the CPU would then write 1 to the flag bit.
+
+- :class:`RW1S` flags may be used for self-clearing bits, such as the enable bit of a one-shot timer. When the timer reaches its maximum value, it would automatically disable itself by clearing its enable bit.
+
+- A pair of :class:`RW1C` and :class:`RW1S` flags may be used to target the same range of bits (e.g. that drives an array of GPIO pins). This allows a CSR bus initiator to set and clear bits in one write transaction (which is guaranteed to be atomic). If a single :class:`RW` field was used instead, a read-modify-write transaction would be needed, and would require locking to insure its atomicity in a multi-tasked environment.
+
 .. autoclass:: RW1C()
 .. autoclass:: RW1S()
+
+.. _csr-action-reserved:
+
+Reserved fields
+===============
+
+Reserved fields may be defined to provide placeholders for past, future or undocumented functions of a peripheral.
+
+Suggested use cases
++++++++++++++++++++
+
+Reserved for future use (as value)
+..................................
+
+A :class:`ResRAWL` field can be used as a placeholder to ensure forward compatibility of software binaries with future SoC revisions, where it may be replaced with a :ref:`basic field <csr-action-basic>`.
+
+The value returned by reads (and written back) must have defined semantics (e.g. a no-op) that can be relied upon in future SoC revisions. When writing to this field, software drivers targeting the current SoC revision must set up an atomic read-modify-write transaction.
+
+Reserved for future use (as flag)
+.................................
+
+If a field is expected to be implemented as a :ref:`flag <csr-action-flag>` in a future SoC revision, it can be defined as a :class:`ResRAW0` field in the current revision to ensure forward compatibility of software binaries.
+
+Software drivers targeting the current SoC revision should ignore the value returned by reads. Writing a value of 0 must be a no-op if the field is implemented in a future SoC revision.
+
+Defined but deprecated
+......................
+
+If a field was deprecated in a previous SoC revision, it can be replaced with a :class:`ResR0WA` field to ensure backward compatibility of software binaries.
+
+The value of 0 returned by reads (and written back) must retain the semantics defined in the SoC revision where this field was deprecated.
+
+Defined but unimplemented
+.........................
+
+If a field is only implemented in some variants of a peripheral, it can be replaced by a :class:`ResR0W0` field in the others.
+
 .. autoclass:: ResRAW0()
 .. autoclass:: ResRAWL()
 .. autoclass:: ResR0WA()

docs/csr/action.rst

@@ -19,7 +19,17 @@ The :mod:`amaranth_soc.csr.bus` module contains primitives to implement and acce
 Introduction
 ============
 
-The CSR bus API provides unopinionated primitives for defining and connecting the *Control and Status Registers* of SoC peripherals, with an emphasis on safety and resource efficiency. It is composed of low-level :ref:`register interfaces <csr-bus-element>`, :ref:`register multiplexers <csr-bus-multiplexer>` that provide access to the registers of a peripheral, and :ref:`bus decoders <csr-bus-decoder>` that provide access to the registers of multiple peripherals.
+Overview
+++++++++
+
+The CSR bus API provides unopinionated primitives for defining and connecting the *Control and Status Registers* of SoC peripherals, with an emphasis on safety and resource efficiency. It is composed of low-level :ref:`register interfaces <csr-bus-element>`, :ref:`multiplexers <csr-bus-multiplexer>` that provide access to the registers of a peripheral, and :ref:`bus decoders <csr-bus-decoder>` that provide access to subordinate bus interfaces.
+
+This diagram shows a CSR bus decoder being used to provide access to the registers of two peripherals:
+
+.. image:: _images/csr-bus.png
+
+Examples
+========
 
 .. _csr-bus-element:
 
@@ -93,16 +103,15 @@ The following example shows a very basic timer peripheral with an 8-bit CSR bus
                ]
                return m
 
-       def __init__(self, name="timer"):
+       def __init__(self):
            super().__init__({
                "csr_bus": In(csr.Signature(addr_width=3, data_width=8)),
            })
 
            self._reg_cnt = self.Cnt()
            self._reg_rst = self.Rst()
 
-           self.csr_bus.memory_map = MemoryMap(addr_width=3, data_width=8, alignment=2,
-                                               name=name)
+           self.csr_bus.memory_map = MemoryMap(addr_width=3, data_width=8, alignment=2)
            self.csr_bus.memory_map.add_resource(self._reg_cnt, size=3, name=("cnt",))
            self.csr_bus.memory_map.add_resource(self._reg_rst, size=3, name=("rst",))
 
@@ -132,8 +141,8 @@ The following example shows a very basic timer peripheral with an 8-bit CSR bus
    >>> timer = BasicTimer()
    >>> for res_info in timer.csr_bus.memory_map.all_resources():
    ...     print(res_info)
-   ResourceInfo(path=(('cnt',),), start=0x0, end=0x4, width=8)
-   ResourceInfo(path=(('rst',),), start=0x4, end=0x8, width=8)
+   ResourceInfo(path=(Name('cnt'),), start=0x0, end=0x4, width=8)
+   ResourceInfo(path=(Name('rst'),), start=0x4, end=0x8, width=8)
 
 
 Registers are always accessed atomically, regardless of their size. Each register is split into chunks according to the CSR bus data width, and each chunk is assigned a consecutive address on the bus.
@@ -264,28 +273,28 @@ In the following example, a CSR decoder provides access to the registers of two
 
 .. testcode::
 
-   timer0 = BasicTimer(name="timer0")
-   timer1 = BasicTimer(name="timer1")
+   timer0 = BasicTimer()
+   timer1 = BasicTimer()
 
    csr_dec = csr.Decoder(addr_width=16, data_width=8)
-   csr_dec.add(timer0.csr_bus, addr=0x0000)
-   csr_dec.add(timer1.csr_bus, addr=0x1000)
+   csr_dec.add(timer0.csr_bus, addr=0x0000, name="timer0")
+   csr_dec.add(timer1.csr_bus, addr=0x1000, name="timer1")
 
 .. doctest::
 
    >>> for res_info in csr_dec.bus.memory_map.all_resources():
    ...     print(res_info)
-   ResourceInfo(path=('timer0', ('cnt',)), start=0x0, end=0x4, width=8)
-   ResourceInfo(path=('timer0', ('rst',)), start=0x4, end=0x8, width=8)
-   ResourceInfo(path=('timer1', ('cnt',)), start=0x1000, end=0x1004, width=8)
-   ResourceInfo(path=('timer1', ('rst',)), start=0x1004, end=0x1008, width=8)
+   ResourceInfo(path=(Name('timer0'), Name('cnt')), start=0x0, end=0x4, width=8)
+   ResourceInfo(path=(Name('timer0'), Name('rst')), start=0x4, end=0x8, width=8)
+   ResourceInfo(path=(Name('timer1'), Name('cnt')), start=0x1000, end=0x1004, width=8)
+   ResourceInfo(path=(Name('timer1'), Name('rst')), start=0x1004, end=0x1008, width=8)
 
 Although there is no functional difference between adding a group of registers directly to a :class:`Multiplexer` and adding them to multiple :class:`Multiplexer`\ s that are aggregated with a :class:`Decoder`, hierarchical CSR buses are useful for organizing a hierarchical design.
 
 If many peripherals are directly served by a single :class:`Multiplexer`, a very large amount of ports will connect the peripheral registers to the multiplexer, and the cost of decoding logic would not be attributed to specific peripherals. With a :class:`Decoder`, only five signals per peripheral will be used, and the logic could be kept together with the peripheral.
 
-Register interface
-==================
+Register interfaces
+===================
 
 .. autoclass:: amaranth_soc.csr.bus::Element.Access()
    :no-members:
@@ -307,8 +316,8 @@ Register interface
 
 .. _csr-bus-interface:
 
-Bus interface
-=============
+Bus interfaces
+==============
 
 .. autoclass:: Signature()
    :no-members: