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

Urukul: support hw rev v1.3 #1170

Closed
wants to merge 1 commit into from
Closed

Conversation

hartytp
Copy link
Collaborator

@hartytp hartytp commented Oct 8, 2018

  • The 0x08 (hardware revisions <=1.2) and 0x09 (hardware revision v1.3) are essentially compatible. The only difference being that the v1.3 revision has an extra clock mux
  • Clocking options for new hw rev: 100MHz oscillator 0x00; SMA 0x01 (oscillator powered down); MMCX 0x02 (oscillator powered down).
  • In the 0x08 CPLD code the clocking was: 0x00 100MHz XO/MMCX (choice by population options); 0x01 SMA (oscillator not disabled by software).
  • We check that the new clocking options aren't used with the old CPLD revision

@hartytp
Copy link
Collaborator Author

hartytp commented Oct 8, 2018

NB tested in the lab using v1.3 hardware, but I don't currently have access to any <=v1.2 hardware for testing

@jordens jordens self-requested a review October 8, 2018 15:33
raise ValueError("Urukul proto_rev mismatch")
if bool(cfg & CFG_CLK_SEL1) \
and proto_rev == STA_PROTO_REV_MATCH[0]:
raise ValueError("Unsupported clocking option")
Copy link
Member

Choose a reason for hiding this comment

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

That's the question. Is it worth trying to unify the cpld gateware over v1.2 and v1.3, not icrement the protocol and ignore the missing functionality on v1.2 or should we be strict and add the complexity of supporting multiple protocol versions and different gateware for different hardware.
I am leaning towards the latter (and documenting the NOP-ness of clk_sel1 on v1.2/earlier).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am leaning towards the latter (and documenting the NOP-ness of clk_sel1 on v1.2/earlier).

"latter" or "former"?

I am leaning towards the latter (and documenting the NOP-ness of clk_sel1 on v1.2/earlier).

Where do you want this documented? Just the constructor?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Sorry, I meant: I am leaning towards keeping the protocol version and documenting the NOP-ness of clk_sel1 on old hardware it in the constructor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, let's do that.

I'll update the PR later today

@jordens
Copy link
Member

jordens commented Oct 8, 2018

quartiq/urukul#5 for the CPLD gateware changes.

@hartytp
Copy link
Collaborator Author

hartytp commented Oct 9, 2018

updated

  • changed the osc/mmcx cfg bit to match the cpld revision
  • sticking with 0x08 cpld revision, no attempt to check for use of unsupported clocking options
  • added docs

Will test on hw in the morning

@jordens
Copy link
Member

jordens commented Oct 10, 2018

This pull request has been rebased via git hub pull rebase. Original pull request HEAD was ff4e25c, new (rebased) HEAD is 08074d5

@jordens jordens closed this Oct 10, 2018
@jordens
Copy link
Member

jordens commented Oct 10, 2018

Thanks.

@hartytp
Copy link
Collaborator Author

hartytp commented Oct 10, 2018

Thanks @jordens, sorry that dragged on a bit more than I'd hoped.

Did you check that on hw? If not, I'll do it now.

@jordens
Copy link
Member

jordens commented Oct 10, 2018

I checked v1.3 with clk_sel=0x00. This would be a nice entry point for CI. The hardware is already set up.

@hartytp
Copy link
Collaborator Author

hartytp commented Oct 10, 2018

Checked all three clocking options with v1.3 hardware. All work as expected, including oscillator power down.

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

2 participants