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

add zotino example code #842

Closed
wants to merge 31 commits into from
Closed

add zotino example code #842

wants to merge 31 commits into from

Conversation

mntng
Copy link
Contributor

@mntng mntng commented Oct 26, 2017

No description provided.

@@ -70,9 +68,7 @@ class SPIMaster(Module):
completed transfer.
* If desired, change xfer register for the next transfer.
* If desired, write data queuing the next (possibly chained) transfer.

Copy link
Member

Choose a reason for hiding this comment

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

Why remove those lines?

@@ -368,4 +362,4 @@ def __init__(self, t):
# print(convert(dut))

Tristate.lower = _TestTristate
run_simulation(dut, _test_gen(dut.bus), vcd_name="spi_master.vcd")
run_simulation(dut, _test_gen(dut.bus), vcd_name="spi_master.vcd")
Copy link
Member

Choose a reason for hiding this comment

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

Please look at the patch and remove things like this.

self.setattr_device("ser_config")

@kernel
def dir_config(self, rclk, srclk, ser, data):
Copy link
Member

Choose a reason for hiding this comment

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

I would call those consistently with io_config and with better names, e.g. clk, ser_data, latch.

rclk.off()
delay(dt)
x = 0x80000000
for i in range(32):
Copy link
Member

@sbourdeauducq sbourdeauducq Oct 27, 2017

Choose a reason for hiding this comment

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

There are other shift registers in the system, e.g. for the Novogorny PGIA and the Zotino LEDs, with other lengths. It makes sense to make this function generic to transfer any number of bits (not just 32), document it, and move it into some library (e.g. artiq.coredevice.shiftreg).

Copy link
Member

Choose a reason for hiding this comment

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

And this function could be turned into a device that uses and depends on the TTL devices, just like the DAC driver that depends on a SPI device for the data and a TTL device for LDAC.

"class": "TTLOut",
"arguments": {"channel": 29}
},
"shift_reg": {
Copy link
Member

Choose a reason for hiding this comment

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

those device names could be improved.

@kernel
def run(self):
self.core.reset()
self.fmc_io_config(self, 0x00008800) ## set lvds direction on fmc
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 not going to work.

self.dt = dt

@kernel
def fmc_io_config(self, data):
Copy link
Member

Choose a reason for hiding this comment

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

Shift register + latches are not specific to FMC nor configuring I/O.

@kernel
def run(self):
self.core.reset()
self.shift_reg.shiftreg_config(self, 0x00008800) ## set lvds direction on fmc
Copy link
Member

Choose a reason for hiding this comment

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

Why have self as parameter?

The DDS bus is on channel 32.

The Zotino(3U DAC rev1.0) is connected to kc705 FMC HPC ports using VHDCI board(VHDCI breakout rev1.0) to FMC card(FMC DIO ch32 lvds a v1.2).
On the VHDCI board, J44A is connected to J41 since LVDS2 is now chosen to be connected to J37 on the zotino.
Copy link
Member

Choose a reason for hiding this comment

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

This is not clear. It sounds like both J44A and J41 are on the same board. And which "VHDCI board", the FMC/VHDCI or the VHDCI breakout?

Copy link
Member

Choose a reason for hiding this comment

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

"Name ambiguity due to encoding metadata in the name" strikes again. Nice indication that opaque code names are the way to go and that any approach to make names "intuitive" leads to actual issues with inconsistency, misunderstanding that are way more tangible and hard to relevant than merely complaints à la "can't remember the name".

This configuration supports a Zotino connected to the KC705 FMC HPC through a FMC DIO 32ch LVDS v1.2 and a VHDCI breakout board rev 1.0.
The KC705 FMC HPC should be connected to J44A and the Zotino should be connected to J41 of the VHDCI breadout board.
Copy link
Member

Choose a reason for hiding this comment

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

breakout

@@ -1,3 +1,16 @@
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

Please look at the patches you are proposing.

@@ -11,10 +24,30 @@ def __init__(self, dmgr, clk, ser, latch, n=32, dt=10*us):
self.clk = dmgr.get(clk)
self.ser = dmgr.get(ser)
self.latch = dmgr.get(latch)
>>>>>>> 8407b2c400f4c4565f2504d7bc2b6a8ee70e6918
Copy link
Member

Choose a reason for hiding this comment

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

ditto

This configuration supports a Zotino connected to the KC705 FMC HPC through a FMC DIO 32ch LVDS v1.2 and a VHDCI breakout board rev 1.0.
The KC705 FMC HPC should be connected to J44A and the Zotino should be connected to J41 of the VHDCI breakout board.
Copy link
Member

@sbourdeauducq sbourdeauducq Nov 1, 2017

Choose a reason for hiding this comment

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

The VHDCI breakout board does not have a "J44A" marking on its silkscreen and even less on its future front panel, so referencing it does not make the reader's life easier. You should write this documentation for someone who is setting up their boards the first time, and you should have said the bottom connector.

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