Skip to content

Commit 0cbfe58

Browse files
elliotb-lowriscelliotb-lowrisc
committed
Rationalise GPIO
Remove the gpio_array level of hierarchy, remove redundant parameters, standardise generated block labelling, replace bulky if-else chain with a case statement, change from MUX to OR for combining read data, remove redundant address bits being passed around and other minor touch-ups. Hopefully these make the code easier to read, and save some FPGA resources.
1 parent 68847c4 commit 0cbfe58

File tree

5 files changed

+84
-152
lines changed

5 files changed

+84
-152
lines changed

‎rtl/ip/gpio/gpio.core‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ filesets:
1212
files:
1313
- rtl/debounce_step.sv
1414
- rtl/gpio_core.sv
15-
- rtl/gpio_array.sv
1615
- rtl/gpio.sv
1716
- rtl/pcint.sv
1817
file_type: systemVerilogSource

‎rtl/ip/gpio/rtl/gpio.sv‎

Lines changed: 51 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55
module gpio #(
66
parameter int unsigned GpiMaxWidth = 8,
77
parameter int unsigned GpoMaxWidth = 16,
8-
parameter int unsigned AddrWidth = 32,
98
parameter int unsigned DataWidth = 32,
10-
parameter int unsigned RegAddrWidth = 12,
119
parameter int unsigned NumInstances = 1,
1210
parameter int unsigned GpiInstWidths[NumInstances] = {8},
1311
parameter int unsigned GpoInstWidths[NumInstances] = {16}
@@ -24,50 +22,66 @@ module gpio #(
2422

2523
output logic pcint_o[NumInstances]
2624
);
25+
localparam int unsigned NumBytesPerInstance = 16 * DataWidth/8;
26+
localparam int unsigned AddrBitsPerInstance = $clog2(NumBytesPerInstance);
27+
localparam int unsigned AddrBitsInstanceIdx = $clog2(NumInstances);
28+
localparam int unsigned RegAddrWidth = AddrBitsInstanceIdx + AddrBitsPerInstance;
2729

28-
logic device_req;
29-
logic [AddrWidth-1:0] device_addr;
30-
logic device_re; // Read enable.
31-
logic device_we; // Write enable.
32-
logic [3:0] device_be;
33-
logic [DataWidth-1:0] device_wdata;
34-
logic [DataWidth-1:0] device_rdata;
30+
logic device_req;
31+
logic [RegAddrWidth-1:0] device_addr;
32+
logic device_re; // Read enable.
33+
logic device_we; // Write enable.
34+
logic [3:0] device_be;
35+
logic [DataWidth-1:0] device_wdata;
36+
logic [DataWidth-1:0] device_rdata;
37+
38+
logic inst_req[NumInstances];
39+
logic [DataWidth-1:0] inst_rdata[NumInstances];
40+
41+
logic [AddrBitsInstanceIdx-1:0] selected_inst_idx;
3542

3643
assign device_req = device_re | device_we;
3744

38-
// Tie off upper bits of address.
39-
assign device_addr[AddrWidth-1:RegAddrWidth] = '0;
40-
41-
gpio_array #(
42-
.GpiMaxWidth ( GpiMaxWidth ),
43-
.GpoMaxWidth ( GpoMaxWidth ),
44-
.AddrWidth ( AddrWidth ),
45-
.DataWidth ( DataWidth ),
46-
.RegAddrWidth ( RegAddrWidth ),
47-
.NumInstances ( NumInstances ),
48-
.GpiInstWidths ( GpiInstWidths ),
49-
.GpoInstWidths ( GpoInstWidths )
50-
) u_gpio (
51-
.clk_i,
52-
.rst_ni,
45+
// Route req based on the high bits of the request address
46+
assign selected_inst_idx = device_addr[RegAddrWidth-1:AddrBitsPerInstance];
5347

54-
.device_req_i (device_req),
55-
.device_addr_i (device_addr),
56-
.device_we_i (device_we),
57-
.device_be_i (device_be),
58-
.device_wdata_i (device_wdata),
59-
.device_rdata_o (device_rdata),
48+
for (genvar inst_idx = 0; inst_idx < NumInstances; inst_idx++) begin : gen_gpio_core
49+
assign inst_req[inst_idx] = (selected_inst_idx == inst_idx) && device_req;
6050

61-
.gp_i,
62-
.gp_o,
63-
.gp_o_en,
51+
gpio_core #(
52+
.GpiWidth ( GpiInstWidths[inst_idx] ),
53+
.GpoWidth ( GpoInstWidths[inst_idx] ),
54+
.AddrWidth ( AddrBitsPerInstance ),
55+
.DataWidth ( DataWidth )
56+
) u_gpio_inst (
57+
.clk_i,
58+
.rst_ni,
59+
.device_req_i(inst_req[inst_idx]),
60+
.device_addr_i(device_addr[AddrBitsPerInstance-1:0]),
61+
.device_we_i(device_we),
62+
.device_be_i(device_be),
63+
.device_wdata_i(device_wdata),
64+
.device_rdata_o(inst_rdata[inst_idx]),
65+
.gp_i(gp_i[inst_idx][GpiInstWidths[inst_idx]-1:0]),
66+
.gp_o(gp_o[inst_idx][GpoInstWidths[inst_idx]-1:0]),
67+
.gp_o_en(gp_o_en[inst_idx][GpoInstWidths[inst_idx]-1:0]),
68+
.pcint_o(pcint_o[inst_idx])
69+
);
70+
end
6471

