Skip to content
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

set_latency_timer: allow 0ms duration #71

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Conversation

srwalter
Copy link
Contributor

The datasheet for the FT2232H says:

"Latency Timer. This is really a feature of the driver and is used
to as a timeout to flush short packets of data back to the PC. The
default is 16ms, but it can be altered between 0ms and 255ms. At 0ms
latency you get a packet transfer on every high speed microframe."

I have observed that using 0 results in significantly improved throughput with small packets on the FT2232H.

The datasheet for the FT2232H says:

    "Latency Timer. This is really a feature of the driver and is used
    to as a timeout to flush short packets of data back to the PC. The
    default is 16ms, but it can be altered between 0ms and 255ms. At 0ms
    latency you get a packet transfer on every high speed microframe."

I have observed that using 0 results in significantly improved
throughput with small packets on the FT2232H.
Copy link
Member

@newAM newAM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull-request!

@newAM newAM merged commit a730261 into ftdi-rs:main Oct 19, 2023
14 checks passed
@newAM
Copy link
Member

newAM commented Oct 19, 2023

Created a new release with this fix, 0.32.3

@Farmadupe
Copy link
Contributor

The quoted text from the datasheet for the FT2232H directly conflicts with the following wording in the official API docs:

In the FT8U232AM and FT8U245AM devices, the receive buffer timeout that is used to flush remaining
data from the receive buffer was fixed at 16 ms. In all other FTDI devices, this timeout is programmable
and can be set at 1 ms intervals between 2ms and 255 ms. This allows the device to be better optimized
for protocols requiring faster response times from short data packets.

This forces us to have to decide which official documentation is correct. I suggest the driver docs should overrule the FT2232H datasheet, as at best the datasheet applies to a single device, but the driver applies to every device. Assuming that the API docs are correct, the removal of the above assertion actually creates an unsoundness issue as values below 2 are explicitly forbidden.

Possible solutions may be:

  • Reintroduce range checks (would suggest full assertions rather than debug assertions, as perf is probably not an issue)
  • Mark this function as unsafe
  • Add an unsafe_set_latency_timer function to allow <2ms timeouts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants