Improvements/Changes to USB/DeviceFS

I don’t know whether this is the right group to discuss this. Perhaps Bugs or Code review would be better.

After a long long time I am now able to access my FTDI OBD device correctly. Now I am thinking about writing a general FTDI-driver for RISC OS. But …

I was only able to access my device after I wrote a slightly extended USB stack. So I want to discuss whether my modifactions are welcome or whether other ways are preferred.

The problem is the handling of short packets inside the USB stack. DeviceFS in the actual version forces a padding of short packets to full requested lenght. As far as I can see there is a conceptional problem inside DeviceFS with EOF handling (documentation of this feature would be nice) at least inside sleep mode. In case DeviceFS gets only a fraction of the requested bytes it won’t check for EOF at the device driver. The driver has to return 0 Bytes to force a check but this isn’t the case at short packets. If DeviceFS is not sleeping this doesn’t seems to be a problem for the time will come that there is no further data at all and DeviceFS will permanently check.

May be that the padding has been also introduced due to backward compatibility. However due to this padding behaviour I can’t determine the real number of Bytes which have been really sent by the device and I am requiring this. So I extended the USB driver with an additional DeviceFS_CallDevice function which returns me the state and the number od padded bytes of the actual/last transfer and everything is well. I also expanded Return handles to return me the USB stream handle and the Device drivers handle.

I searched a lot but I am not seeing a better way to obtain the information. More elegant would be flag which supresses the padding on demand but I fear this would require a change of DeviceFS and a much more complicated change of the USBdriver.

Please tell me whether this would be ok.

Thomas, I’m not sure if I understand 100% of what you’re saying. However, in the case of input via bulk or interrupt transfers, the USB protocol knows when a transfer has finished. An application that requested the input, normally does so via OS_GBPB 4, which is capable of telling the application that the transfer is shorter than requested, without generating any error (the information is returned in R2 and R3, in different ways). So the solution looks to be as simple as writing RISC OS’s USB stack to convey that information by following its specification.

Have I missed something?

Dave you are describing how it should work but unfortunately it doesn’t seems to do so.

The USB driver internally is aware of short packages and their real length but it isn’t providing this to DeviceFS properly (or better as I would exspect it (and you too according to your remark above)) and so FileSwitch (the master behind the OS_GBPB operations) gets the wrong result.

Only in the case that you are knowing the exact number of Bytes which is following the driver will work as exspected. This is a bit odd and I couldn’t believe it at first. If you are requesting eg 64 Byte and only 2 Bytes are returned by the USB device you will get your 64 Bytes. 62 are padded internally by the USB driver with 0. More worse is the result if you are requesting eg 128 Bytes and 2 Bytes are returned. 62 Bytes are padded with 0 and 64 Bytes with garbage.

98% percent of USB devices/protocols won’t cause trouble for they are either providing a constant length or a multiple of 64 Bytes or you can extract the number of Bytes to follow from an earlier information (like a directory entry etc.). However the FTDI-chips are not behaving in this way.

I tried lots of combinations including noblock DeviceFS option but I never got the right result with OS_GPBP 4 (as you described above) and if looking into the USB code I must say that the code matches my expriences. Better would be to supress the padding within the driver (quite simple by commenting out some lines) but the side effects are subtile.

I used USB driver 0.47 and this was also the version which source was available at that time.

I can’t say I’ve ever looked too deeply into the DeviceFS side of the USB stack, but if what Thomas says is true then it certainly sounds like something that should be fixed. The best way of doing that would probably be to add an extra option that can be specified in the filename special field – that way existing code which relies on/expects the padding won’t be affected by the change.

If an extra option is added that can be specified in the filename special field, and an application calls an earlier USB DeviceFS that doesn’t implement it, will an error be generated?

I’m not sure. But if an error isn’t generated then the device driver can always check the version number of the USBDriver module to check whether the feature is available.

It may not even be necessary to add the special field… there are very few of us who have ever created USB applications, and we are both still active in the world of RISC OS :-) so our drivers can be updated if they are found to work differently.

