m-labs / misoc

The original high performance and small footprint system-on-chip based on Migen™
https://m-labs.hk
Other
305 stars 86 forks source link

CSRStorage with write_from_dev=True seems broken in simulation #95

Open mithro opened 5 years ago

mithro commented 5 years ago

My understanding is that asserting csr.we should cause the storage to get the contents from csr.dat_w. This doesn't seem to be happening during simulation.

I also found the following; https://github.com/m-labs/misoc/blob/57ebe119d80beabad30d232fc3c9229882042807/misoc/cores/spi2.py#L496-L497

mithro commented 5 years ago

The issue seems to be that CSRStorage has a def do_finalize method which is never called.

When you add the object as a submodule, the method fails because of a missing "busword" method. If you give the busword a default argument, simulation seems to start working.

mithro commented 5 years ago

I created the following test to demonstrate the issue;

#!/usr/bin/env python3

import unittest

from migen import *

from litex.soc.interconnect.csr import *

class CSRModule(Module, AutoCSR):
    def __init__(self, *args, **kw):
        self.csr = CSRStorage(*args, **kw)

class TestCSRStorage(unittest.TestCase):

    def assert_csr(self, csr, expected):
        actual = yield from csr.read()
        self.assertEqual(expected, actual)

    def stim_csr_write(self, dut, values, mask):
        yield from self.assert_csr(dut.csr, 0)
        for v in values:
            yield from dut.csr.write(v)
            yield from self.assert_csr(dut.csr, v & mask)

    def test_readwrite_nibble(self):
        dut = CSRModule(size=4)
        stim = self.stim_csr_write(dut, [0, 0xff, 0x0b, 0xaa], 0x0f)
        run_simulation(dut, stim, vcd_name="vcd/%s.vcd" %  self.id())

    def test_readwrite_byte(self):
        dut = CSRModule(size=8)
        stim = self.stim_csr_write(dut, [0, 0xff, 0x0b, 0xaa], 0xff)
        run_simulation(dut, stim, vcd_name="vcd/%s.vcd" %  self.id())

    def test_readwrite_32bit(self):
        dut = CSRModule(size=32)
        stim = self.stim_csr_write(dut, [0, 0xffffffff, 0x0a0a0a0a, 0xa0a0a0a0], 0xffffffff),
        run_simulation(dut, stim, vcd_name="vcd/%s.vcd" %  self.id())

    def test_readwrite_byte_alignment(self):
        dut = CSRModule(size=16, alignment_bits=8)
        stim = self.stim_csr_write(dut, [0, 0xffff, 0x0a0a, 0xa0a0], 0xff00),
        run_simulation(dut, stim, vcd_name="vcd/%s.vcd" %  self.id())

    def test_readwrite_from_dev(self):

        dut = CSRModule(size=8, write_from_dev=True)

        def stim(glitch=-1):
            yield from self.assert_csr(dut.csr, 0)
            for v in [0, 0xff, 0x0a, 0xa0]:
                # Write the value from the device
                yield dut.csr.dat_w.eq(v)
                yield dut.csr.we.eq(1)
                yield
                yield dut.csr.we.eq(0)

                # Read the CSR value and check it
                actual = yield from dut.csr.read()
                self.assertEqual(actual, v)

        run_simulation(dut, stim(), vcd_name="vcd/%s.vcd" %  self.id())

if __name__ == '__main__':
    import doctest
    doctest.testmod()
    unittest.main()

It currently fails with;

======================================================================
FAIL: test_readwrite_from_dev (__main__.TestCSRStorage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./test_csr.py", line 64, in test_readwrite_from_dev
    run_simulation(dut, stim(), vcd_name="vcd/%s.vcd" %  self.id())
  File "/home/tansell/github/timvideos/litex-buildenv/third_party/migen/migen/sim/core.py", line 407, in run_simulation
    s.run()
  File "/home/tansell/github/timvideos/litex-buildenv/third_party/migen/migen/sim/core.py", line 396, in run
    self._process_generators(cd)
  File "/home/tansell/github/timvideos/litex-buildenv/third_party/migen/migen/sim/core.py", line 350, in _process_generators
    request = generator.send(reply)
  File "./test_csr.py", line 62, in stim
    self.assertEqual(actual, v)
AssertionError: 0 != 255

----------------------------------------------------------------------
Ran 5 tests in 0.062s

FAILED (failures=1)
mithro commented 5 years ago

If I make the CSRStorage object a submodule of CSRModule and set busword=8, then;

test_readwrite_32bit (__main__.TestCSRStorage) ... FAIL
test_readwrite_byte (__main__.TestCSRStorage) ... FAIL
test_readwrite_byte_alignment (__main__.TestCSRStorage) ... ok
test_readwrite_from_dev (__main__.TestCSRStorage) ... FAIL
test_readwrite_nibble (__main__.TestCSRStorage) ... FAIL