Sayma HMC830: update interface and register writes. #1068

hartytp · 2018-06-11T16:01:19Z

Break the HMC830 init into separate functions for general purpose (but, integer-N) init, setting dividers and checking lock
Use 1.6mA ICP (which the loop filter was optimized for)
Go through the data sheet carefully and set all registers to the correct value (e.g. ensure that all settings are correctly optimized for integer-N usage)
Change divider values (now using 100MHz PFD, which should give lower noise in theory)
Tested at Oxford and all locked reliably well here, but someone else should check this before merging!

jordens

Looks conceptually fine. Haven't tested it.

jordens · 2018-06-11T17:28:06Z

artiq/firmware/libboard_artiq/hmc830_7043.rs

+        Ok(())
+    }
+
+    pub fn set_dividers(r_div: u32, n_div: u32, m_div: u32, out_div: u32) -> Result<(), &'static str> {


This doesn't need a Result<>

jordens · 2018-06-11T17:28:28Z

artiq/firmware/libboard_artiq/hmc830_7043.rs

@@ -113,18 +80,66 @@ mod hmc830 {
    }

    pub fn init() -> Result<(), &'static str> {


This doesn't need a Result<> anymore.

thanks! will fix.

Do you want this all squashed into a single commit at the end?

jordens · 2018-06-11T17:31:12Z

artiq/firmware/libboard_artiq/hmc830_7043.rs

- * fvco = (refclk / r_divider) * n_divider
- * fout = fvco/2
- *
- * HMC7043 config:


That part of the comment shouldn't be removed.

Okay, I'm happy to revert that change.

My thinking was:

* fvco = (refclk / r_divider) * n_divider

that's information is now here, which seems like a better place for it: https://github.com/hartytp/artiq/blob/2621782f8c6bcf05f0e0ca85df42704922e5945b/artiq/firmware/libboard_artiq/hmc830_7043.rs#L113

- * fout = fvco/2 - * - * HMC7043 config:

That information is now here https://github.com/hartytp/artiq/blob/2621782f8c6bcf05f0e0ca85df42704922e5945b/artiq/firmware/libboard_artiq/hmc830_7043.rs#L161-L182 and here https://github.com/hartytp/artiq/blob/2621782f8c6bcf05f0e0ca85df42704922e5945b/artiq/firmware/libboard_artiq/hmc830_7043.rs#L335

Personally, I don't like duplicating this kind of information in comments at the top of the file, since it's easy for people to forget to update them when changing the code.

To check, which bit of the comment do you want back?

* HMC7043 config: * dac clock: 600MHz (div=2) * fpga clock: 150MHz (div=8) * sysref clock: 9.375MHz (div=128)

or this bit

- * fvco = (refclk / r_divider) * n_divider - * fout = fvco/2

Sorry. I meant just the hmc7043 comment should not get lost.

Okay, I'll reintroduce that if you'd prefer.

This still seems a bit redundant to me given https://github.com/hartytp/artiq/blob/ddb47524fb030c88ed4bd82f8dfdbd602ffd7366/artiq/firmware/libboard_artiq/hmc830_7043.rs#L159-L180 What about I just add the frequencies to the comments where the divider values are defined?

Perfect. I just wanted to avoid having to calculate the frequencies.

jordens · 2018-06-11T17:32:03Z

artiq/firmware/libboard_artiq/hmc830_7043.rs

+        write(0x6, 0x307ca); // integer-N mode (NB data sheet table 5.8 not self-consistent)
+        write(0x7, 0x4d);    // digital lock detect, 1/2 cycle window (6.5ns window)
+        write(0x9, 0x2850);  // charge pump: 1.6mA, no offset
+        write(0xa, 0x2045);  // for wideband devices like the HMC830


I guess you are OK with not doing a review of the values, right?

Extra reviews are always welcome. However, in this case I don't think it's necessary. I've checked this all through against the data sheet carefully and tested it on my hardware. I'm inclined to say that if it works reliably on other HW as well then further review isn't worth the time it would take (but if you have spare time then go for it!).

hartytp · 2018-06-11T19:34:49Z

@marmeladapk @jbqubit @sbourdeauducq it would be great if one or more of you could check that this works on your boards before we merge this. Thanks!

jordens · 2018-06-12T08:34:55Z

I'll check this on sayma-2 in HK.

hartytp · 2018-06-12T09:05:55Z

FWIW, I still don't like this https://github.com/hartytp/artiq/blob/2db82806235a2cebe0d1b2650df1c64a48cf61a1/artiq/firmware/libboard_artiq/hmc830_7043.rs#L309-L324 as it goes back to hard-coding divider values. So, if you change SYSREF_DIV that won't actually change the HMC7043 sysref output frequencies. I'll fix that in a commit later on. Plan would probably be to add a general 'hmc7043::configure_channel' function that's called from the hmc803/hmc7043 init function. Then move all the phase/divider values to that init. That puts all the divider values (830 + 7043) in one place, making it easier to follow.

