sinara-hw / Sampler

ARTIQ - compatible 8 channel 16bit 1MS/s ADC card in EEM standard
11 stars 1 forks source link

IC19 not populated and IC18/IC19 labels exchanged on silk screen #19

Closed airwoodix closed 3 years ago

airwoodix commented 5 years ago

This might be known already but a quick scan through the issues did not hit. Sorry for the noise if that's the case. If I'm not mistaken, the silk of Sampler v2.2 says IC18 where it should be IC19 (PGIA SR miso LVDS driver). Also, is there a good reason to not populate this driver by default? It's pretty annoying to solder once the EEM connectors are mounted. This is also confusing since the ARTIQ coredevice driver has a get_gains_mu method which won't work without that chip (or am I overlooking something here? I didn't test that). sampler_v2_2_ic18_19

gkasprow commented 5 years ago

You are right, the IC18/IC19 labels are swapped. So the PGIA0-MISO is not connected. The only use case for this signal is verification of IC10/IC11 SPI operation.

airwoodix commented 5 years ago

Thanks for the confirmation. I think it's nice to be able to read back the gain settings for the PGIAs rather than keeping track of them is software. Is there a big cost factor associated with populating these chips in the factory? The chips themselves are well below 1 € for large quantities, aren't they?

hartytp commented 5 years ago

Nb reading back the pgia gain can change it e.g. if it was set to something other than the device db value in a previous kernel.

I did wonder if it’s worth introducing a SR rtio pho with backing state on the FPGA to deal with this, but it’s a bit of a corner case so prob not worth the added complexity.

jordens commented 5 years ago

You can change get_gains_mu so that it doesn't have side effects. See Urukul.get_att_mu. A backing state in the FPGA is not really a solution. E.g. for the really interesting case where you drive the trap RF from Urukul and want your ion to survive a Kasli reboot. It's doable currently and backing state in the FPGA would actively prevent that.

hartytp commented 5 years ago

You can change get_gains_mu so that it doesn't have side effects. See Urukul.get_att_mu.

Aah, nice, I hadn't seen that.

E.g. for the really interesting case where you drive the trap RF from Urukul and want your ion to survive a Kasli reboot. It's doable currently and backing state in the FPGA would actively prevent that.

Point taken. I hadn't considered that use case (and, it wouldn't work for us since we clock our Urukul from Kasli).

gkasprow commented 5 years ago

You cannot read real settings of latches using this line. You can only verify if data coming out of SR is the same as you clock in. Not really useful.

jordens commented 5 years ago

No. You can read out the current state of the shift register just fine without any side effects or prior knowledge. That's good enough and important in practice.

jordens commented 5 years ago

it wouldn't work for us since we clock our Urukul from Kasli

Even there, if it's not a DRTIO satellite the Si5324 code could probably be made non-disruptive (check lock and configuration first). On a satellite the digital hold feature would come in handy.

gkasprow commented 5 years ago

@jordens that is true providing that there was no activity in the shift register after latching its content :)

jordens commented 5 years ago

@gkasprow Yes. We do assume SPI protocol. Then any previous transaction is guaranteed to have ended in raising CS and thus latching from the SR to the outputs ensuring they are the same.

airwoodix commented 5 years ago

To summarize: with Sampler.get_gains_mu implemented like Urukul.get_att_mu, there is a non-invasive way of getting the gain settings, so populating the PGIA-MISO line driver by default would make sense, correct?

jordens commented 5 years ago

Sure. Populating it always made sense. I don't know why it was DNP ed in v2.2.

gkasprow commented 5 years ago

Some of you requested to DNP it. Take into account that you can read the gain settings only once since readout is destructive.

jordens commented 5 years ago

No. Readout does not need to be destructive. See the explanation above. And if there was such a request it should always be tracked as an issue and as a commit message. That allows review and discussion.

gkasprow commented 5 years ago

to make the readout non-destructive, you must connect MOSI with MISO to clock in data that you clock out :)

jordens commented 5 years ago

Again no. There can be an arbitrary number of things/registers in between: Lower cs, clock in zeros while clocking out the current state, then clock that same state which you just read back in while ignoring the zeros coming out, finally raise cs (activating the same state non destructively).

gkasprow commented 5 years ago

@jordens ACK. But you have to make read-write cycles. You cannot read it twice like in a regular SPI IO registers

jordens commented 5 years ago

For each bit written into the shift register one is being read. That's intrinsic to four wire SPI. But yes you need to shift twice the length of the register to do such a nondestructive read.

marmeladapk commented 5 years ago

@gkasprow This buffer is required to check if PGIA works properly in our production tests. We at Creotech are strongly for mounting it.

gkasprow commented 5 years ago

I was requested to DNPit. It was mounted in earlier versions. It's no problem to mount them.

gkasprow commented 3 years ago

swapped labels, IC19 will be mounted