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

add new lib Memory_Flash_NAND #2601

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

nerdyscout
Copy link
Contributor

@nerdyscout nerdyscout commented Apr 3, 2020

This adds a new library for ONFI NAND flashes which all follow this documentation
http://www.onfi.org/-/media/client/onfi/specs/onfi_4_2-gold.pdf?la=en

These generic symbols could be used in two different ways:

  • use the generic symbol as template to add new symbols. eg GD9FU1G8F2AMGI could easily be created by slight modification of TSOP-48_SDRx8 (just VDDi, VPP and some CE, R/B signals have to be removed).
  • use the generic symbol straight ahead in a schematic and leaving the not used pins unconnected.

Further generic symbols (mainly BGA-100, BGA-152) will be inserted by me once the current are merged.

All symbols are drop in compatible to each other.

SDR DDRx8 DDR2x8 DDR3x8
TSOP48 image will be part of later pull request not supported not supported
BGA63 image image not supported not supported
BGA132 image image image is now an alias of DDR2x8
BGA100 will be part of later pull request will be part of later pull request will be part of later pull request will be part of later pull request
BGA152 will be part of later pull request will be part of later pull request will be part of later pull request will be part of later pull request

All contributions to the kicad library must follow the KiCad library convention

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
    • A new fitting footprint must be submitted if the library does not yet contain one.
  • 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
  • Give a reason behind any intentional library convention rule violation.

@nerdyscout nerdyscout mentioned this pull request Apr 3, 2020
6 tasks
@cpresser cpresser added Addition Adds new symbols to library Pending reviewer A pull request waiting for a reviewer labels Apr 3, 2020
@moho1
Copy link

moho1 commented Apr 3, 2020

Hi, nice pull request. I just created some more specific NAND symbols for an own project. Looking forward to see this merged.

Have you considered also adding support for a 16-bit IO-bus to the symbols? Or would that conflict with the 8-bit version?

@nerdyscout
Copy link
Contributor Author

I do not work with SDRx16 modules but had a quick view into this. It could be easily supported by changing the SDRx8 layout (extending IO pins and moving the R/B signals somewhere else). As 8 / 16 bit modules are per definition not drop in compatible to each other I would suggest to make a separate layout for the 16 bit variants, this could look like this:
image
For the moment I won't add it here as the current pull request already feels overloaded.

@nerdyscout nerdyscout force-pushed the Memory_Flash_NAND branch 2 times, most recently from fdbb759 to a480a29 Compare April 4, 2020 08:10
@moho1
Copy link

moho1 commented Apr 4, 2020

Ok, making separate symbols for 8/16 bit modules probably makes more sense.

@cpresser cpresser self-assigned this Apr 8, 2020
@cpresser cpresser removed the Pending reviewer A pull request waiting for a reviewer label Apr 8, 2020
@cpresser
Copy link
Contributor

cpresser commented Apr 8, 2020

That looks like a nice addition. Thank you very much ❤️

I have not done a full review yet, but want to start the discussion on some points that are visible from the screenshot and the diff:

  1. Good thing that you did not add all variants in one PR. This is already quite big.
  2. If possible, stop force pushing to this branch now. This allows me to check individual commits for review.
  3. The pin-lengths look weird. I assume that is for compatibility with future devices in larger packages? (BGA-152 uses row11)
  4. I don't like the names. Instead of BGA-132_NV-DDR2x8 how about ONFI_NV-DDR2x8_BGA-132. That would follow the scheme of other libs that have the package name at the end (eg MC78L05_TO92).
  5. I am not sure if those should go into a new library. Memory_Flash seems like it is a good fit. With the ONFI prefix in the name they are also quite easy to spot. However, that would add a generic device into a lib that right now has only fully specified symbols. What do you think?

@nerdyscout
Copy link
Contributor Author

nerdyscout commented Apr 8, 2020

Hi @cpresser,

thank you a lot for a the comments.

  1. yes, there was something weird... usually i don't force push.
  2. I just checked the KLC and really could shorten the pins to 150mil
    this comment I don't understand "(BGA-152 uses row11)"
    max pin name character count should never be < 3, so there is no problem. BGA152 and BGA100 symbols will look 100% like BGA132 just with different pin names.
  3. I agree
  4. current Memory_Flash is very messy. There are symbols of EEPROM and NOR Flashes, symbols with very long pins, symbols not centered, broken datasheet links...
    I don't like to put my shiny new symbols into this swamp, how about building two new libs Memory_Flash_NAND and Memory_Flash_NOR and moving all parts from the current Memory_Flash over the time? But this should be decided by a librarian.
  5. DDR3 is currently a 100% copy of DDR2, I know this is not nice and an alias could be used. I see several options:
    • keep separate symbols, therefore specific DDRx flashes will be an alias of DDRx symbol
    • rename the symbol to something like ONFI_NV-DDR2x8_DDR3x8_BGA-132 and make all DDR2/DDR3 flashes an alias of this - this is currently my favorite.
    • keep ONFI_NV-DDR2x8_BGA-132 with one alias ONFI_NV-DDR3x8_BGA-132 and make all DDR2/DDR3 flashes an alias of it.
  6. I might remove the pins VDDi and VPP, as far as I understand these are for flash manufacturers only... can not remember to have seen these in the datasheets the last two years... but this will need a deeper check.