65-
.pcint_o
66-
);
72+
// Merge read data from all instances using a bitwise OR.
73+
// Instance rdata outputs are all-zeroes unless they have a valid request.
74+
always_comb begin
75+
device_rdata = 'b0;
76+
for (integer inst_idx = 0; inst_idx < NumInstances; inst_idx++) begin
77+
device_rdata |= inst_rdata[inst_idx];
78+
end
79+
end
6780

6881
tlul_adapter_reg #(
6982
.AccessLatency ( 0 ),
70-
.RegAw ( RegAddrWidth )
83+
.RegAw ( RegAddrWidth ),
84+
.RegDw ( DataWidth )
7185
) gpio_device_adapter (
7286
.clk_i,
7387
.rst_ni,
@@ -83,7 +97,7 @@ module gpio #(
8397
// Register interface.
8498
.re_o (device_re),
8599
.we_o (device_we),
86-
.addr_o (device_addr[RegAddrWidth-1:0]),
100+
.addr_o (device_addr),
87101
.wdata_o (device_wdata),
88102
.be_o (device_be),
89103
.busy_i (1'b0),

‎rtl/ip/gpio/rtl/gpio_array.sv‎

Lines changed: 0 additions & 67 deletions
This file was deleted.

‎rtl/ip/gpio/rtl/gpio_core.sv‎

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
module gpio_core #(
66
parameter int unsigned GpiWidth = 8,
77
parameter int unsigned GpoWidth = 16,
8-
parameter int unsigned AddrWidth = 32,
98
parameter int unsigned DataWidth = 32,
10-
parameter int unsigned RegAddr = 6,
9+
parameter int unsigned AddrWidth = 6,
1110
parameter int unsigned DbncWidth = 10
1211
) (
1312
input logic clk_i,
@@ -35,7 +34,7 @@ module gpio_core #(
3534
localparam int unsigned GPIO_STATUS_REG = 32'h14;
3635
localparam int unsigned GPIO_PIN_MASK_REG = 32'h18;
3736

38-
logic [RegAddr-1:0] reg_addr;
37+
logic [DataWidth-1:0] rdata;
3938

4039
logic [GpiWidth-1:0] gp_i_sync;
4140
logic [GpiWidth-1:0] gp_i_dbnc;
@@ -84,7 +83,7 @@ module gpio_core #(
8483
);
8584

8685
// Instantiate step-based debouncers for all GP inputs.
87-
for (genvar i = 0; i < GpiWidth; i++) begin
86+
for (genvar i = 0; i < GpiWidth; i++) begin : gen_dbnc
8887
debounce_step dbnc (
8988
.clk_i,
9089
.rst_ni,
@@ -140,7 +139,7 @@ module gpio_core #(
140139
logic [3:0] unused_i_device_be;
141140

142141
// Assign gp_o_d regarding to device_be_i and GpoWidth.
143-
for (genvar i_byte = 0; i_byte < 4; ++i_byte) begin : g_gp_o_d;
142+
for (genvar i_byte = 0; i_byte < 4; ++i_byte) begin : gen_gp_o_d;
144143
if (i_byte * 8 < GpoWidth) begin : gen_gp_o_d_inner
145144
localparam int gpo_byte_end = (i_byte + 1) * 8 <= GpoWidth ? (i_byte + 1) * 8 : GpoWidth;
146145
assign gp_o_d[gpo_byte_end - 1 : i_byte * 8] =
@@ -156,7 +155,7 @@ module gpio_core #(
156155
end
157156

158157
// Assign pcint_mask_d regarding to device_be_i and GpiWidth.
159-
for (genvar i_byte = 0; i_byte < 4; ++i_byte) begin : g_pcint_mask_d;
158+
for (genvar i_byte = 0; i_byte < 4; ++i_byte) begin : gen_pcint_mask_d;
160159
if (i_byte * 8 < GpiWidth) begin : gen_pcint_mask_d_inner
161160
localparam int gpi_byte_end = (i_byte + 1) * 8 <= GpiWidth ? (i_byte + 1) * 8 : GpiWidth;
162161
assign pcint_mask_d[gpi_byte_end - 1 : i_byte * 8] =
@@ -174,42 +173,32 @@ module gpio_core #(
174173
assign pcint_mode_d = device_be_i[0] ? device_wdata_i[1:0] : pcint_mode;
175174

176175
// Decode write requests.
177-
assign reg_addr = device_addr_i[RegAddr-1:0];
178-
assign gp_o_sel = device_req_i & (reg_addr == GPIO_OUT_REG[RegAddr-1:0]);
179-
assign gp_o_en_sel = device_req_i & (reg_addr == GPIO_OUT_EN_REG[RegAddr-1:0]);
180-
assign ctrl_sel = device_req_i & (reg_addr == GPIO_CTRL_REG[RegAddr-1:0]);
181-
assign status_sel = device_req_i & (reg_addr == GPIO_STATUS_REG[RegAddr-1:0]);
182-
assign pcint_mask_sel = device_req_i & (reg_addr == GPIO_PIN_MASK_REG[RegAddr-1:0]);
183-
184-
// Assign device_rdata_o according to request type.
176+
assign gp_o_sel = device_req_i & (device_addr_i == GPIO_OUT_REG[AddrWidth-1:0]);
177+
assign gp_o_en_sel = device_req_i & (device_addr_i == GPIO_OUT_EN_REG[AddrWidth-1:0]);
178+
assign ctrl_sel = device_req_i & (device_addr_i == GPIO_CTRL_REG[AddrWidth-1:0]);
179+
assign status_sel = device_req_i & (device_addr_i == GPIO_STATUS_REG[AddrWidth-1:0]);
180+
assign pcint_mask_sel = device_req_i & (device_addr_i == GPIO_PIN_MASK_REG[AddrWidth-1:0]);
181+
182+
// Assign rdata according to request type.
185183
always_comb begin
186-
if (reg_addr[RegAddr-1:2] == GPIO_OUT_REG[RegAddr-1:2])
187-
device_rdata_o = {{(DataWidth - GpoWidth){1'b0}}, gp_o};
188-
else if (reg_addr[RegAddr-1:2] == GPIO_IN_REG[RegAddr-1:2])
189-
device_rdata_o = {{(DataWidth - GpiWidth){1'b0}}, gp_i_sync};
190-
else if (reg_addr[RegAddr-1:2] == GPIO_IN_DBNC_REG[RegAddr-1:2])
191-
device_rdata_o = {{(DataWidth - GpiWidth){1'b0}}, gp_i_dbnc};
192-
else if (reg_addr[RegAddr-1:2] == GPIO_OUT_EN_REG[RegAddr-1:2])
193-
device_rdata_o = {{(DataWidth - GpoWidth){1'b0}}, gp_o_en};
194-
else if (reg_addr[RegAddr-1:2] == GPIO_CTRL_REG[RegAddr-1:2])
195-
device_rdata_o = {pcint_enable, 27'b0, pcint_i_sel, 1'b0, pcint_mode};
196-
else if (reg_addr[RegAddr-1:2] == GPIO_STATUS_REG[RegAddr-1:2])
197-
device_rdata_o = {pcint_status, 31'b0};
198-
else if (reg_addr[RegAddr-1:2] == GPIO_PIN_MASK_REG[RegAddr-1:2])
199-
device_rdata_o = {{(DataWidth - GpiWidth){1'b0}}, pcint_mask};
200-
else
201-
device_rdata_o = {(DataWidth){1'b0}};
184+
unique case (device_addr_i[AddrWidth-1:2])
185+
GPIO_OUT_REG[AddrWidth-1:2]: rdata = {{(DataWidth - GpoWidth){1'b0}}, gp_o};
186+
GPIO_IN_REG[AddrWidth-1:2]: rdata = {{(DataWidth - GpiWidth){1'b0}}, gp_i_sync};
187+
GPIO_IN_DBNC_REG[AddrWidth-1:2]: rdata = {{(DataWidth - GpiWidth){1'b0}}, gp_i_dbnc};
188+
GPIO_OUT_EN_REG[AddrWidth-1:2]: rdata = {{(DataWidth - GpoWidth){1'b0}}, gp_o_en};
189+
GPIO_CTRL_REG[AddrWidth-1:2]: rdata = {pcint_enable, 27'b0, pcint_i_sel, 1'b0, pcint_mode};
190+
GPIO_STATUS_REG[AddrWidth-1:2]: rdata = {pcint_status, 31'b0};
191+
GPIO_PIN_MASK_REG[AddrWidth-1:2]: rdata = {{(DataWidth - GpiWidth){1'b0}}, pcint_mask};
192+
default: rdata = {(DataWidth){1'b0}};
193+
endcase
202194
end
203195

196+
assign device_rdata_o = device_req_i ? rdata : '0;
197+
204198
assign pcint_o = pcint_enable & pcint_status;
205199

206200
// Unused signals.
207-
if (AddrWidth > RegAddr) begin : g_unused_addr_bits
208-
logic [AddrWidth-1-RegAddr:0] unused_device_addr;
209-
assign unused_device_addr = device_addr_i[AddrWidth-1:RegAddr];
210-
end
211-
212-
if (DataWidth > GpoWidth) begin : g_unused_gpo_bits
201+
if (DataWidth > GpoWidth) begin : gen_unused_gpo_bits
213202
logic [DataWidth-1-GpoWidth:0] unused_device_wdata;
214203
assign unused_device_wdata = device_wdata_i[DataWidth-1:GpoWidth];
215204
end

‎rtl/system/sonata_system.sv‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ module sonata_system
125125
localparam int unsigned BusDataWidth = 32;
126126
localparam int unsigned DRegAddrWidth = 12; // Debug module uses 12 bits of addressing.
127127
localparam int unsigned TRegAddrWidth = 16; // Timer uses more address bits.
128-
localparam int unsigned GRegAddrWidth = 9; // GPIO array uses 9 bits of addressing.
129128
localparam int unsigned FixedSpiNum = 2; // Number of SPI devices that don't pass through the pinmux
130129
localparam int unsigned TotalSpiNum = SPI_NUM + FixedSpiNum; // The total number of SPI devices
131130
localparam int unsigned FixedGpioNum = 1; // Number of GPIO instances that don't pass through the pinmux
@@ -854,24 +853,22 @@ module sonata_system
854853
gpio #(
855854
.GpiMaxWidth ( GPIO_IOS_WIDTH ),
856855
.GpoMaxWidth ( GPIO_IOS_WIDTH ),
857-
.AddrWidth ( BusAddrWidth ),
858856
.DataWidth ( BusDataWidth ),
859-
.RegAddrWidth ( GRegAddrWidth ),
860857
.NumInstances ( TotalGpioNum ),
861858
.GpiInstWidths( GPIO_INST_IN_WIDTH ),
862859
.GpoInstWidths( GPIO_INST_OUT_WIDTH )
863860
) u_gpio (
864-
.clk_i (clk_sys_i),
865-
.rst_ni (rst_sys_ni),
861+
.clk_i (clk_sys_i),
862+
.rst_ni (rst_sys_ni),
866863

867-
.tl_i (tl_gpio_h2d),
868-
.tl_o (tl_gpio_d2h),
864+
.tl_i (tl_gpio_h2d),
865+
.tl_o (tl_gpio_d2h),
869866

870-
.gp_i(gpio_from_pins),
871-
.gp_o(gpio_to_pins),
872-
.gp_o_en(gpio_to_pins_enable),
867+
.gp_i (gpio_from_pins),
868+
.gp_o (gpio_to_pins),
869+
.gp_o_en (gpio_to_pins_enable),
873870

874-
.pcint_o(gpio_interrupts)
871+
.pcint_o (gpio_interrupts)
875872
);
876873

877874
// Digital inputs from Arduino shield analog(ue) pins currently unused

0 commit comments

Comments
 (0)