jordens · 2018-06-12T09:09:25Z

Yes. But please don't go overboard with the generalization and interdependency of that code. Abstracting the over the register map and feature sets of those chips won't work.

hartytp · 2018-06-12T09:12:46Z

Yes. But please don't go overboard with the generalization and interdependency of that code. Abstracting the over the register map and feature sets of those chips won't work.

Agreed, I'm keen to keep this as simple as possible.

jordens · 2018-06-12T10:28:32Z

This is your branched merge into master (with cb6e44b)

Gateware ident 4.0.dev+1137.ga9d97101
Initializing SDRAM...
DQS initial delay: 99 taps
Write leveling scan:
Module 3:
00000000000000000000000000000000000000000000000000000000000000000000000000000011001101111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111101010000000100000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000
Module 2:
00000000000000000000000000000000000000000000000000000000000000000001011111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$11111111111111111111111111111111111111111111111111111111111111111111111111111111011001010000011000001000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000010010111011
Module 1:
00000000000000000000000000000000000000000000000000000000000000001011110111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$11111111111111111111111111111111111111111111111111111111111111111111111111011000110000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000010111101110
Module 0:
00000000000000000000000000000000000000000000000000000000000001011101111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$11111111111111111111111111111111111111111111111111111111111111111111111111110111101100000100000000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000010100110111011110111
DQS initial delay: 99 taps
Write leveling: 60 66 69 78 done
Read leveling scan:
Module 3:
00000000000000000000000000000000000000000000000000000000000000001000111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$11111111111111111111110110000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 2:
00000000000000000000000000000000000000000000000001000111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$11111111111101111100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 1:
00000000000000000000000000000000011110001110011111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111010100100$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 0:
00000000000000000000000001111011101111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111011011000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Read leveling: 130+-54 119+-59 101+-53 89+-52 done
SDRAM initialized
Memory test failed (8256/4458496 words incorrect)
Halting.

That's the first memory test failure I have seen in a long time.
IMHO the shift of the window is also weird.
Second attempt:

Gateware ident 4.0.dev+1137.ga9d97101                                       
Initializing SDRAM...
DQS initial delay: 99 taps
Write leveling scan:
Module 3:
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000111111111111111111111111111111111111111111111111111111111111111111
111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111101000000000000000000000000000000000
00000000000000000000000000000000000000000000000000000000000000000
Module 2:
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000011111111111111111111111111111111111111111111111111111111111111111111111111111
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111101000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000
Module 1:
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001011111111111111111111111111111111111111111111111111111111111111111111111111111111$
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110101000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000
Module 0:
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000011111111111111111111111111111111111111111111111111111111111111111111111111111111111111$
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110010100000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000
DQS initial delay: 99 taps
Write leveling: 85 90 95 106 done
Read leveling scan:
Module 3:
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000011001111111111111111111111111111111111111111111111111111111111111111$
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 2:
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000010111111111111111111111111111111111111111111111111111111111111111111111111111111111111$
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111110100100000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 1:
00000000000000000000000000000000000000000000000000000000000000000000000001001111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$
11111111111111111111111111111111111111111111111111111111111111101001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 0:
00000000000000000000000000000000000000000000000000000000000000011111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111$
11111111111111111111111111111111111111111111111101000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000$
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Read leveling: 184+-79 171+-84 151+-77 139+-77 done
SDRAM initialized
Memory test passed

...

[     5.033925s]  INFO(board_artiq::serwb): RTM gateware version 4.0.dev+1137.ga9d97101
[     5.041323s]  INFO(runtime): press 'e' to erase startup and idle kernels...
[     6.041006s]  INFO(runtime): continuing boot
[     6.044069s]  INFO(board_artiq::hmc830_7043::hmc830): HMC830 found
[     6.050135s]  INFO(board_artiq::hmc830_7043::hmc830): loading HMC830 configuration...
[     6.058190s]  INFO(board_artiq::hmc830_7043::hmc830): ...done
[     6.063664s]  INFO(board_artiq::hmc830_7043::hmc830): setting HMC830 dividers...
[     6.071130s]  INFO(board_artiq::hmc830_7043::hmc830): done
[     6.076515s]  INFO(board_artiq::hmc830_7043::hmc830): waiting for HMC830 lock...
[     6.083935s]  INFO(board_artiq::hmc830_7043::hmc830): ...locked
[     6.089801s]  INFO(board_artiq::hmc830_7043::hmc7043): enabling hmc7043
[     6.096683s]  INFO(board_artiq::hmc830_7043::hmc7043): HMC7043 found
[     6.102755s]  INFO(board_artiq::hmc830_7043::hmc7043): loading configuration...
[     6.112105s]  INFO(board_artiq::hmc830_7043::hmc7043):   ...done

