Error (10028): Can't resolve multiple constant drivers

Pastel · Oct 28, 2019

Hi guys!

It's been a while since my last post, I had to work on other things.
To summarize: I made a signal generator, 8 channels, using a MAX10 and parallel
DACs, and the first part of the design works fine.
I had timing problems at some point, but after re-reading replies (Thanks FVM!!),
I found out that using a PLL to generate my clocks is a lot better. Basically it
fixed everything.

Now I have another problem that looks like a classical one. I needed to communicate
with the FPGA to ask it to send data by SPI to other devices. I first made a very
simple SPI slave engine. NB: There are verilog SPI engines on the net, but a way
too complicated to grasp at once, so I wanted to build my own brew.

The code is shown below. It operates at a single mode (clock pulse positive, I think
it's the (0,0) mode). The version below works perfectly well, but it has to receive
32 bit frames only. If for some reason the MCU transfer is interrupted, then the
data frame (shift_reg) will not be empty but invalid. Not a problem if the next frame is exactly
32 bits, because everything would be cleared by shift, but anyway I would like it to be a bit
cleaner, so as soon as I drop the chip select, I would like to set the shift register to 0.

Code Verilog - [expand]

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37

module inspi (
leds,
chip_id,
miso, mosi, clk, cs
);
output reg[3:0] chip_id; // This will get the address of the slave devices
output reg[5:0] leds; // LEDs to check the functionality
output reg miso; // As this is a slave, only MISO is output
input mosi;
input clk;
input cs;
// Internal variables
reg[31:0] shift_reg; // Register to receive the MOSI bits
// Initialization
initial begin
chip_id <= 'hF; // Set the address of the external chips to F (none selected)
shift_reg <= 0;
end
// At every clock, shift and add the current MOSI bit
always@(posedge clk) begin
shift_reg = shift_reg << 1;
if(mosi == 1) begin
shift_reg <= shift_reg+1;
end
end
// At end of transfer, CS goes high. Return the LEDs status and chip IDs
// received by SPI. All other bits are ignored for the time being.
always@(posedge cs) begin
leds <= shift_reg[5:0];
chip_id = shift_reg[27:24];
end
// At negative edge of CS, we know that the transfer is about to happen.
// Set the shift register to 0.
always@(negedge cs) begin
// shift_reg <= 0;
end
endmodule

So basically:
1. On rising edge of the clock, I shift the register left, and add one data bit on
the right.
2. When cs goes high, it means that the transfer is finished, so I output some
parts of the frame. Leds is an array of 6 leds. This works fine. When I receive
a 32 bit frame, I take the last 6 bits and send them to the LEDs. I also have
15 SPI devices driven by the FPGA. I receive the chip id externally and can verify
with the scope that the right bit has been dropped.
3. When the cs goes low, it means that I'm going to receive data. I would like to
clear the shift_reg for a new transfer, but I get this error:

Error (10028): Can't resolve multiple constant drivers for net "shift_reg[26]" at inspi.sv(35)

Not only shift_reg[26], I get one error like this per bit.

I have tried to google for this error, and found many replies, but I still don't
understand the problem. The shift_reg is accessed at different events, so I don't
understand where there could be a clash.
Could anybody (try to) explain me what's wrong and how to fix it?
NB: when compiled as shown below, it works. The problem arises when I uncomment
the // shift_reg <=0; line.

NB: I have also checked the similar issues on this forum, but apparently everybody is
using VHDL.

Thanks for any hint!

Pastel

std_match · Oct 28, 2019

Only drive signals from one process block.
Use as few clocks as possible. In this case only use clk as a clock, and detect the cs edges inside the "always@(posedge clk)" process block.

Don't set the lowest bit by doing "shift_reg <= shift_reg+1;" even if you "know" that the lowest bit is zero.
That is fine for software where such an addition costs nothing, but you don't want an adder for this in hardware.
You should not assume that the tool is smart enough to know that the lowest bit is zero before the addition, so only the lowest bit is affected by the operation.
In hardware, if you want to set the lowest bit, just do it. Why add one to the whole 32-bit register?

Pastel · Oct 28, 2019

Hello!

Thanks for your reply.
Yes, indeed, using a 32 bit adder was a bit too much.
For those who may have the same problem, I changed the always@(posedge clk) like this:

Code Verilog - [expand]
1
2
3
4
always@(posedge clk) begin
    shift_reg <= shift_reg << 1;
    shift_reg[0] <= mosi;
end

One question about your comment:

Use as few clocks as possible. In this case only use clk as a clock, and detect the cs edges inside
the "always@(posedge clk)" process block.

I wasn't aware I was using more than one clock (called clk) in my code. Do you mean that as soon
as you have an always@(posedge sig) block, the signal sig is considered as a clock?

Now one problem:
The clock will happen only if CS is already low and stop before goes back to high. So if I test CS inside
of the always block, there is no way I will get an edge on CS, it will always be low.

Now I know that I can say always@(posedge clk or negedge cs), but isn't this equivalent to what I have
already written? I tried and it works, but it also worked before.

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
always@(posedge clk or posedge cs) begin
        if(cs == 1) begin
            leds <= shift_reg[5:0];
            chip_id = shift_reg[27:24];
        end
        else begin
            shift_reg <= shift_reg << 1;
            shift_reg[0] <= mosi;
        end
    end

Beside this, I need to do something on falling edge of CS (reset the shift register) and also on rising
edge of cs (output what I need from the SPI frame). And always@(posedge clk or negedge cs or posedge cs)
doesn't compile.
To fix this, I tried to define a pcs (for positive cs, active high) as ~cs.
Now always@(posedge clk or posedge pcs or posedge cs) compiles fine, but it doesn't work anymore
(the LEDs are not lit anymore as they used to be).

Here is the whole (non working) code: (I found the "wire pcs = ~cs;" trick on the net)

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
module inspi (
    leds,
    chip_id,
    miso, mosi, clk, cs
);
    output reg[3:0] chip_id;
    output reg[5:0] leds;
    output reg miso;
    input mosi;
    input clk;
    input cs;
 
    reg[31:0]   shift_reg;
    wire pcs = ~cs;
    
    initial begin
        chip_id <= 'hF;
        shift_reg <= 0;
    end
 
    always@(posedge clk or posedge pcs or posedge cs) begin
        if(pcs == 1) begin
            shift_reg <= 0;
        end
        else if(cs == 1) begin
            leds <= shift_reg[5:0];
            chip_id = shift_reg[27:24];
        end
        else begin
            shift_reg <= shift_reg << 1;
            shift_reg[0] <= mosi;
        end
    end
endmodule

Thanks for any hint!

Pastel

std_match · Oct 28, 2019

Ok, now I see that clk is your SPI clock from the master.
I suggest that you use some other high speed clock as your only clock in the whole design and treat all SPI signals as normal I/O.
Everything is much easier with everything in one clock domain. It also solves your problem with no clock when CS is inactive.

The "master clock" must have a higher frequency the frequency than the SPI clock.
I suggest at least 8 times higher for an SPI slave to give margin for synchronization registers on all inputs. Use 50 MHz, 100 MHz or whatever you have available (or create with a PLL).
Clock all SPI inputs via 2 registers before using them to avoid metastability problems.
Try to minimize the number of clocks. Only one clock in the whole design is nice.

ads-ee · Oct 28, 2019

Pastel said:

Hello!

Thanks for your reply.
Yes, indeed, using a 32 bit adder was a bit too much.
For those who may have the same problem, I changed the always@(posedge clk) like this:

Code Verilog - [expand]
1
2
3
4
always@(posedge clk) begin
    shift_reg <= shift_reg << 1;
    shift_reg[0] <= mosi;
end

Ugh, why use two lines do the same thing that can be done in one line. I also think the one line is easier to see is a shift register (at least to my hardware centric brain).

Code Verilog - [expand]
1
2
3
4
always @ (posedge clk) begin
  shift_reg <= {shift_reg, mosi};  // a shift reg, Verilog doesn't give a hoot about the 33-bits getting loaded into the 32-bit value, though you will get warnings about this
  shift_reg <= {shift_reg[30:0], mosi};  // if you need to want to get rid of the warning above.
end

- - - Updated - - -

std_match said:
Ok, now I see that clk is your SPI clock from the master.
I suggest that you use some other high speed clock as your only clock in the whole design and treat all SPI signals as normal I/O.
Everything is much easier with everything in one clock domain. It also solves your problem with no clock when CS is inactive.

The "master clock" must have a higher frequency the frequency than the SPI clock.
I suggest at least 8 times higher for an SPI slave to give margin for synchronization registers on all inputs. Use 50 MHz, 100 MHz or whatever you have available (or create with a PLL).
Clock all SPI inputs via 2 registers before using them to avoid metastability problems.
Try to minimize the number of clocks. Only one clock in the whole design is nice.

Using the SPI clock as a true clock, is sometimes required....
I worked on a design that had no clocks running until the external PLL was programmed. The processor used SPI to command the FPGA to program the PLL. The SPI clock was the only clock running in the system besides the processors internal osc after boot up (FPGA did not get that clock).

- - - Updated - - -

The use of posedge cs is probably going to result in non-optimal code being generated to be able to asynhronously load the values leds and chip_id.

You should not use such coding as the second posedge or negedge should only be used as an asynchronous reset/set with a static constant value being assigned to the FFs produced by synthesis. This is not software and this language may look like C syntax, but is not C.

Uh, wire pcs = ~cs; isn't some trick. It's perfectly valid Verilog code with a continuous assignment done as part of the declaration of the wire. If someone else referred to it as a trick then they need to read the LRM.

Also as you are mixing multiple different signals into a single always block you are running into a bunch of problems as you need all those extra posedge terms to get the simulation to do what you want. Ever consider that the always blocks should only have the FFs necessary to implement one specific function. e.g. the shfit_reg should only be in a single always block all by itself. The leds should be in it's own separate always block or may with only one other signal chip_id as they share the same common cs==1 statement.

initial blocks should not be used in synthesizable code, they may or may not be synthesized depending on the FPGA and the tools used. They absolutely won't synthesize in ASICs and are ignored, if you design requires a initial value to work then you absolutely need to have a reset of some sort (i.e like the CS == 1'b1) to initialize those signals.

