search

sinara-hw / Fastino

Fast 32-channel, 3MS/s per channel, 16bit DAC EEM card compatible with Zotino
11 stars 2 forks source link

v1.2 design review #81

Closed hartytp closed 3 years ago

hartytp commented 3 years ago

@pathfinder49 can you confirm that you've addressed all of Greg's issues? If so, @gkasprow please could you have a final review of @pathfinder49's pr and merge? Once that's done and https://github.com/sinara-hw/Fastino/issues/74 is fixed we can move to the v1.2 release

hartytp commented 3 years ago

@gkasprow: @pathfinder49 has now fixed all issues you raised. Would you mind reviewing his PR and merging?

gkasprow commented 3 years ago

@pathfinder49 did you repour the polygons after modifying the rules? The DRC finds a lot of errors obraz Rest of the issues are fixed so I'm working with the right PCB file.

pathfinder49 commented 3 years ago

I did. Must have forgotten to re-check the rules. The errors are not fixed by re-pouring the polygons.

pathfinder49 commented 3 years ago

The only rule change I made was to enable the rule you pointed out. I believe these traces are unchanged from V1.0. I think the rule was already disabled in V1.0.

hartytp commented 3 years ago

@gkasprow what's the best way of fixing these issues? It sounds like they are not related to @pathfinder49's changes. Is it easiest if you fix them, or would you prefer @pathfinder49 to do it?

jordens commented 3 years ago

One note: It may make sense to increase the analog bandwidth. I don't think it matches the original assumptions of sample rate.

hartytp commented 3 years ago

IIRC the filter response was more chosen on the basis of keeping the noise down in the regime where we expect our motional modes to be (particularly during splitting/merging) that because of concern about image frequencies. @dnadlinger will have thought more about this than I did.

gkasprow commented 3 years ago

@hartytp rules in ADI are sometimes tricky. I will take care of that.

hartytp commented 3 years ago

Thanks!

To check we're on the same page, can you merge the PR and then run DRC/ERC and do a final review for manufacture.

dnadlinger commented 3 years ago

@jordens: Which type/parameters of an one-op-amp filter would you consider instead? The current filter response is somewhat incidental (see old discussions).

jordens commented 3 years ago

The noise considerations depend a lot on what you actually have in terms of low secular frequencies, how long you are there and what your RF bypass does. If you get into the 500 kHz range then increasing the filter corner won't affect noise during those operations because that's already there with the current design.

@dnadlinger Looking at your nice current filter (#4) it won't be a big change (assuming we don't want to change the topology and order). With Nyquist at 1.3 MHz a transition band from 0.75 (-3 dB) to 1.8 MHz (maybe around -30 dB) might be doable. More or less scaling the frequency response by 1.5, giving 50% more bandwidth. As to the noise at the ion, I agree with your considerations in #4. If by chance or skill that response dip stays close to 2.55 MHz, even better Since the noise measured (#51) matches your design (#4) reasonably well the overall approach looks good.

NB a simple noise generator (XORSHIFT or something else) on Fastino with configurable scaling in powers of two would be a nice tool. There is one in redpid that could be copied.

dnadlinger commented 3 years ago

Actually designing this is a bit non-trivial, as the GBW of the OPA197 is only 10 MHz. Just scaling the values gave this, with a -3 dB bandwidth of -630 kHz:

Quick filter modification

Since I don't think we would want to change the topology much without good reason (as considerable effort has gone into fine-tuning the layout), it should be entirely feasible to just optimise the response numerically (i.e. taking the op-amp models into account in SPICE). It would be nice if somebody could look into this, but I don't have the bandwidth right now to think much about what sensible optimisation goals would be.

hartytp commented 3 years ago

@gkasprow other than the filter, is this all complete?

gkasprow commented 3 years ago

Yes, I already fixed the issue with DIFF pairs clearance

hartytp commented 3 years ago

@gkasprow have you completed your final design review? Are all of @pathfinder49's changes incorporated (if so, please close the PR)? Are all other issues fixed (if so, please close them)?

gkasprow commented 3 years ago

Yes, I already fixed the issue with DIFF pairs clearance

hartytp commented 3 years ago

🎆

@pathfinder49 can you confirm you're happy with the final layout and we'll move to production

gkasprow commented 3 years ago

@hartytp let me publish the release. One moment.

gkasprow commented 3 years ago

fixed some cosmetic issues https://github.com/sinara-hw/Fastino/releases/tag/v1.2rc1 once @pathfinder49 ACKs, I will change it to "release"

pathfinder49 commented 3 years ago

I've had a look at the v1.2rc1

Most of the changes look good. However, the diff pair clearance has also been applied to the analogue outputs. The increased clearance causes these to have no ground between the analogue signal and digital vias (red). I believe this may result in digital to analogue crosstalk. There now also isn't ground between the analogue out and the P5V0 reference vias (green). This may result in analogue to analogue crosstalk.

@gkasprow Is there a reason the increased differential pair clearance should also be applied to the analogue outputs? Otherwise, I'd suggest reverting this clearance back to the old value.

image

pathfinder49 commented 3 years ago

Possibly increase the spacing here (though I think we concluded analogue-analogue coupling was likely to be dominated by the ribbon cables and connectors.) image

pathfinder49 commented 3 years ago

Move the P6V0 and P3V3 away from noisy GND. image

pathfinder49 commented 3 years ago

For some channels the P5V0Ref capacitor ground is on a GND pour with the neighboring channel.

Edit: I don't have enough data to say anything conclusive. However, this seems to coincide with the unexplained -80 dBmV digital to analogue crosstalk. image

gkasprow commented 3 years ago

@pathfinder49 good catch!

hartytp commented 3 years ago

@pathfinder49 can you confirm that you've completed your review so that once this issue is fixed we can send the design to manufacture?

pathfinder49 commented 3 years ago

Aside from those points, I'm done with the review.

hartytp commented 3 years ago

cool. @gkasprow let's fix this and send for manufacture

gkasprow commented 3 years ago

I added a net class for LVDS signals and fixed all issues @pathfinder49 found.

hartytp commented 3 years ago

@pathfinder49 if you're happy to sign off, let's move to manufacture!

pathfinder49 commented 3 years ago

Unfortunately, we've missed one of the split grounds (red). image

pathfinder49 commented 3 years ago

I'm not sure if the parallel termination DNP variant is the default. According to #80 this was done. However, looking at the render of "[no variations]" the parallel termination appears populated.

pathfinder49 commented 3 years ago

That's everything.

gkasprow commented 3 years ago

DNP parallel is default. The output BOM and PnP files are marked with variant name.

gkasprow commented 3 years ago

on the render,, the termination caps are DNP obraz

gkasprow commented 3 years ago

fixed, production files are here https://github.com/sinara-hw/Fastino/releases/tag/v1.2rc3

pathfinder49 commented 3 years ago

One nit is that there is now no GND between the analogue out and the digital vias. I've made a slight routing tweak such that there is GND. See PR #83

pathfinder49 commented 3 years ago

Thank you for all those detailed tweaks 👍

gkasprow commented 3 years ago

I updated the release https://github.com/sinara-hw/Fastino/releases/tag/v1.2rc4