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

Zilog Z80 PIO Parallel I/O DIP-40 #1333

Merged
merged 28 commits into from Mar 14, 2019
Merged

Zilog Z80 PIO Parallel I/O DIP-40 #1333

merged 28 commits into from Mar 14, 2019

Conversation

b1ackmai1er
Copy link
Contributor

@b1ackmai1er b1ackmai1er commented Dec 27, 2018

Zilog Z80 PIO Parallel I/O DIP-40

Second pass with Travis fixes.
http://www.zilog.com/docs/z80/um0081.pdf
Regards Phil

Thanks for creating a pull request to contribute to the KiCad libraries! To speed up integration of your PR, please check the following items:

  • Provide a URL to a datasheet for the symbol(s) you are contributing
  • An example screenshot image is very helpful
  • Ensure that the associated footprints match the official footprint library
  • If there are matching footprint PRs, provide link(s) as appropriate
  • Check the output of the Travis automated check scripts - fix any errors as required

@b1ackmai1er
Copy link
Contributor Author

Datasheet extract vs schematic

image

@b1ackmai1er
Copy link
Contributor Author

Hi, now that I am more familar with the submission process I would be interested in submitting other Zilog legacy schematics (CTC, DMA, SIO). There is still a lot of activity in the retro community with these parts. In this case would it be better to create a new catagory Zilog-Legacy and move this and the Z80 CPU into this new catagory.

@aewallin
Copy link
Contributor

  • your commit has changes to .github/PULL_REQUEST_TEMPLATE.md, which is not desirable. The comments here in the github discussion are not in the actual git-repo. could you revert those changes?
  • Interface_PIO doesn't say much to someone who isn't a Zilog-enthusiast - could these symbols go into an existing MCU-library instead?
  • if you take a screenshot from the symbol editor it would show pin-types also, which would help review.

@aewallin
Copy link
Contributor

There is an existing symbol CPU:Z80CPU (DIP-40) - would these go into CPU also?

Did not understand how this worked. Fixed.
@b1ackmai1er
Copy link
Contributor Author

Hi, CPU:Z80CPU (DIP-40) is in the correct catagory in my opinion unless a new catogory for Zilog Z80 family is created. In which case the Z80, PIO, CTC and DMA chip would all be included.

After looking further I can see similar devices by other manufacturers (8255,6821 ) are in interface.lib.

So PIO could go in there as well. Ideally a new catagory interface_parallel (instead of PIO) could capture these devices. However, if there is a desire to not extend the catagories then I think the way forward is to put PIO in interface.lib.

Let me know and I will submit update. Thankyou.

@b1ackmai1er
Copy link
Contributor Author

image

@myfreescalewebpage myfreescalewebpage added Pending reviewer A pull request waiting for a reviewer Addition Adds new symbols to library labels Dec 30, 2018
@antoniovazquezblanco antoniovazquezblanco added this to the Backlog milestone Jan 8, 2019
@myfreescalewebpage myfreescalewebpage self-assigned this Mar 10, 2019
@myfreescalewebpage myfreescalewebpage removed the Pending reviewer A pull request waiting for a reviewer label Mar 10, 2019
@myfreescalewebpage
Copy link
Collaborator

Hi @b1ackmai1er thanks for this contribution and @aewallin thanks for the first steps review.

@b1ackmai1er can you update your branch, you have a conflict to solve, then will be able to make the review for this PR.

Thanks
Joel

@b1ackmai1er
Copy link
Contributor Author

Hi Joel, Try now. Made some mistakes along the way - still learning. Regards Phil

@myfreescalewebpage
Copy link
Collaborator

@b1ackmai1er thanks for the update.

I'm troubled by the name of the symbol. The datasheet don't seems to be the right one. Looking at Z8420 and Z84C20, I have found this document: https://www.zilog.com/appnotes_download.php?FromPage=DirectLink&dn=PS0180&ft=Product%20Specification%20(Data%20Sheet)%20%20&f=YUhSMGNEb3ZMM2QzZHk1NmFXeHZaeTVqYjIwdlpHOWpjeTk2T0RBdmNITXdNVGd3TG5Ca1pnPT0= which is better I think. Is it ?

Also, in this case, the device should be named Z8420 with a single alias called Z84C20. Z80PIO is not a known reference from Zilog according to me.

I also agree with that this device is betetr in Interface.lib. Please, move it there.

Joel

@b1ackmai1er
Copy link
Contributor Author

All requested changed completed. Yes your reference data sheet is better. Thank you.

@myfreescalewebpage
Copy link
Collaborator

myfreescalewebpage commented Mar 12, 2019

Thanks, let's start a full review now:

  • Pin length should be 100mil (symbol < 100pins)
  • Name of pins A0 to A7 should be PA0 to PA7
  • Name of pins B0 to B7 should be PB0 to PB7
  • Graphic Style of pin 25 should be Line
  • The height and width of the symbol should be reduce a bit, there is too much blank space lost

Else, seems ok for me.

Thanks,
Joel

Pin length reduces to 100mil
Name of pins A0 to A7 changed to PA0 to PA7
Name of pins B0 to B7 changed PB0 to PB7
Graphic Style of pin 25 schanged to Line
The height and width of the symbol reduced
Pin length reduces to 100mil
Name of pins A0 to A7 changed to PA0 to PA7
Name of pins B0 to B7 changed PB0 to PB7
Graphic Style of pin 25 schanged to Line
The height and width of the symbol reduced
Pin length reduces to 100mil
Name of pins A0 to A7 changed to PA0 to PA7
Name of pins B0 to B7 changed PB0 to PB7
Graphic Style of pin 25 schanged to Line
The height and width of the symbol reduced
@b1ackmai1er
Copy link
Contributor Author

Hi Joel, think this is done now. Made update with Kicad 4.07 and I think it messed stuff up so had to restore so left a bit of a mess in between. Regards Phil

@myfreescalewebpage
Copy link
Collaborator

@b1ackmai1er something is not correct, you have now twice the definition, each with the aliases, please look at the git diff at https://github.com/KiCad/kicad-symbols/pull/1333/files.

Yes I strongly recommend you to install KiCad 5 !

Joel

Remove old definition.
@b1ackmai1er
Copy link
Contributor Author

Thanks Joel, fixed. Sorry yes I am running both 4.07 and 5.x at the moment. Will do any further work on 5.x branch. Once again, thanks for your patience.

@myfreescalewebpage
Copy link
Collaborator

No worries @b1ackmai1er thanks for the fixes ! No more comment to do, merging.

@myfreescalewebpage myfreescalewebpage merged commit 358b31c into KiCad:master Mar 14, 2019
@antoniovazquezblanco antoniovazquezblanco modified the milestones: Backlog, 6.0.0 Mar 15, 2019
@antoniovazquezblanco antoniovazquezblanco modified the milestones: 6.0.0, 5.1.1 Apr 12, 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.

None yet

4 participants