Skip to content

Commit 362febc

Browse files
robtaylorclaude
andcommitted
Fix documentation issues from review
Address gatecat's review comments on documentation: - docs/using-pin-signatures.rst: * Fix I2C example to use OPEN_DRAIN_STRONG_DOWN (not STRONG_UP) * Add note linking to chipflow-examples for CPU/Wishbone examples - docs/simulation-guide.rst: * Fix input.json format to use wait_for/action (not timestamps) * Update CXXRTL code to use cxxrtl::value (not wire) * Use .get()/.set() methods instead of .curr/.next * Retitle "Simulation Hangs" to "Incomplete Simulation Output" * Clarify that simulation always stops after num_steps - docs/architecture.rst: * Add example showing how to attach simulation models to custom signatures - docs/contributor-pin-signature-internals.rst: * Replace verbose "Summary" section with concise "Key Files" list * Remove redundant feature bullet points 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d72a217 commit 362febc

File tree

4 files changed

+51
-33
lines changed

4 files changed

+51
-33
lines changed

docs/architecture.rst

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,28 @@ Create new interface types:
428428
"custom": Out(BidirIOSignature(4, **kwargs))
429429
})
430430
431+
To attach a simulation model to your custom signature:
432+
433+
.. code-block:: python
434+
435+
from chipflow_lib.platform import SimModel, BasicCxxBuilder
436+
437+
# Define the C++ model
438+
MY_BUILDER = BasicCxxBuilder(
439+
models=[
440+
SimModel('my_custom', 'my_namespace', MyCustomSignature),
441+
],
442+
hpp_files=[Path('design/sim/my_custom_model.h')],
443+
)
444+
445+
# In your custom SimStep
446+
class MySimPlatform(SimPlatform):
447+
def __init__(self, config):
448+
super().__init__(config)
449+
self._builders.append(MY_BUILDER)
450+
451+
See :doc:`simulation-guide` for complete examples of creating custom simulation models.
452+
431453
Custom Steps
432454
~~~~~~~~~~~~
433455

docs/contributor-pin-signature-internals.rst

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -755,24 +755,14 @@ Pydantic's ``TypeAdapter`` provides:
755755
- Type hints for IDE support
756756
- Serialization to JSON-compatible Python dicts
757757

758-
Summary
759-
-------
760-
761-
The ChipFlow annotation architecture provides:
762-
763-
1. **Type-safe metadata** - Pydantic validates all annotations
764-
2. **JSON schema compatibility** - External tools can parse RTLIL annotations
765-
3. **Extensibility** - New annotation types via ``@amaranth_annotate``
766-
4. **Platform independence** - Same metadata consumed by silicon, simulation, software platforms
767-
5. **Compile-time validation** - Errors caught during elaboration, not during synthesis
768-
769-
Key files to study:
758+
Key Files
759+
---------
770760

771761
- ``chipflow_lib/platform/io/annotate.py`` - Core annotation infrastructure
772762
- ``chipflow_lib/platform/io/iosignature.py`` - I/O signature base classes
773763
- ``chipflow_lib/platform/io/signatures.py`` - Concrete signatures and decorators
774-
- ``chipflow_lib/platform/silicon.py`` - Silicon platform port creation
775-
- ``chipflow_lib/platform/software.py`` - Software platform extraction
764+
- ``chipflow_lib/platform/silicon.py`` - Silicon platform consumption
765+
- ``chipflow_lib/platform/software.py`` - Software platform consumption
776766
- ``chipflow_lib/software/soft_gen.py`` - Code generation
777767

778768
See Also

docs/simulation-guide.rst

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -345,17 +345,16 @@ Creating Reference
345345
Input Commands (Optional)
346346
~~~~~~~~~~~~~~~~~~~~~~~~~~
347347

348-
You can provide input commands via ``design/tests/input.json``:
348+
You can provide input commands via ``design/tests/input.json``. To reduce test churn from timing changes, input files use output events as triggers rather than timestamps:
349349

