-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SPI Support #125
SPI Support #125
Conversation
@WeierAndreas or @mincrmatt12, is one of you possibly able to test this? I don't currently have an SPI device, and I have not actually tried to run this code in any way so far. |
Alright, so I gave this a go and it didn't seem to work. After running
Separately, the thing wouldn't even build without a few patches to do with Running it with
(*6B00 is indeed SPI_IOC_MESSAGE) but it doesn't seem to be writing them to the file, for whatever reason. |
Hmm, no, that message means that we are not interceptin the ioctl properly. It is the fallback that happens if the code decides to do nothing. The question is what is going on. One thing one could do is adding logging into This is confusing. With the Do you see the |
Sorry for the delay, just tried it again and got the entire log this time, here. It doesn't appear to want to capture the SPI device based on:
This is from running |
Uhh, could you really install the new umockdev version (or set LD_PRELOAD_PATH to include the build directory)? I think what is happening is that you are using the new umockdev-record with the old preload library, and then the result is completely expected. EDIT: You can also double check the right version is loaded. e.g. by using |
Ah, that was the problem. It does appear to record the ioctls now, but trying to play them back fails with an assert:
some |
Hm, need to check the regexp. Could you post the generated .ioctl file for me to look at? The assert likely means that it is not read in correctly by the replayer. |
Alright, here's the first couple hundred lines of it (truncated for privacy reasons): https://static.mm12.xyz/capture.ioctl. I will point out that this is from running my prototype code, not the actual driver (since it was easier to hack out the hid stuff from that), which uses read/write calls in addition to IOC_MESSAGE, but your code looks like it should support that. EDIT: Here's the umockdev file as well: https://static.mm12.xyz/device.umockdev |
Haha, I wrote "%20x" rather than "%02x" and the hex code was space padded to 20 characters rather than zero padded to two characters. I guess fixable using sed, but you can also just re-record, I suppose. Need to check the loading code to fail harder in this case. EDIT: Looking at the code, the problem causing the assertion was a bug in the loader code. |
Alright so after trying this it's now getting stuck on trying to capture a
Adding a debug print inside
|
Hah! To fix alpine (different libc), I needed to force the ioctl request to be a 32bit unsigned integer. Doing that caused the data pointer of the write to be truncated. I pushed a fixup to change the location which will fix the issue. If you provide me with the relevant information (new dump in the correct format, the umockdev file and which code to run), then I can also try it here. Might be simpler as I suspect there will be a few more stupid bugs like this one. |
#124 landed, so this can be rebased now. |
Alright, now recording works and generates files that look sensible. Unfortunately, trying to play them back segfaults somewhere in vala-generated code trying to free something on line 181, while trying to update I've updated the |
OK, so I need to use pointers into the array, otherwise it will copy information … With this, I can replay, but at the end it'll go into an infinite loop, trying to read more data than is in the recording and getting an error. Is that expected or should the application quit at the end of the replay? EDIT: Looking at the program output (without a shitton of debugging), it seems quite expected. |
Yeah, the prototype is supposed to run forever dumping frames; it's not exactly the greatest thing for testing a clean exit. Just tried the latest code, seems to work properly now! Just for reference, I've still had to replace all instances of |
Right, The Though, to be honest, Ubuntu 18 is rather old at this point, and we did drop support for older distributions to some extend. That said, if that is all you need, then it probably should be working. |
The errors are all occurring after valac:
There are also a number of similar failures in |
Woha, that is new in kernel 4.15, I doubt we care about compatibility with that at this point. |
198d10d
to
6c0da6b
Compare
Alright, so I tried using this to replay a capture from
The ioctl file is on the EDIT: I tracked this down to a typo in --- a/src/umockdev-spi.vala
+++ b/src/umockdev-spi.vala
@@ -307,7 +307,7 @@ internal class IoctlSpiHandler : IoctlSpiBase {
if (trans > chunk.rx.length - replay_byte)
trans = chunk.rx.length - replay_byte;
- left = chunk.tx.length - replay_byte - trans;
+ left = chunk.rx.length - replay_byte - trans;
Posix.memcpy(&rx.data[offset], &chunk.rx[replay_byte], trans);
/* We are happy. */ |
3473f60
to
72fc103
Compare
Signals should be emitted on both the IoctlClient and IoctlBase object, but emission on the IoctlBase object was missing.
Thanks @benzea : I didn't review this with a fine comb, but overall it makes sense to me. Not sure if you want to do the |
It really needs to go into here, because it only happens due to the added read/write emulation support. |
This permits overriding the read/write handler for devices. Doing so permits us to e.g. correctly emulate SPI devices which may be accessed both through read/write and ioctl. The default implementation is to fall back, which may e.g. pick the underlying PTY that was created. Note that read/write emulation using this method is not done for the fallback ioctl emulation socket as that would result in uneccessary overhead for script handlers. We also use a fine-grained lock for each of the sockets in order to prevent deadlocks during replay.
The SPI recorder will write "@dev device (SPI)", which allows selecting the SPI replayer when --ioctl is passed.
Dynamically select the SPI recorder if we are passed an SPI device instead of using the generic ioctl tree recorder. Closes: martinpitt#121
@martinpitt OK, we are good to go now (>=5 runs and all clean). Thanks for your patience! The problem was, the following:
The fix is simple:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Danke!
Implement SPI recording and reply with using a new
ioctl
format (recording/replay using the usual ioctl related commands forumockdev-run
andumockdev-record
).This is not tested. Implemented on top of #124.