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

Sayma RTM: hold hmc7043 in reset/mute state during init. #1049

Merged
merged 3 commits into from Jun 5, 2018

Conversation

hartytp
Copy link
Collaborator

@hartytp hartytp commented Jun 5, 2018

Tested on my Sayma. Since implementing this fix, I haven't seen any more runtime crashes (although, serwb still seems unreliable on my board).

This requires the HMC7043 RESET line to be connected in hardware. My board had been reworked to short this to GND. NB connecting the HMC7043 RESET line breaks the Si5324 because they share a reset line.

def __init__(self, platform):

Copy link
Member

Choose a reason for hiding this comment

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

Remove the empty line.

@@ -110,6 +113,8 @@ def __init__(self, platform):
csr_devices = []

self.submodules.crg = CRG(platform)
csr_devices.append("crg")
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the GPIO out core instead of adding this to the CRG? CRG is normally used to clock the local design only.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lack of familiarity with misoc/to the man with a hammer everything is a nail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mind changing this when merging?

@sbourdeauducq
Copy link
Member

This will need a hardware fix (or at least a review) as well, once the RTM FPGA is loaded by the AMC FPGA.
Maybe add a pull-up on the reset line? Though Xilinx FPGAs sometimes have weak pull-ups when unconfigured (depending on the state of certain pins) so maybe nothing needs to be changed.

info!("enabling hmc7043");

unsafe {
csr::crg::hmc7043_rst_write(0);
Copy link
Member

Choose a reason for hiding this comment

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

Indent

Copy link
Member

Choose a reason for hiding this comment

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

And there is no time for the HMC7043 to wreak havoc between the release of its reset line and the SPI write that mutes it?

@hartytp
Copy link
Collaborator Author

hartytp commented Jun 5, 2018

This will need a hardware fix (or at least a review) as well, once the RTM FPGA is loaded by the AMC FPGA.
Maybe add a pull-up on the reset line?

hmmm...yes. That's a good idea. Let's do that.

@sbourdeauducq sbourdeauducq merged commit 988054f into m-labs:master Jun 5, 2018
@hartytp
Copy link
Collaborator Author

hartytp commented Jun 5, 2018

And there is no time for the HMC7043 to wreak havoc between the release of its reset line and the SPI write that mutes it?

Yes, there is. Although I haven't looked on a scope yet to see if this causes problems in practice (that's next on my to do list).

I think that for the next version of Sayma it would be wise to consider adding FETs that can be used to shut down the HMC7043 outputs by cutting off the biasing. I believe that this PR represents the best we can do without HW changes.

@gkasprow
Copy link
Collaborator

gkasprow commented Jun 5, 2018 via email

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.

None yet

3 participants