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

QsfpModuleController: SCL stretch detection is too naive for our QSFP control use case #246

Closed
Aaron-Hartwig opened this issue Dec 5, 2024 · 0 comments · Fixed by #248
Closed

Comments

@Aaron-Hartwig
Copy link
Collaborator

The I2C SCL stretch detection is very simple (

// After the delay we know SCL is being stretched if we aren't the ones
// holding it low.
(* fire_when_enabled *)
rule do_sample_scl_stretch(scl_stretch_sample_strobe && scl_in == 0);
scl_stretching <= scl_out_en == 0;
scl_stretch_sample_delay <= False;
endrule
): When the I2C controller is not driving SCL, wait a small delay and see if someone else is holding it low. The problem in the context of our QSFP controller is that each module's power supply is only enabled when a module is installed. That rail also energizes the pull-up resistors on the module's I2C rail, so the SCL line will go low. We erroneously detect this as a SCL stretch.

@Aaron-Hartwig Aaron-Hartwig added this to the 12 milestone Dec 5, 2024
@Aaron-Hartwig Aaron-Hartwig self-assigned this Dec 5, 2024
Aaron-Hartwig added a commit that referenced this issue Dec 9, 2024
This commit fixes a couple of bugs related to SCL stretching. First off,
in the `I2CBitController` I adjusted how we sample SCL to detect
stretching so we're not errantly sampling. Then in the `I2CCore` I added
a mechanism to handle the SCL stretch timeouts when we see them. Before
we lacked this and that meant if that was seen, we were wedged. The rest
of the changes were to test cases in order to mimic the behavior
observed that brought the problems to our attention.

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

Successfully merging a pull request may close this issue.

1 participant