Change signal names to match polarity #131
Conversation
Signed-off-by: Oguz Meteer <info@guztech.nl>
|
Sadly, you did the exact opposite of what I'd like you to do... the names that you have changed correspond to active high signals. Here's what I had in mind: diff --git a/nmigen_boards/resources/interface.py b/nmigen_boards/resources/interface.py
index 8d5bd7e..0e0118a 100644
--- a/nmigen_boards/resources/interface.py
+++ b/nmigen_boards/resources/interface.py
@@ -56,21 +56,21 @@ def IrDAResource(number, *, rx, tx, en=None, sd=None,
return Resource("irda", number, *io)
-def SPIResource(*args, cs, clk, copi, cipo, int=None, reset=None,
+def SPIResource(*args, cs_n, clk, copi, cipo, int=None, reset=None,
conn=None, attrs=None, role="controller"):
assert role in ("controller", "peripheral")
assert copi is not None or cipo is not None # support unidirectional SPI
io = []
if role == "controller":
- io.append(Subsignal("cs", PinsN(cs, dir="o", conn=conn)))
+ io.append(Subsignal("cs", PinsN(cs_n, dir="o", conn=conn)))
io.append(Subsignal("clk", Pins(clk, dir="o", conn=conn, assert_width=1)))
if copi is not None:
io.append(Subsignal("copi", Pins(copi, dir="o", conn=conn, assert_width=1)))
if cipo is not None:
io.append(Subsignal("cipo", Pins(cipo, dir="i", conn=conn, assert_width=1)))
else: # peripheral
- io.append(Subsignal("cs", PinsN(cs, dir="i", conn=conn, assert_width=1)))
+ io.append(Subsignal("cs", PinsN(cs_n, dir="i", conn=conn, assert_width=1)))
io.append(Subsignal("clk", Pins(clk, dir="i", conn=conn, assert_width=1)))
if copi is not None:
io.append(Subsignal("copi", Pins(copi, dir="i", conn=conn, assert_width=1)))
|
|
Ah, you want to keep the signal names the same, but only change the user-facing function parameter names? |
|
Correct. As I mentioned before, the choice of signal names as they are exposed to the design is very much deliberate. But I think the parameter names simply slipped by without anyone noticing this flaw until now. |
|
Ok, then the change should be smaller than what I've done here. The function parameters should reflect the polarity, and any board file that uses these functions should have their parameter names updated, but the rest of the signal names should be untouched. Let me start over with the commit and PR in order not to pollute the log history with reverts and such. |
FYI, if you force-push to a PR branch, you can replace the entire history. But there's no real harm in closing and reopening it either. |
This commit adds the
_nsuffix to signal names and function parameters that usePinsN. It also changes the default reset signal name where necessary to reflect the_nsuffix.