350350
.. code-block:: json
351351
352352
[
353-
{"timestamp": 1000, "type": "gpio_set", "pin": 0, "value": 1},
354-
{"timestamp": 2000, "type": "uart_tx", "data": "test"},
355-
{"timestamp": 3000, "type": "gpio_set", "pin": 0, "value": 0}
353+
{"wait_for": {"type": "uart_rx", "data": ">"}, "action": {"type": "uart_tx", "data": "help\n"}},
354+
{"wait_for": {"type": "uart_rx", "data": "Ready"}, "action": {"type": "gpio_set", "pin": 0, "value": 1}}
356355
]
357356
358-
Models process these commands at the specified timestamps.
357+
See the `mcu_soc example <https://github.com/ChipFlow/chipflow-examples/blob/main/mcu_soc/design/tests/input.json>`_ for a working input.json file.
359358

360359
Customizing Simulation
361360
----------------------
@@ -378,18 +377,18 @@ To add a custom peripheral model:
378377
379378
template<size_t WIDTH>
380379
class my_peripheral_model {
381-
cxxrtl::wire<WIDTH>& output;
382-
cxxrtl::wire<WIDTH>& input;
380+
cxxrtl::value<WIDTH>& output;
381+
cxxrtl::value<WIDTH>& input;
383382
384383
public:
385384
my_peripheral_model(const char* name,
386-
cxxrtl::wire<WIDTH>& o,
387-
cxxrtl::wire<WIDTH>& i)
385+
cxxrtl::value<WIDTH>& o,
386+
cxxrtl::value<WIDTH>& i)
388387
: output(o), input(i) {}
389388
390389
void step(unsigned timestamp) {
391-
// Model behavior
392-
input.next = output.curr;
390+
// Model behavior - use .get() for reading, .set() for writing
391+
input.set(output.get());
393392
}
394393
};
395394
@@ -441,19 +440,23 @@ Performance Tips
441440
Common Issues
442441
-------------
443442

444-
Simulation Hangs
445-
~~~~~~~~~~~~~~~~
443+
Incomplete Simulation Output
444+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
445+
446+
**Symptom**: Simulation completes but expected operations are incomplete
446447

447-
**Symptom**: Simulation runs but never finishes
448+
**Note**: The simulation will always stop after ``num_steps`` clock cycles, regardless of what the design or software is doing. If your firmware hasn't completed by then, you'll see incomplete output.
448449

449450
**Causes**:
451+
- ``num_steps`` too low for the operations being performed
450452
- Firmware stuck in infinite loop
451453
- Waiting for peripheral that never responds
452454

453455
**Solutions**:
454-
- Reduce ``num_steps`` to see how far it gets
455-
- Enable ``DEBUG=1`` and attach debugger
456-
- Add timeout checks in your firmware
456+
- Increase ``num_steps`` in chipflow.toml if legitimate operations need more time
457+
- Enable ``DEBUG=1`` and attach debugger to see where execution is stuck
458+
- Add timeout checks in your firmware to detect hangs
459+
- Use event logging to see how far the simulation progressed
457460

458461
No UART Output
459462
~~~~~~~~~~~~~~

docs/using-pin-signatures.rst

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,11 @@ For Sky130 chips, you can configure the I/O cell drive mode:
111111
112112
from chipflow_lib.platforms import Sky130DriveMode, GPIOSignature
113113
114-
# Use open-drain with strong pull-up for I2C
114+
# Use open-drain with strong pull-down for I2C
115115
super().__init__({
116116
"i2c_gpio": Out(GPIOSignature(
117117
pin_count=2,
118-
sky130_drive_mode=Sky130DriveMode.OPEN_DRAIN_STRONG_UP
118+
sky130_drive_mode=Sky130DriveMode.OPEN_DRAIN_STRONG_DOWN
119119
))
120120
})
121121
@@ -395,8 +395,11 @@ Here's a complete working example combining all concepts:
395395
396396
return m
397397
398+
**Note:** For more advanced examples including CPU cores and Wishbone bus integration, see the `chipflow-examples repository <https://github.com/ChipFlow/chipflow-examples>`_, which contains tested and working SoC designs.
399+
398400
See Also
399401
--------
400402

401403
- :doc:`chipflow-toml-guide` - Configuring your ChipFlow project
402404
- :doc:`platform-api` - Complete platform API including SimPlatform and attach_data
405+
- `ChipFlow Examples <https://github.com/ChipFlow/chipflow-examples>`_ - Complete working examples with CPU and Wishbone bus

0 commit comments

Comments
 (0)