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

v1.2 design review #81

Closed
hartytp opened this issue Sep 16, 2020 · 38 comments
Closed

v1.2 design review #81

hartytp opened this issue Sep 16, 2020 · 38 comments

Comments

@hartytp
Copy link
Collaborator

hartytp commented Sep 16, 2020

@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 #74 is fixed we can move to the v1.2 release

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 16, 2020

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

@gkasprow
Copy link
Member

@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
Copy link
Collaborator

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

@pathfinder49
Copy link
Collaborator

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
Copy link
Collaborator Author

hartytp commented Sep 17, 2020

@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
Copy link
Member

jordens commented Sep 18, 2020

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

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 18, 2020

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
Copy link
Member

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

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 18, 2020

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
Copy link
Member

@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
Copy link
Member

jordens commented Sep 18, 2020

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
Copy link
Member

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
Copy link
Collaborator Author

hartytp commented Sep 21, 2020

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

@gkasprow
Copy link
Member

Yes, I already fixed the issue with DIFF pairs clearance

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 21, 2020

@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
Copy link
Member

Yes, I already fixed the issue with DIFF pairs clearance

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 23, 2020

🎆

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

@gkasprow
Copy link
Member

@hartytp let me publish the release. One moment.

@gkasprow
Copy link
Member

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

@pathfinder49
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

Move the P6V0 and P3V3 away from noisy GND.
image

@pathfinder49
Copy link
Collaborator

pathfinder49 commented Sep 24, 2020

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
Copy link
Member

@pathfinder49 good catch!

@hartytp hartytp reopened this Sep 25, 2020
@hartytp
Copy link
Collaborator Author

hartytp commented Sep 25, 2020

@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
Copy link
Collaborator

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

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 25, 2020

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

@gkasprow
Copy link
Member

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

@hartytp
Copy link
Collaborator Author

hartytp commented Sep 29, 2020

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

@pathfinder49
Copy link
Collaborator

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

@pathfinder49
Copy link
Collaborator

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
Copy link
Collaborator

That's everything.

@gkasprow
Copy link
Member

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

@gkasprow
Copy link
Member

on the render,, the termination caps are DNP
obraz

@gkasprow
Copy link
Member

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

@pathfinder49
Copy link
Collaborator

pathfinder49 commented Sep 29, 2020

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
Copy link
Collaborator

Thank you for all those detailed tweaks 👍

@gkasprow
Copy link
Member

gkasprow commented Sep 29, 2020

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

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

No branches or pull requests

5 participants