hartytp · 2018-06-12T10:31:43Z

Is that reproducible over several loads/power cycles?

hartytp · 2018-06-12T10:33:47Z

That's the first memory test failure I have seen in a long time.
IMHO the shift of the window is also weird.
Second attempt:

Uurgh, that's horrible. But, I can't see how that can be related to any of the firmware changes I made in those two commits. Does it go away if you revert the changes?

Looks like similar symptoms to #1066.

jordens · 2018-06-12T11:05:03Z

I've seen it 3/10 with that firmware. Each time accompanied with a shift of the window.
It can also look really bad:

Gateware ident 4.0.dev+1137.ga9d97101
Initializing SDRAM...
DQS initial delay: 99 taps
Write leveling scan:
Module 3:
00000000000000000000000000000000000000000000000000000010000101000010100111000111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110111111111111111111111110111111111101110100100101111100000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000000000010000100000100101111110111111111111111111
Module 2:
00000000000000000000000000000000000000000000100001111110011111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110111111111111111101101000110000011010010101010001000000001001000000000000000000000000000000000000000000000000000000000000000000000000000001000010000010010000010100011111011111011111111111110001111111111111111111111111111111
Module 1:
00000000000000000000000000000000000000000010001001000111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111010110001110100110000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000111100110000001100001110111111010011010111111111111111111111111111111111111111111111111111111111
Module 0:
00000000000000000000000000000000010001110111111011111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111010010010000000000000000100000000000000000000000000000000000000000000000000000000000000100000000000000000000000000010100110110000010111111001000110101110110111111110111111111111110111111111111111111111111111
DQS initial delay: 99 taps
Write leveling: 30 33 34 52 done
Read leveling scan:
Module 3:
00001101001101010000110100000110001110011111001111111111111111111111110111111111111111111111111110011101111011111110110001011000000001110010100101110011000001000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Module 2:
0011010100011101100001000011001000111

followed by lock up (in the bios! i.e. the corruption in this case is not just sdram)

And 1/10 with a9d9710 (without your changes). Also accompanied with a shift of the window.
Gateware and bios are the same (a9d9710) in both cases.

At least the fact that the firmware doesn't make a difference is reassuring.

jordens · 2018-06-12T11:13:09Z

I'm fine with merging this if you either revert the "...done"/"...locked" change or also change the one on the hmc7043 "...done". And please fix the conflict.

hartytp · 2018-06-12T11:26:36Z

@jordens / @sbourdeauducq can you repeat that without the RTM connected? That will tell us if this is related to any RTM-based noise source.

Anyway, the HMC7043 should be held in the reset state during bios/mem test so it can't be that this time...

jordens · 2018-06-12T12:18:11Z

Thanks. Squashing was the right thing to do.

Sayma HMC830: update interface and register writes.

2621782

jordens requested changes Jun 11, 2018

View reviewed changes

Sayma AMC: remove unnecessary results from hmc830 init code.

ddb4752

Sayma: add frequencies to HMC7043 comments.

2db8280

hartytp added 2 commits June 12, 2018 12:34

Sayma: fix white space in log messages.

a154089

Merge branch 'master' into hmc830_tidy

3c9f64c

hartytp merged commit 7a0140e into m-labs:master Jun 12, 2018

hartytp deleted the hmc830_tidy branch June 12, 2018 11:37

hartytp mentioned this pull request Jun 12, 2018

Sayma: memory corruption #1065

Closed

sbourdeauducq mentioned this pull request Jun 12, 2018

Clocking, DAC support and JESD synchronization on one Sayma card #794

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sayma HMC830: update interface and register writes. #1068

Sayma HMC830: update interface and register writes. #1068

hartytp commented Jun 11, 2018

jordens left a comment

jordens Jun 11, 2018

hartytp Jun 11, 2018

jordens Jun 11, 2018

hartytp Jun 11, 2018

jordens Jun 11, 2018

hartytp Jun 11, 2018 •

edited

hartytp Jun 11, 2018

jordens Jun 11, 2018

hartytp Jun 11, 2018

jordens Jun 12, 2018

hartytp Jun 12, 2018

jordens Jun 11, 2018

hartytp Jun 11, 2018

hartytp commented Jun 11, 2018

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018 •

edited

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018

		@@ -113,18 +80,66 @@ mod hmc830 {
		}

		pub fn init() -> Result<(), &'static str> {

Sayma HMC830: update interface and register writes. #1068

Sayma HMC830: update interface and register writes. #1068

Conversation

hartytp commented Jun 11, 2018

jordens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hartytp Jun 11, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hartytp commented Jun 11, 2018

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018 • edited

jordens commented Jun 12, 2018

hartytp commented Jun 12, 2018

jordens commented Jun 12, 2018

hartytp Jun 11, 2018 •

edited

jordens commented Jun 12, 2018 •

edited