Please be aware I didn't update the screenshots after the little fixes after commit 3270326.

@cpresser
Copy link
Contributor

cpresser commented Apr 8, 2020

  1. 'BGA-152 uses row11The pin-number could beA11`. Smaller parts will only have one character for the numbers since the used pins are in rows 0..9. I order to allow drop-in replacement for all types the pins should stay at the same position. So choosing 150mil for pin lengths to accommodate 3 characters seems like the correct choice.
  2. I agree, it is very messy. See Memory symbols require extensive rework #18 I will ask the other librarians.
  3. The usual approach here is to have just one full symbol and all symbols with identical pinout and footprint are an alias.
  4. I just skimmed the datasheet and at least VPP seems to be use-full in some designs. The downside of keeping them is that a DRC error will be thrown if a power-input is left unconnected.

@KiCad/librarians
Problem: Memory_Flash is rather messy, the contributor does not want to add new, shiny symbols there.
Possible solutions that don't break existing designs:

  1. Make new libraries Memory_Flash_NAND and Memory_Flash_NOR, populate them with good, new or reworked parts. Keep the current Memory_Flash until we can remove it (6.0). That would also allow us to rework the messy parts now. Downside is
    1. More confusion about which library is correct
    2. Duplicate symbols (this might be a showstopper)
  2. Just one new library Memory_Flash_ONFI for the Parts from the ONFI specification.
  3. Other new libs and splits. Perhaps Memory_Flash_Serial as we already have a lot of them?
  4. No nothing special. Keep the current library as it is, add the new parts there. Hopefully somebody can find time to fix this when we are close to 6.0.
  5. other suggestions?

I personally would like to rework the library, I have merged at least one new symbol recently and was already thinking of improving it. Option 1 would allow that.

@nerdyscout
Copy link
Contributor Author

screenshots in initial post are now updated.
I don't know what happened to my git log, according travis output several other libs seem to be deleted... don't know what is going on. might need later fixes.

@keesj
Copy link

keesj commented Aug 24, 2020

In the official ONFI specs but also in the documentation for the chip mentioned above the ground uses for pads

  • K3 and K8 are used for VSSQ
  • C5 and F7 for VSS

but in the diagram(BGA63) above only two ground are shown K3 and C5 is this intentional?

upon closer inspection these are marked as "passive"

vss

Is this acceptable ?

@nerdyscout
Copy link
Contributor Author

In the official ONFI specs but also in the documentation for the chip mentioned above the ground uses for pads
* K3 and K8 are used for VSSQ
* C5 and F7 for VSS
but in the diagram(BGA63) above only two ground are shown K3 and C5 is this intentional?
upon closer inspection these are marked as "passive"
Is this acceptable ?

https://kicad-pcb.org/libraries/klc/S4.3/
according 8. I guess this should be fine.

@cpresser
Copy link
Contributor

upon closer inspection these are marked as "passive"

Is this acceptable ?

Yes. This is perfectly fine and in accordance to KLC.

Regarding the library where this will go into: We will soon start merging stuff for KiCad v6.0 which will include breaking changes.
Reworking Memory_Flash is on my todo-list. I will commonalize all the symbols for serial flash. And perhaps move them to Memory_Flash_Serial. Anyway, we will find a nice place for those symbols soon.

That also means I have to put in some time to finally give this PR a full review.

@nerdyscout
Copy link
Contributor Author

@cpresser I am currently on vacation and more than happy to invest one week (from 1.9 on) to help sorting these thing. Just let me know.

DEF ONFI_NV-DDR2x8_BGA-132 U 0 20 Y Y 2 L N
F0 "U" 0 0 50 H V C CNN
F1 "ONFI_NV-DDR2x8_BGA-132" 0 -100 50 H V C CNN
F2 "Package_BGA:BGA-132_12x18mm_Layout11x17_P0.5mm" 0 -200 50 H I C CNN
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package name appears to be wrong. The pitch is 0.8 in the footprint is correct but the filename suggest is it P0.5mm

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was intended for the BGA63 package

@keesj
Copy link

keesj commented Sep 2, 2020

Also after discussion on IRC it might be better to make the pads smaller 0.4mm instead of 0.5mm to make it more easy to route.
paraphrasing here:

