Origen-SDK / origen

The Origen Semiconductor Developer's Kit core platform
http://origen-sdk.org
MIT License
20 stars 24 forks source link

Updated pin type to be set automatically unless set by user #407

Closed info-rchitect closed 3 years ago

info-rchitect commented 3 years ago

The reason for this change is so we can put the pin type in the metadata automatically vs having to set it. It is backwards compatible.

info-rchitect commented 3 years ago

Any issues here preventing merge?

pderouen commented 3 years ago

@info-rchitect I haven't responded because I haven't ever paid attention to the pin type and don't know where that information gets consumed. The only thing that raises a question to my mind is that it now defaults to a pin type that is new in this PR. As long as that doesn't cause backward compatibility problems, I don't have any issue with merging.

info-rchitect commented 3 years ago

@pderouen Thanks for the feedback. You do bring up a corner case where some apps at various companies could be expecting a pin type to be nil. I don't think that would be a common case but it is a real possibility. The PR is backwards compatible with all of the origen spec tests.

pderouen commented 3 years ago

It seems reasonable to have a default type of signal. I guess if the tests pass and there are no objections it should be fine.

info-rchitect commented 3 years ago

It seems reasonable to have a default type of signal. I guess if the tests pass and there are no objections it should be fine.

Not all pins default to :signal just to be clear. If it is a ground pin it will default to :ground, etc.

priyavadan commented 3 years ago

@chrishume @coreyeng @chrisnappi can ya'll confirm this change would be okay within NXP? Since all spec tests are passing currently, I don't expect there to be issues but just want to confirm.