...and that if (cs == 1) statement...synthesis (depending on the tool) will report comparing between a 32-bit and a 1-bit it is better to use either if (cs == 1'b1) or preferably if (cs).

Pastel · Oct 29, 2019

Hello!

Thanks for your replies.
I changed the code as you suggested (shift_reg <= {shift_reg[30:0], mosi}

Indeed, by processing many different edges, the code became a real mess,
and everytime I tried to implement initialization, I got compilation errors.
So I rewrote another version as suggested by std_match. It's a kind of state
machine that only depends on the system clock and the SPI inputs (which are
at a very low frequency compared to the system, 10MHz, vs 100).
In the always@(posedge sysclock), I keep track of the values of spi_clock
and spi_cs by using 2 internal variables, m_spi_cs and m_spi_clk.

It looks more or less like this. There is no error like I used to have
(can't resolve multiple constant drivers), and it seems to work fine.
I'm not sure that's the most efficient way to do it, but at least it works,
and now the shift register is initialized before filling it. I'm going to
add a bit counter and a status report, possibly a 1 bit error report
if the number of bits is wrong.

And I will now implement the master SPI which communicates with board
devices, sending the contents of what has been received from SPI slave.

Anyway thanks again for your replies which were really helpful, and feel free
to comment if you have time.

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
always@(posedge sysclock) begin
    if(spi_cs != m_spi_cs) begin        //  There was a change on cs
        m_spi_cs <= spi_cs;             //  Store the new cs state
        if(spi_cs) begin                //  Chip select had a rising edge
            leds <= shift_reg[5:0];     //  Output the leds status
            chip_id = shift_reg[27:24]; //  Output the chip ID
        end
        else begin                      //  Chip select had a falling edge
            shift_reg <= 0;             //  Reset the shift register
        end
    end
    if(spi_clk != m_spi_clk) begin      //  The polarity of spi clock has just changed
        m_spi_clk <= spi_clk;           //  Store the new polarity
        if(spi_clk) begin               //  The observed change was a rising edge
            shift_reg <= {shift_reg[30:0], mosi};
        end
    end
end

Pastel

Welcome to EDAboard.com

Error (10028): Can't resolve multiple constant drivers

Pastel

Member level 3

std_match

Advanced Member level 4

Pastel

Member level 3

std_match

Advanced Member level 4

ads-ee

Super Moderator

Pastel

Member level 3

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Connect with us

Online statistics

Forum statistics