Thomas, I’ve just spotted this change of yours that’s been submitted to CVS. Now this may just be an indication of my lack of knowledge about DeviceFS, but it looks like your changes to DeviceFSCallDevice_GetHandles aren’t backwards compatible. Old software which is unaware of the new behaviour won’t expect the contents of R5 and R6 to have been changed by the call.

Jeffrey you are right in general. I am mainly doing C programming and so I didn’t thought about the assembler developers. However I am uncertain whether the change will cause trouble to anyone:

1. The function will be used once in every driver (and you know how “much” USB drivers are available …) 2. Drivers written in C won’t cause problems 3. DeviceFSCallDevice_GetHandles is covered by the DeviceFS_CallDevice-SWI documentation and this say that R0-R7 may be changed by the driver. So a programmer should not make any assumptions about preserved registers in certain cases. But I agree this can be regarded in a different way.

So how to continue? Leave my changes as they are or adding an additional DeviceFSCallDevice-operation containing the information and perhaps returning Workspace pointer to accelerate DeviceFS_CallDevice?

I don’t have a problem to change my changes quickly but ROOL has to add the new version and they are busy enough.

I think adding a new CallDevice operation is the right thing to do. I agree that there’s a good chance that your change wouldn’t have affected anyone, but we’ve still got our principle to uphold of not making backwards-incompatible changes unless there’s no other way available.

Ok I shall try to do the changes at weekend if I have time. In the moment I have to sort out some trouble to access the CVS with !CVS of John Tytgat which worked as tried it some months ago however …

Any updates on this Thomas? I’m thinking that I might as well just fix this myself while I work on some of the other USB bugs.

Apart from adding the new GetHandles call I’m thinking of adding at least two new capabilities:

• An option to disable padding
• An option equivalent to USBD_FORCE_SHORT_XFER (which forces the last packet of a transmit IRP to be non-full, even if the data size was an exact multiple of the max packet size)

Both these options will be available in the filename special field, and toggleable on/off via a DeviceFS call for flexibility. Plus I’ll fix the code that adds the padding so that it doesn’t write random garbage or crash the system if the padded area is large enough!

Thomas: My changes to your changes are now in CVS. Basically:

• Your changes to GetHandles have now been split off to a new call, GetHandles2
• I’ve added options to disable RX padding and to enable the USBD_FORCE_SHORT_XFER flag on TX. These options are available both via the filesystem special field and a new DeviceFS call (GetSetOptions).
• Packet padding has been fixed to not pad with garbage or potentially crash the system
• The USBDriver version number has been increased to allow detection of these new features (your change didn’t increase the version number – since the USBDriver, EHCIDriver and OHCIDriver modules all share the same source, they take their version numbers from NetBSD.build.Version file instead of the auto-updated NetBSD.VersionNum file. I don’t expect this to be the last time that someone will forget to manually update the version numbers!
• There’s also a new SWI, USBDriver_Version, which returns the version of the module, providing a potentially easier way for device drivers to check which DeviceFS features are available (it’s also used internally by the HC drivers, but that’s a different story)

There’s more info about the above in the new USB doc

Sorry to answer so late. However the last six weeks had been not my weeks. I was ill three times.

Thank you Jeffrey for implementing the changes! However there is a certain danger: I posted my changes as discussed to ROOL at 2. May (sorry I didn’t manage it to setup a direct CVS account until now due Perl password generation trouble under RISC OS). It seems that they had not been implemented since then. Of course this source is obsolete now and ROOL must take care to forgot this code submission finally.

My FTDI driver is now ready for testing and I shall try to use the lasted module version for it.

Sorry again. Oregano looped for ever after posting so I psoted again. Both entries were recorded. I wanted to delete the second one but I had no rights. So trying to replace the second entry by this text.

Sorry to answer so late. However the last six weeks had been not my weeks. I was ill three times.

No problem. Let me know if you run into any problems with the new module!

Hi Thomas. Sorry we didn’t do anything with your submission sooner. I’ll make a note not to do anything with it now that Jeffrey has implemented an alternative.