pads should be a bit smaller but you need clearance to match the largest the ball can get post-reflow
So in this case, you want 0.4mm pads, 0.5mm minimum mask opening
0.4mm pads, 0.1mm additional soldermask clearance (0.4+0.1 == 0.5)

@cpresser
Copy link
Contributor

cpresser commented Sep 2, 2020

Also after discussion on IRC it might be better to make the pads smaller 0.4mm instead of 0.5mm to make it more easy to route.
paraphrasing here:

pads should be a bit smaller but you need clearance to match the largest the ball can get post-reflow
So in this case, you want 0.4mm pads, 0.5mm minimum mask opening
0.4mm pads, 0.1mm additional soldermask clearance (0.4+0.1 == 0.5)

I strongly disagree on arbitrarily shrinking pads. We do follow manufacturers conventions and IPC rules.
A pull request related to that was just merged: pointhi/kicad-footprint-generator#592

The default footprint should be exact. (The BGA63 should be build according to JEDEC MO-207)
If somebody wants another footprint for routing-reasons they can generate/use a custom footprint.

@evanshultz
Copy link
Collaborator

@chschlue
You mind thinking through and commenting on the library naming topic above?

@nerdyscout
Copy link
Contributor Author

Also after discussion on IRC it might be better to make the pads smaller 0.4mm instead of 0.5mm to make it more easy to route.
paraphrasing here:
pads should be a bit smaller but you need clearance to match the largest the ball can get post-reflow
So in this case, you want 0.4mm pads, 0.5mm minimum mask opening
0.4mm pads, 0.1mm additional soldermask clearance (0.4+0.1 == 0.5)

I strongly disagree on arbitrarily shrinking pads. We do follow manufacturers conventions and IPC rules.
A pull request related to that was just merged: pointhi/kicad-footprint-generator#592

The default footprint should be exact. (The BGA63 should be build according to JEDEC MO-207)
If somebody wants another footprint for routing-reasons they can generate/use a custom footprint.

yes I agree and just had an deeper look into the onfi datasheet, page 13

The 63-ball BGA package has two solder ball diameter sizes: 0.45±0.05mm and 0.55±0.05mm post reflow.

to keep it simple I would suggest to make all pads 0.5mm instead of building two variants.
Anyway.. why do we discuss this in the symbol pull request 😞

@evanshultz
Copy link
Collaborator

See the IPC 7351B pad sizes at https://github.com/pointhi/kicad-footprint-generator/blob/master/scripts/Packages/Package_BGA/ipc_7351b_bga_land_patterns.yaml. 0.45mm nominal balls should get 0.4mm pads and 0.55mm balls should be 0.5mm pads.

I agree with @cpresser that the default footprint should match the part according to the IPC rules we have selected for this library. Any deviation could be handled by a manufacturer-specific footprint, but I don't see recommended pad sizes for the BGA-63 in the ONFI spec. Am I missing it? All I see is on page 17 that the 0.45mm ball size is repeated.

https://www.gigadevice.com/datasheet/gd9fx1gxf2a/ shows 0.45mm balls. And JEDEC MO-207 (https://www.jedec.org/sites/default/files/docs/MO-207L.pdf) variant Dyy-1 on pages 3-4 is the one with 0.8mm pitch and it shows 0.45mm balls with variant DH-z on page 13 appearing to match the ball layout. So... do we actually see 0.55mm balls used in practice? I didn't look any further than that but if the packages are supposed to meet JEDEC MO-207 I only see 0.45mm balls.

That doesn't really clear things up but maybe it helps?

Also, @cpresser the script updates you merged don't include the ball diameter in the footprint name. We don't always have that info if the IPC pads aren't used, but perhaps it should be added if known. What do you think?

@chschlue
Copy link
Contributor

chschlue commented Sep 4, 2020

Flash lib reorg [RFC]:

  1. Looks like most current symbols are both NAND and serial. Also, I assume that combination will stay the most common for the foreseeable future (being kind of a general purpose NV memory IC, as opposed to parallel NAND for drives and such or NOR overall). What I'm saying is, whether we split into Memory_Flash_NAND/Memory_Flash_NOR or Memory_Flash_Serial/Memory_Flash_Parallel, we aren't going to get remotely balanced libraries (by symbol count)
  2. I'm against adding a generic symbol to Memory_Flash. Device looks to be the best fit currently, although it's certainly more complex than what we usually consider to be a Device. A generic ONFI symbol is more in line with generic USB, HDMI, DIMM and such, but certainly isn't a connector, so we can't put it there either. If we have more candidates like this, we might want to add some sort of "generic physical interface" lib. Not sure about the name though as Interface is already taken.

Thoughts @cpresser @evanshultz?

@evanshultz
Copy link
Collaborator

I get what you're saying. It is rather annoying. Can we pick one candidate part that conforms to the ONFI spec and add it so the symbol isn't generic? Then other specific devices could be an ALIAS or the user could place the one symbol in our lib and modify the properties as they see fit for another MPN they prefer.

@nerdyscout
Copy link
Contributor Author

Yesterday I stumbled across the IPC footprint generator and it just got into my mind we should use something similar here now. This would make the need of a symbol template obsolete and is already because the amount of various flashes very useful

@keesj
Copy link

keesj commented Sep 4, 2020

Yesterday I stumbled across the IPC footprint generator and it just got into my mind we should use something similar here now. This would make the need of a symbol template obsolete and is already because the amount of various flashes very useful

is this different from this project?
https://github.com/pointhi/kicad-footprint-generator/blob/master/scripts/Packages/Package_BGA/ipc_bga_generator.py

@evanshultz
Copy link
Collaborator

I assume you mean one of the IPC-compliant footprint generators at https://github.com/pointhi/kicad-footprint-generator.

Did you see https://github.com/KiCad/kicad-library-utils/tree/master/schlib/autogen? There is a generator for some STM32 symbols there, along with other simple devices like connectors and resistor networks.

The schematic and schematic symbol file format is changing for KiCad v6 and there is simple inheritance which may help?

@nerdyscout
Copy link
Contributor Author

@evanshultz yes this is the generator I was talking about.

The one for the symbols is really cool as well, together with this
https://dev.peratx.net:444/fd/decode?lang=eng&pn=MT29F128G08CBECBH6
it should be possible to quickly generate a lot of symbols. Unfortunately I am not really capable to do it by myself.

@cpresser
Copy link
Contributor

cpresser commented Sep 6, 2020

Flash lib reorg [RFC]:

1. Looks like most current symbols are both NAND and serial. Also, I assume that combination will stay the most common for the foreseeable future (being kind of a general purpose NV memory IC, as opposed to parallel NAND for drives and such or NOR overall). What I'm saying is, whether we split into `Memory_Flash_NAND`/`Memory_Flash_NOR` or `Memory_Flash_Serial`/`Memory_Flash_Parallel`, we aren't going to get remotely balanced libraries (by symbol count)

2. I'm against adding a generic symbol to `Memory_Flash`. `Device` looks to be the best fit currently, although it's certainly more complex than what we usually consider to be a `Device`. A generic ONFI symbol is more in line with generic USB, HDMI, DIMM and such, but certainly isn't a connector, so we can't put it there either. If we have more candidates like this, we might want to add some sort of "generic physical interface" lib. Not sure about the name though as `Interface` is already taken.

Thoughts @cpresser @evanshultz?

As for splitting libraries: Currently there are not nearly enough symbols in there. The only reason we are discussing a split is because the current library is messy. I plan to fix that (with breaking changes).
So my vote here is keep everything in one library for now.

Currently I see 3 major options for ONFI-Symbols (and similar Devices, for example SPI and QSPI serial-flash with 8 pins)
A. Only fully-specified symbols in Memory_Flash
B. Generic symbols in Device and fully-specified ones in Memory_Flash
C. Both generic and fully-specified in Memory_Flash

I would argue that the B option is not good. Its more maintaining effort when a small change is needed.
Currently I tend to option C, even though that violates KLC (generic symbols are not allowed here). But I think there is a use-case for that. We also did violate KLC G2.1 for RJ45 connectors (most are fully specified).

My current idea is to prefix generic symbols. Examples:

  • Generic_Flash_SPI
  • Generic_Flash_QSPI (unsure if there should be package variants like -SO8, -QFN8, ... should be included here)
  • Generic_Flash_ONFI_SDR_TSOP48
  • ...

The fully specified devices could be aliases to those. Afaik with the v6 symbol file format we can also have aliases that have a different package.

That said, I am also okay with option A (only fully specified symbols). If new compatible devices show up we can just add an alias.

@nerdyscout
Copy link
Contributor Author

nerdyscout commented Sep 8, 2020

had an deeper look into the SchLib/AutoGen today, never used python before, so it is rather dirty.
https://github.com/nerdyscout/kicad-library-utils/tree/onfi/schlib/autogen/onfi
needs still some work, but I guess it will produce some usable symbols.
If this is merged once I would say there is no need for the generic symbols anymore, therefore I am fine to close this ticket.

@evanshultz
Copy link
Collaborator

@cpresser
B is what we do for transistors and diodes, for example, but I agree it's awkward. At least for now, I vote for A and then picking one or more compatible parts instead of making generic symbols. Reading https://kicad-pcb.org/libraries/klc/G2.1/, moving towards fully-specified symbols is the way the library is moving and as these symbols do have a defined default footprint I'm not even sure generic symbols are an option if KLC compliance is to be achieved.

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.

6 participants