xmos / lib_xud

XMOS USB code and associated examples
Other
8 stars 25 forks source link

XS2 tests are currently disabled #77

Open xross opened 3 years ago

xross commented 3 years ago

XS2 tests are currently disabled - not even built (build config is commented out in test_makefile.mak)

DaveAtkinsonXMOS commented 3 years ago

First pass diagnosis. If you run runtests.py --arch=xs2, there are two failures: one in the code itself, and another in the error handler. The first failure is at line 21 of usb_phy_shim.py in the init for class UsbPhyShim. It is making a call to it's super class, UsbPhy, which I presume has acquired two new arguments in it's argument list: ls0 and ls1. Need to find out what these are to give them a sensible value or remove them. The second failure, in the error handler for the first, is caused by the fact that the xmostest_subprocess.py function 'pstreekill' is being called with a process id (integer) when it expects some sort of 'process' object. I'm guessing this is an object of type multiprocess.Process, and the xmostest_subprocess.py imports multiprocess. Not sure if the fix is to replicate the functionality of the call to pstreekill, as we do not have a Process object, or to find the process object and use it instead of an integer. I don't know yet if the parent uses multiprocess. If not, we'll have to find another way of terminating the children.

mbanth commented 3 years ago

@xross, can you please comment on @DaveAtkinsonXMOS's findings and discuss your thoughts with him?

xross commented 3 years ago

The first issue I think I will have to fix as it will require some knowledge of the XS2 usb shim I'm afraid..

The second one is slightly puzzling.. we should look at whatever the xs3 implementation does and replicate for xs2.

DaveAtkinsonXMOS commented 3 years ago

The second issue is independent of the architecture under test. It's an error in the xmostest error handler. The code's been like that for 3 years as far as I can tell, but presumably there were no errors in the xs2 tests until the xs3 tests came along... As I said, it's purely an xmostest issue (error handler expects an instance of Process, gets passed an integer instead). If we're canning xmostest in favour of pytest, there's presumably little value in trying to fix it? I've raised an issue on tools_xmostest: https://github0.xmos.com/xmos-int/tools_xmostest/issues/42

mbanth commented 3 years ago

It sounds sensible to me to ignore the issue in the xmostest error handler since we will replace xmostest with pytest.

xross commented 3 years ago

Agreed

xross commented 3 years ago

On inspection this task requires some effort to complete due to the more complex (and largely undocumented) nature of the XS2A shim processing vs XS3A.

This processing includes a "do tokens" mode which includes a fairly complete state-machine - the XS3A shim processing has no real state.

A design decision needs to be made as to whether this shim processing is implemented in the xsim or not. This is the ideal situation since it allows a full tests to be authored with register writes to the USB peripheral via the switch etc. This would require careful implementation of the shim to match the XS2A RTL.

A "cheaper" solution (and one the current implementation uses) is to send packet data in a pre-processed form i.e. representing the output of the shim, from the Python test-bench. The drawback of this approach is that the shim essentially resides outside the chip simulation and therefore register writes etc either need to be removed from the test code or the test bench needs to have some sort of Xlink implementation.

There is also a wider question of whether xsim should be an architecture or device simulation, I would lean toward the latter and such the shim should be in xsim.

The latter would require significant work, the former requires less work but will ideally be thrown away and replaced by the former at some point.

Considering all of the above a decision has currently been made to de-prioritise this XS2A work in favour of XS3A related items. In addition, COVID related operational issues effecting XS2A further backup into the decision to de-prioritise XS2A work at this time.