Skip to content
This repository has been archived by the owner on Oct 2, 2020. It is now read-only.

add AD9513, AD9514 and AD9515 #1558

Merged
merged 5 commits into from
Jun 2, 2019
Merged

Conversation

hvraven
Copy link
Contributor

@hvraven hvraven commented Feb 21, 2019

This adds the clock distribution ICs AD9513, AD9514 and AD9515. They are very similar except their outputs. Footprint PR will follow. I placed VREF on top even though it is an output as there was space. It is used for the configuration of the S[0..10] pins. Alternatively I was thinking of splitting the part into two, moving the configuration to another part.

ad9515
ad9514
ad9513

@myfreescalewebpage
Copy link
Collaborator

myfreescalewebpage commented Feb 23, 2019

Hi @lorem-ipsum , thanks for contributing,

A few comments I have during my review:

For all devices:

  • Description, "Output" => "Outputs" (plural)
  • The footprint filter never contain the EP size, but contain presence of EP, this should be LFCSP*1EP*5x5mm*P0.5mm* for this symbol
  • VS pins should not be stacked, even if this enlarge the symbol a lot
  • VREF should be on the right of the symbol (Power out, not power in)
  • "B" in pin name is for complementary in/out. would suggest to use "~", thus OUT0B => ~OUT0, OUT1B => ~OUT1, OUT1B => ~OUT1, CLKB => ~CLK, SYNCB => ~SYNC
  • The datasheet indicate the EPAD is connected to GND and is a power pin, so you should stack EPAD with GND
  • Reference in top left corner, name in top right corner is great, instead of inside the symbol

Cheers,
Joel

@evanshultz
Copy link
Collaborator

I'd prefer to keep the pin names as on the datasheet and not remove the "B" so the symbol and documentation match. KLC doesn't request this, either.

GND pins can always be stacked. Power pins should be stacked unless there's a reason not to, like the datasheet requiring a decoupling cap on each pin. The top of page 22 of the datasheet and https://www.analog.com/media/en/technical-documentation/evaluation-documentation/53447263508842AD9513_14_15_RevB_Eval_Board_Schematic.pdf both indicate to me that each VS pin needs a cap, so yes, not stacking is the right way to go here.

@myfreescalewebpage
Copy link
Collaborator

@evanshultz

I'd prefer to keep the pin names as on the datasheet and not remove the "B" so the symbol and documentation match. KLC doesn't request this, either.

The datasheet indicate it is the complementary output, and we usually do that for example when a pin is "RSTn" or "CSn", they become "~RST" and "~CS".
Here do you mean it's bette to have "~OUTB" ?

Joel

@evanshultz
Copy link
Collaborator

KLC discusses overbars for logic low signals, and not complementary/differential signals. Here the usage of "B" in pin names is mixed. I would keep the "B" on outputs but it's uncommon (to me) for logic low inputs; for consistency maybe keep all "B"? IMO the overbar adds clarity and probably wouldn't cause any confusion so what you mention works me. This isn't really covered in KLC except for the part about the datasheet winning if there's conflict. What do you like?

@myfreescalewebpage
Copy link
Collaborator

@evanshultz thanks to point me the difference, you're right it's not really some "logic low" in/out.
I have look for other devices in the library to find examples and found very few devices, for example SAD1024, which is not with the overbar and keep the name of the datasheet. I have just removed my comment above for this reason, to keep the consistency, and we will need to remember this choice or indicate the difference in the KLC in the future.

@myfreescalewebpage myfreescalewebpage self-assigned this Feb 25, 2019
@myfreescalewebpage myfreescalewebpage removed the Pending reviewer A pull request waiting for a reviewer label Feb 25, 2019
@myfreescalewebpage
Copy link
Collaborator

@lorem-ipsum any news here ? Thanks for the update, Joel

@hvraven
Copy link
Contributor Author

hvraven commented May 12, 2019

Sorry, I was occupied for some time with personal issues. Trying to slowly work off the queue of PRs now. I've integrated the review feedback, the symbols now look like this:
AD9513
AD9514
AD9515

@myfreescalewebpage
Copy link
Collaborator

myfreescalewebpage commented May 12, 2019

@lorem-ipsum no worries and thanks for the update of the symbol. Just notice one remaining issue:

  • On the 3 devices, RSET should be Passive to avoid ERC errors

Then only need to wait the LFCSP footprint before merging (KiCad/kicad-footprints#1426).

Cheers,
Joel

@myfreescalewebpage myfreescalewebpage added the Pending footprint Pending footprint acceptance before merging label May 12, 2019
@evanshultz
Copy link
Collaborator

Because of all the VS pins, the symbol has a lot of unused area. Including at the bottom left. What about moving all the pins down a bit and the pushing in the sides so VS is all across the top? Here is a quick mock-up that doesn't even require making the symbol taller:
image

@myfreescalewebpage
Copy link
Collaborator

Good proposition of @evanshultz too !

@hvraven
Copy link
Contributor Author

hvraven commented May 15, 2019

Good idea. I've compressed the symbol as suggested. Now only the reference and the name stick out and look a bit ugly:
AD9513

@myfreescalewebpage
Copy link
Collaborator

That's great for me, just need to wait the footprint now.
Cheers,
Joel

@myfreescalewebpage
Copy link
Collaborator

Footprint merged. Merging the symbols.

@myfreescalewebpage myfreescalewebpage merged commit b73c7e8 into KiCad:master Jun 2, 2019
@hvraven hvraven deleted the AD9513 branch June 2, 2019 17:40
@antoniovazquezblanco antoniovazquezblanco removed the Pending footprint Pending footprint acceptance before merging label Jun 3, 2019
@antoniovazquezblanco antoniovazquezblanco added this to the 5.1.3 milestone Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Addition Adds new symbols to library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants